Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed invalid function date_trunc when connecting to hive -- issue#7270 #7657

Closed
wants to merge 8 commits into from
6 changes: 3 additions & 3 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ pip install -r requirements-dev.txt
pip install -e .

# Create an admin user in your metadata database
fabmanager create-admin --app superset
flask fab create-admin --app superset

# Initialize the database
superset db upgrade
Expand Down Expand Up @@ -594,7 +594,7 @@ LANGUAGES = {
### Extracting new strings for translation

```bash
fabmanager babel-extract --target superset/translations --output superset/translations/messages.pot --config superset/translations/babel.cfg -k _ -k __ -k t -k tn -k tct
flask fab babel-extract --target superset/translations --output superset/translations/messages.pot --config superset/translations/babel.cfg -k _ -k __ -k t -k tn -k tct
```

You can then translate the strings gathered in files located under
Expand All @@ -607,7 +607,7 @@ For the translations to take effect:
```bash
# In the case of JS translation, we need to convert the PO file into a JSON file, and we need the global download of the npm package po2json.
npm install -g po2json
fabmanager babel-compile --target superset/translations
flask fab babel-compile --target superset/translations
# Convert the en PO file into a JSON file
po2json -d superset -f jed1.x superset/translations/en/LC_MESSAGES/messages.po superset/translations/en/LC_MESSAGES/messages.json
```
Expand Down
2 changes: 1 addition & 1 deletion contrib/docker/docker-init.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
set -ex

# Create an admin user (you will be prompted to set username, first and last name before setting a password)
fabmanager create-admin --app superset
flask fab create-admin --app superset

# Initialize the database
superset db upgrade
Expand Down
111 changes: 106 additions & 5 deletions superset/assets/spec/javascripts/components/OnPasteSelect_spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,47 @@ describe('OnPasteSelect', () => {
});

describe('onPaste', () => {
it('calls onChange with pasted values', () => {
it('calls onChange with pasted comma separated values', () => {
wrapper.instance().onPaste(evt);
expected = props.options.slice(0, 4);
expect(props.onChange.calledWith(expected)).toBe(true);
expect(evt.preventDefault.called).toBe(true);
expect(props.isValidNewOption.callCount).toBe(5);
});

it('calls onChange without any duplicate values and adds new values', () => {
it('calls onChange with pasted new line separated values', () => {
evt.clipboardData.getData = sinon.spy(() =>
'United States\nChina\nRussian Federation\nIndia',
);
wrapper.instance().onPaste(evt);
expected = [
props.options[0],
props.options[1],
props.options[4],
props.options[2],
];
expect(props.onChange.calledWith(expected)).toBe(true);
expect(evt.preventDefault.called).toBe(true);
expect(props.isValidNewOption.callCount).toBe(9);
});

it('calls onChange with pasted tab separated values', () => {
evt.clipboardData.getData = sinon.spy(() =>
'Russian Federation\tMexico\tIndia\tCanada',
);
wrapper.instance().onPaste(evt);
expected = [
props.options[4],
props.options[6],
props.options[2],
props.options[3],
];
expect(props.onChange.calledWith(expected)).toBe(true);
expect(evt.preventDefault.called).toBe(true);
expect(props.isValidNewOption.callCount).toBe(13);
});

it('calls onChange without duplicate values and adds new comma separated values', () => {
evt.clipboardData.getData = sinon.spy(() =>
'China, China, China, China, Mexico, Mexico, Chi na, Mexico, ',
);
Expand All @@ -94,12 +126,43 @@ describe('OnPasteSelect', () => {
wrapper.instance().onPaste(evt);
expect(props.onChange.calledWith(expected)).toBe(true);
expect(evt.preventDefault.called).toBe(true);
expect(props.isValidNewOption.callCount).toBe(9);
expect(props.isValidNewOption.callCount).toBe(17);
expect(props.options[0].value).toBe(expected[2].value);
props.options.splice(0, 1);
});

it('calls onChange with currently selected values and new values', () => {
it('calls onChange without duplicate values and parses new line separated values', () => {
evt.clipboardData.getData = sinon.spy(() =>
'United States\nCanada\nMexico\nUnited States\nCanada',
);
expected = [
props.options[0],
props.options[3],
props.options[6],
];
wrapper.instance().onPaste(evt);
expect(props.onChange.calledWith(expected)).toBe(true);
expect(evt.preventDefault.called).toBe(true);
expect(props.isValidNewOption.callCount).toBe(20);
});

it('calls onChange without duplicate values and parses tab separated values', () => {
evt.clipboardData.getData = sinon.spy(() =>
'China\tIndia\tChina\tRussian Federation\tJapan\tJapan',
);
expected = [
props.options[1],
props.options[2],
props.options[4],
props.options[5],
];
wrapper.instance().onPaste(evt);
expect(props.onChange.calledWith(expected)).toBe(true);
expect(evt.preventDefault.called).toBe(true);
expect(props.isValidNewOption.callCount).toBe(24);
});

it('calls onChange with currently selected values and new comma separated values', () => {
props.value = ['United States', 'Canada', 'Mexico'];
evt.clipboardData.getData = sinon.spy(() =>
'United States, Canada, Japan, India',
Expand All @@ -115,7 +178,45 @@ describe('OnPasteSelect', () => {
wrapper.instance().onPaste(evt);
expect(props.onChange.calledWith(expected)).toBe(true);
expect(evt.preventDefault.called).toBe(true);
expect(props.isValidNewOption.callCount).toBe(11);
expect(props.isValidNewOption.callCount).toBe(26);
});

it('calls onChange with currently selected values and new "new line" separated values', () => {
props.value = ['China', 'India', 'Japan'];
evt.clipboardData.getData = sinon.spy(() =>
'Mexico\nJapan\nIndia',
);
wrapper = shallow(<OnPasteSelect {...props} />);
expected = [
props.options[1],
props.options[2],
props.options[5],
props.options[6],
];
wrapper.instance().onPaste(evt);
expect(props.onChange.calledWith(expected)).toBe(true);
expect(evt.preventDefault.called).toBe(true);
expect(props.isValidNewOption.callCount).toBe(27);
});

it('calls onChange with currently selected values and new tab separated values', () => {
props.value = ['United States', 'Canada', 'Mexico', 'Russian Federation'];
evt.clipboardData.getData = sinon.spy(() =>
'United States\tCanada\tJapan\tIndia',
);
wrapper = shallow(<OnPasteSelect {...props} />);
expected = [
props.options[0],
props.options[3],
props.options[6],
props.options[4],
props.options[5],
props.options[2],
];
wrapper.instance().onPaste(evt);
expect(props.onChange.calledWith(expected)).toBe(true);
expect(evt.preventDefault.called).toBe(true);
expect(props.isValidNewOption.callCount).toBe(29);
});
});
});
4 changes: 2 additions & 2 deletions superset/assets/src/components/OnPasteSelect.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export default class OnPasteSelect extends React.Component {
}

OnPasteSelect.propTypes = {
separator: PropTypes.string.isRequired,
separator: PropTypes.array.isRequired,
selectWrap: PropTypes.func.isRequired,
refFunc: PropTypes.func,
onChange: PropTypes.func.isRequired,
Expand All @@ -96,7 +96,7 @@ OnPasteSelect.propTypes = {
isValidNewOption: PropTypes.func,
};
OnPasteSelect.defaultProps = {
separator: ',',
separator: [',', '\n', '\t'],
selectWrap: Select,
valueKey: 'value',
labelKey: 'label',
Expand Down
15 changes: 14 additions & 1 deletion superset/db_engine_specs.py
Original file line number Diff line number Diff line change
Expand Up @@ -1812,10 +1812,23 @@ def latest_sub_partition(cls, table_name, schema, database, **kwargs):

class HiveEngineSpec(PrestoEngineSpec):

"""Reuses PrestoEngineSpec functionality."""
"""Based on PrestoEngineSpec but using hive date functions instead"""

engine = 'hive'
max_column_name_length = 767
time_grain_functions = {
None: '{col}',
'PT1S': "from_unixtime(unix_timestamp({col}), 'yyyy-MM-dd HH:mm:ss')",
Copy link
Member

@john-bodley john-bodley Jun 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jeepxiaozi I don't believe this logic is correct, i.e., it's assuming that the {col} is of type STRING (with a pre-specified format) where it could be a DATE or TIMESTAMP. I plan to address this issue in SIP-15A but for the meantime I would suggest implying that {col} can be successfully cast to a TIMESTAMP, i.e., CAST({col} AS TIMESTAMP} (see here for details) as this is consistent with the logic in the other engines.

I'll address the invalid casting in SIP-15A which I'm actively working on.

'PT1M': "from_unixtime(unix_timestamp({col}), 'yyyy-MM-dd HH:mm:00')",
'PT1H': "from_unixtime(unix_timestamp({col}), 'yyyy-MM-dd HH:00:00')",
'P1D': "from_unixtime(unix_timestamp({col}), 'yyyy-MM-dd 00:00:00')",
'P1W': "next_day(date_add({col}, -7),'Monday')",
'P1M': "trunc({col}, 'MM')",
'P0.25Y': "add_months(trunc({col}, 'MM'), -(month({col})-1)%3)",
'P1Y': "from_unixtime(unix_timestamp({col}), 'yyyy-01-01 00:00:00')",
'P1W/1970-01-03T00:00:00Z': "next_day({col}, 'Saturday')",
'1969-12-28T00:00:00Z/P1W': "next_day(date_add({col}, -7),'Sunday')",
}

# Scoping regex at class level to avoid recompiling
# 17/02/07 19:36:38 INFO ql.Driver: Total jobs = 5
Expand Down
6 changes: 6 additions & 0 deletions superset/views/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,12 @@ class SliceModelView(SupersetModelView, DeleteMixin): # noqa
'viz_type': _('Visualization Type'),
}

add_form_query_rel_fields = {
'dashboards': [['name', DashboardFilter, None]],
}

edit_form_query_rel_fields = add_form_query_rel_fields

def pre_add(self, obj):
utils.validate_json(obj.params)

Expand Down