QT/superset2
1
0
Fork 0
mirror of https://github.com/apache/superset.git synced 2026-08-12 11:11:01 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(query): order by adhoc metrics should trigger group by (#13434)

Browse Source 
* fix(query): properly select adhoc metrics in orderby

* Throw error when sql is empty

* Allow `metrics` to be None

* Always use alias in orderby for metrics

* Bump table chart version and migrate histogram to typescript

* Fix Histogram without groupby

* Fix Presto birth names test

* Raw records mode should not aggregate
This commit is contained in:
Jesse Yang
2021-03-16 21:00:03 -07:00
committed by GitHub
parent 72d19b6b35
commit bd1d6acb0c
22 changed files with 8896 additions and 171 deletions
Download Patch File Download Diff File Expand all files Collapse all files
superset-frontend/cypress-base/cypress/integration/explore/visualizations/histogram.test.js → superset-frontend/cypress-base/cypress/integration/explore/visualizations/histogram.test.ts
+5 -5
View File
@@ -16,8 +16,10 @@
* specific language governing permissions and limitations
* under the License.
*/
import { QueryFormData } from '@superset-ui/core';
describe('Visualization > Histogram', () => {
const HISTOGRAM_FORM_DATA = {
const HISTOGRAM_FORM_DATA: QueryFormData = {
datasource: '3__table',
viz_type: 'histogram',
slice_id: 60,
@@ -36,7 +38,7 @@ describe('Visualization > Histogram', () => {
normalized: false,
};
function verify(formData) {
function verify(formData: QueryFormData) {
cy.visitChartByParams(JSON.stringify(formData));
cy.verifySliceSuccess({ waitAlias: '@getJson', chartSelector: 'svg' });
}
@@ -73,12 +75,10 @@ describe('Visualization > Histogram', () => {
adhoc_filters: [
{
expressionType: 'SIMPLE',
clause: 'WHERE',
subject: 'state',
operator: '==',
comparator: 'CA',
clause: 'WHERE',
sqlExpression: null,
filterOptionName: 'filter_tqx1en70hh_7nksse7nqic',
},
],
});
superset-frontend/cypress-base/cypress/integration/explore/visualizations/table.test.ts
+19 -3
View File
@@ -151,6 +151,21 @@ describe('Visualization > Table', () => {
cy.verifySliceSuccess({ waitAlias: '@chartData', chartSelector: 'table' });
});
it('Test table with groupby + order by + no metric', () => {
cy.visitChartByParams({
...VIZ_DEFAULTS,
metrics: [],
groupby: ['name'],
timeseries_limit_metric: NUM_METRIC,
order_desc: true,
});
// should contain only the group by column
cy.get('.chart-container th').its('length').should('eq', 1);
// should order correctly
cy.get('.chart-container td:eq(0)').contains('Michael');
cy.verifySliceSuccess({ waitAlias: '@chartData', chartSelector: 'table' });
});
it('Test table with groupby and limit', () => {
const limit = 10;
const formData = {
@@ -172,9 +187,9 @@ describe('Visualization > Table', () => {
...VIZ_DEFAULTS,
// should still work when query_mode is not-set/invalid
query_mode: undefined,
all_columns: ['name'],
all_columns: ['state'],
metrics: [],
row_limit: 10,
row_limit: 100,
});
// should display in raw records mode
@@ -183,8 +198,9 @@ describe('Visualization > Table', () => {
cy.get('div[data-test="groupby"]').should('not.exist');
cy.verifySliceSuccess({ waitAlias: '@chartData', chartSelector: 'table' });
cy.get('[data-test="row-count-label"]').contains('100 rows');
// should allow switch to aggregate mode
// should allow switch back to aggregate mode
cy.get('div[data-test="query_mode"] .btn').contains('Aggregate').click();
cy.get('div[data-test="query_mode"] .btn.active').contains('Aggregate');
cy.get('div[data-test="all_columns"]').should('not.exist');
superset-frontend/cypress-base/package-lock.json
Generated
+8678 -19
View File
File diff suppressed because it is too large Load Diff
superset-frontend/cypress-base/package.json
+2
View File
@@ -11,6 +11,8 @@
"license": "Apache-2.0",
"dependencies": {
"@cypress/code-coverage": "^3.9.2",
"@superset-ui/core": "^0.17.18",
"react-dom": "^16.13.0",
"rison": "^0.1.1",
"shortid": "^2.2.15"
},
superset-frontend/package-lock.json
Generated
+8 -8
View File
@@ -14,7 +14,7 @@
"@data-ui/sparkline": "^0.0.84",
"@emotion/core": "^10.0.35",
"@superset-ui/chart-controls": "^0.17.19",
"@superset-ui/core": "^0.17.15",
"@superset-ui/core": "^0.17.18",
"@superset-ui/legacy-plugin-chart-calendar": "^0.17.19",
"@superset-ui/legacy-plugin-chart-chord": "^0.17.19",
"@superset-ui/legacy-plugin-chart-country-map": "^0.17.19",
@@ -38,7 +38,7 @@
"@superset-ui/legacy-preset-chart-deckgl": "^0.4.6",
"@superset-ui/legacy-preset-chart-nvd3": "^0.17.19",
"@superset-ui/plugin-chart-echarts": "^0.17.19",
"@superset-ui/plugin-chart-table": "^0.17.19",
"@superset-ui/plugin-chart-table": "^0.17.20",
"@superset-ui/plugin-chart-word-cloud": "^0.17.19",
"@superset-ui/preset-chart-xy": "^0.17.19",
"@vx/responsive": "^0.0.195",
@@ -16518,9 +16518,9 @@
}
},
"node_modules/@superset-ui/plugin-chart-table": {
"version": "0.17.19",
"resolved": "https://registry.npmjs.org/@superset-ui/plugin-chart-table/-/plugin-chart-table-0.17.19.tgz",
"integrity": "sha512-pYoR4RSUI2dbIDnuBSpa0WchMLnKX5QlVeAnFywp6T3UdPovOdcSVVSWrdfDrmX6UqSEKsje5Rj42FQWjgoAsQ==",
"version": "0.17.20",
"resolved": "https://registry.npmjs.org/@superset-ui/plugin-chart-table/-/plugin-chart-table-0.17.20.tgz",
"integrity": "sha512-gWcHbt36FTvFPc0MZzX+EHgZDiB8jwdAvMQX2AFeb9BEiba45b9ZdkuLyslqa4LQM84KCuVD+V+I29VplacTAg==",
"dependencies": {
"@emotion/core": "^10.0.28",
"@superset-ui/chart-controls": "0.17.19",
@@ -72450,9 +72450,9 @@
}
},
"@superset-ui/plugin-chart-table": {
"version": "0.17.19",
"resolved": "https://registry.npmjs.org/@superset-ui/plugin-chart-table/-/plugin-chart-table-0.17.19.tgz",
"integrity": "sha512-pYoR4RSUI2dbIDnuBSpa0WchMLnKX5QlVeAnFywp6T3UdPovOdcSVVSWrdfDrmX6UqSEKsje5Rj42FQWjgoAsQ==",
"version": "0.17.20",
"resolved": "https://registry.npmjs.org/@superset-ui/plugin-chart-table/-/plugin-chart-table-0.17.20.tgz",
"integrity": "sha512-gWcHbt36FTvFPc0MZzX+EHgZDiB8jwdAvMQX2AFeb9BEiba45b9ZdkuLyslqa4LQM84KCuVD+V+I29VplacTAg==",
"requires": {
"@emotion/core": "^10.0.28",
"@superset-ui/chart-controls": "0.17.19",
superset-frontend/package.json
+2 -2
View File
@@ -66,7 +66,7 @@
"@data-ui/sparkline": "^0.0.84",
"@emotion/core": "^10.0.35",
"@superset-ui/chart-controls": "^0.17.19",
"@superset-ui/core": "^0.17.15",
"@superset-ui/core": "^0.17.18",
"@superset-ui/legacy-plugin-chart-calendar": "^0.17.19",
"@superset-ui/legacy-plugin-chart-chord": "^0.17.19",
"@superset-ui/legacy-plugin-chart-country-map": "^0.17.19",
@@ -90,7 +90,7 @@
"@superset-ui/legacy-preset-chart-deckgl": "^0.4.6",
"@superset-ui/legacy-preset-chart-nvd3": "^0.17.19",
"@superset-ui/plugin-chart-echarts": "^0.17.19",
"@superset-ui/plugin-chart-table": "^0.17.19",
"@superset-ui/plugin-chart-table": "^0.17.20",
"@superset-ui/plugin-chart-word-cloud": "^0.17.19",
"@superset-ui/preset-chart-xy": "^0.17.19",
"@vx/responsive": "^0.0.195",
superset-frontend/src/explore/components/DisplayQueryButton.jsx
+8 -4
View File
@@ -91,10 +91,14 @@ export const DisplayQueryButton = props => {
setError(null);
})
.catch(response => {
getClientErrorObject(response).then(({ error, statusText }) => {
setError(error || statusText || t('Sorry, An error occurred'));
setIsLoading(false);
});
getClientErrorObject(response).then(
({ error, message, statusText }) => {
setError(
error || message || statusText || t('Sorry, An error occurred'),
);
setIsLoading(false);
},
);
});
};
superset/common/query_actions.py
+8 -6
View File
@@ -80,10 +80,12 @@ def _get_query(
query_context: "QueryContext", query_obj: "QueryObject", _: bool,
) -> Dict[str, Any]:
datasource = _get_datasource(query_context, query_obj)
return {
"query": datasource.get_query_str(query_obj.to_dict()),
"language": datasource.query_language,
}
result = {"language": datasource.query_language}
try:
result["query"] = datasource.get_query_str(query_obj.to_dict())
except QueryObjectValidationError as err:
result["error"] = err.message
return result
def _get_full(
@@ -118,7 +120,7 @@ def _get_full(
] + rejected_time_columns
if result_type == ChartDataResultType.RESULTS and status != QueryStatus.FAILED:
return {"data": payload["data"]}
return {"data": payload.get("data")}
return payload
@@ -143,7 +145,7 @@ def _get_results(
query_context: "QueryContext", query_obj: "QueryObject", force_cached: bool = False
) -> Dict[str, Any]:
payload = _get_full(query_context, query_obj, force_cached)
return {"data": payload["data"]}
return {"data": payload.get("data"), "error": payload.get("error")}
_result_type_functions: Dict[
superset/common/query_context.py
+2 -2
View File
@@ -129,7 +129,7 @@ class QueryContext:
if self.enforce_numerical_metrics:
self.df_metrics_to_num(df, query_object)
df.replace([np.inf, -np.inf], np.nan)
df.replace([np.inf, -np.inf], np.nan, inplace=True)
df = query_object.exec_post_processing(df)
return {
@@ -345,7 +345,7 @@ class QueryContext:
col
for col in query_obj.columns
+ query_obj.groupby
+ get_column_names_from_metrics(query_obj.metrics)
+ get_column_names_from_metrics(query_obj.metrics or [])
if col not in self.datasource.column_names and col != DTTM_ALIAS
]
if invalid_columns:
superset/common/query_object.py
+16 -15
View File
@@ -28,7 +28,7 @@ from superset import app, db
from superset.connectors.base.models import BaseDatasource
from superset.connectors.connector_registry import ConnectorRegistry
from superset.exceptions import QueryObjectValidationError
from superset.typing import Metric
from superset.typing import Metric, OrderBy
from superset.utils import pandas_postprocessing
from superset.utils.core import (
ChartDataResultType,
@@ -36,6 +36,7 @@ from superset.utils.core import (
DTTM_ALIAS,
find_duplicates,
get_metric_names,
is_adhoc_metric,
json_int_dttm_ser,
)
from superset.utils.date_parser import get_since_until, parse_human_timedelta
@@ -80,7 +81,7 @@ class QueryObject:
is_timeseries: bool
time_shift: Optional[timedelta]
groupby: List[str]
metrics: List[Union[Dict[str, Any], str]]
metrics: Optional[List[Metric]]
row_limit: int
row_offset: int
filter: List[Dict[str, Any]]
@@ -89,7 +90,7 @@ class QueryObject:
order_desc: bool
extras: Dict[str, Any]
columns: List[str]
orderby: List[List[str]]
orderby: List[OrderBy]
post_processing: List[Dict[str, Any]]
datasource: Optional[BaseDatasource]
result_type: Optional[ChartDataResultType]
@@ -116,11 +117,16 @@ class QueryObject:
order_desc: bool = True,
extras: Optional[Dict[str, Any]] = None,
columns: Optional[List[str]] = None,
orderby: Optional[List[List[str]]] = None,
orderby: Optional[List[OrderBy]] = None,
post_processing: Optional[List[Optional[Dict[str, Any]]]] = None,
is_rowcount: bool = False,
**kwargs: Any,
):
columns = columns or []
groupby = groupby or []
extras = extras or {}
annotation_layers = annotation_layers or []
self.is_rowcount = is_rowcount
self.datasource = None
if datasource:
@@ -128,12 +134,7 @@ class QueryObject:
str(datasource["type"]), int(datasource["id"]), db.session
)
self.result_type = result_type
annotation_layers = annotation_layers or []
self.apply_fetch_values_predicate = apply_fetch_values_predicate or False
metrics = metrics or []
columns = columns or []
groupby = groupby or []
extras = extras or {}
self.annotation_layers = [
layer
for layer in annotation_layers
@@ -169,11 +170,11 @@ class QueryObject:
# 1. 'metric_name' - name of predefined metric
# 2. { label: 'label_name' } - legacy format for a predefined metric
# 3. { expressionType: 'SIMPLE' | 'SQL', ... } - adhoc metric
self.metrics = [
metric
if isinstance(metric, str) or "expressionType" in metric
else metric["label"] # type: ignore
for metric in metrics
self.metrics = metrics and [
x
if isinstance(x, str) or is_adhoc_metric(x)
else x["label"] # type: ignore
for x in metrics
]
self.row_limit = config["ROW_LIMIT"] if row_limit is None else row_limit
@@ -236,7 +237,7 @@ class QueryObject:
@property
def metric_names(self) -> List[str]:
"""Return metrics names (labels), coerce adhoc metrics to strings."""
return get_metric_names(self.metrics)
return get_metric_names(self.metrics or [])
@property
def column_names(self) -> List[str]:
superset/connectors/base/models.py
+5 -4
View File
@@ -112,11 +112,12 @@ class BaseDatasource(
update_from_object_fields: List[str]
@property
def kind(self) -> str:
if self.sql:
return DatasourceKind.VIRTUAL.value
def kind(self) -> DatasourceKind:
return DatasourceKind.VIRTUAL if self.sql else DatasourceKind.PHYSICAL
return DatasourceKind.PHYSICAL.value
@property
def is_virtual(self) -> bool:
return self.kind == DatasourceKind.VIRTUAL
@declared_attr
def slices(self) -> RelationshipProperty:
superset/connectors/sqla/models.py
+121 -81
View File
@@ -48,6 +48,7 @@ from sqlalchemy import (
from sqlalchemy.orm import backref, Query, relationship, RelationshipProperty, Session
from sqlalchemy.schema import UniqueConstraint
from sqlalchemy.sql import column, ColumnElement, literal_column, table, text
from sqlalchemy.sql.elements import ColumnClause
from sqlalchemy.sql.expression import Label, Select, TextAsFrom, TextClause
from sqlalchemy.types import TypeEngine
@@ -67,7 +68,7 @@ from superset.models.core import Database
from superset.models.helpers import AuditMixinNullable, QueryResult
from superset.result_set import SupersetResultSet
from superset.sql_parse import ParsedQuery
from superset.typing import Metric, QueryObjectDict
from superset.typing import AdhocMetric, Metric, OrderBy, QueryObjectDict
from superset.utils import core as utils
from superset.utils.core import GenericDataType
@@ -75,6 +76,8 @@ config = app.config
metadata = Model.metadata # pylint: disable=no-member
logger = logging.getLogger(__name__)
VIRTUAL_TABLE_ALIAS = "virtual_table"
class SqlaQuery(NamedTuple):
extra_cache_keys: List[Any]
@@ -378,7 +381,7 @@ class SqlMetric(Model, BaseMetric):
def get_sqla_col(self, label: Optional[str] = None) -> Column:
label = label or self.metric_name
sqla_col = literal_column(self.expression)
sqla_col: ColumnClause = literal_column(self.expression)
return self.table.make_sqla_column_compatible(sqla_col, label)
@property
@@ -505,10 +508,10 @@ class SqlaTable( # pylint: disable=too-many-public-methods,too-many-instance-at
"""
label_expected = label or sqla_col.name
db_engine_spec = self.database.db_engine_spec
if db_engine_spec.allows_column_aliases:
# add quotes to tables
if db_engine_spec.allows_alias_in_select:
label = db_engine_spec.make_label_compatible(label_expected)
sqla_col = sqla_col.label(label)
sqla_col._df_label_expected = label_expected # pylint: disable=protected-access
return sqla_col
def __repr__(self) -> str:
@@ -796,40 +799,53 @@ class SqlaTable( # pylint: disable=too-many-public-methods,too-many-instance-at
def get_from_clause(
self, template_processor: Optional[BaseTemplateProcessor] = None
) -> Union[table, TextAsFrom]:
# Supporting arbitrary SQL statements in place of tables
if self.sql:
from_sql = self.sql
if template_processor:
try:
from_sql = template_processor.process_template(from_sql)
except TemplateError as ex:
raise QueryObjectValidationError(
_(
"Error in jinja expression in FROM clause: %(msg)s",
msg=ex.message,
)
)
"""
Return where to select the columns and metrics from. Either a physical table
or a virtual table with it's own subquery.
"""
if not self.is_virtual:
return self.get_sqla_table()
from_sql = sqlparse.format(from_sql, strip_comments=True)
if len(sqlparse.split(from_sql)) > 1:
from_sql = self.get_rendered_sql(template_processor)
parsed_query = ParsedQuery(from_sql)
db_engine_spec = self.database.db_engine_spec
if not (
parsed_query.is_unknown() or db_engine_spec.is_readonly_query(parsed_query)
):
raise QueryObjectValidationError(
_("Virtual dataset query must be read-only")
)
return TextAsFrom(sa.text(from_sql), []).alias(VIRTUAL_TABLE_ALIAS)
def get_rendered_sql(
self, template_processor: Optional[BaseTemplateProcessor] = None
) -> str:
"""
Render sql with template engine (Jinja).
"""
sql = self.sql
if template_processor:
try:
sql = template_processor.process_template(sql)
except TemplateError as ex:
raise QueryObjectValidationError(
_("Virtual dataset query cannot consist of multiple statements")
_(
"Error while rendering virtual dataset query: %(msg)s",
msg=ex.message,
)
)
parsed_query = ParsedQuery(from_sql)
db_engine_spec = self.database.db_engine_spec
if not (
parsed_query.is_unknown()
or db_engine_spec.is_readonly_query(parsed_query)
):
raise QueryObjectValidationError(
_("Virtual dataset query must be read-only")
)
return TextAsFrom(sa.text(from_sql), []).alias("expr_qry")
return self.get_sqla_table()
sql = sqlparse.format(sql, strip_comments=True)
if not sql:
raise QueryObjectValidationError(_("Virtual dataset query cannot be empty"))
if len(sqlparse.split(sql)) > 1:
raise QueryObjectValidationError(
_("Virtual dataset query cannot consist of multiple statements")
)
return sql
def adhoc_metric_to_sqla(
self, metric: Dict[str, Any], columns_by_name: Dict[str, Any]
) -> Optional[Column]:
self, metric: AdhocMetric, columns_by_name: Dict[str, TableColumn]
) -> Column:
"""
Turn an adhoc metric into a sqlalchemy column.
@@ -843,7 +859,7 @@ class SqlaTable( # pylint: disable=too-many-public-methods,too-many-instance-at
if expression_type == utils.AdhocMetricExpressionType.SIMPLE:
column_name = metric["column"].get("column_name")
table_column = columns_by_name.get(column_name)
table_column: Optional[TableColumn] = columns_by_name.get(column_name)
if table_column:
sqla_column = table_column.get_sqla_col()
else:
@@ -852,7 +868,7 @@ class SqlaTable( # pylint: disable=too-many-public-methods,too-many-instance-at
elif expression_type == utils.AdhocMetricExpressionType.SQL:
sqla_metric = literal_column(metric.get("sqlExpression"))
else:
return None
raise QueryObjectValidationError("Adhoc metric expressionType is invalid")
return self.make_sqla_column_compatible(sqla_metric, label)
@@ -883,10 +899,10 @@ class SqlaTable( # pylint: disable=too-many-public-methods,too-many-instance-at
def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-many-branches,too-many-statements
self,
metrics: List[Metric],
granularity: str,
from_dttm: Optional[datetime],
to_dttm: Optional[datetime],
metrics: Optional[List[Metric]] = None,
granularity: Optional[str] = None,
from_dttm: Optional[datetime] = None,
to_dttm: Optional[datetime] = None,
columns: Optional[List[str]] = None,
groupby: Optional[List[str]] = None,
filter: Optional[ # pylint: disable=redefined-builtin
@@ -899,7 +915,7 @@ class SqlaTable( # pylint: disable=too-many-public-methods,too-many-instance-at
row_offset: Optional[int] = None,
inner_from_dttm: Optional[datetime] = None,
inner_to_dttm: Optional[datetime] = None,
orderby: Optional[List[Tuple[ColumnElement, bool]]] = None,
orderby: Optional[List[OrderBy]] = None,
extras: Optional[Dict[str, Any]] = None,
order_desc: bool = True,
is_rowcount: bool = False,
@@ -922,8 +938,10 @@ class SqlaTable( # pylint: disable=too-many-public-methods,too-many-instance-at
template_processor = self.get_template_processor(**template_kwargs)
db_engine_spec = self.database.db_engine_spec
prequeries: List[str] = []
orderby = orderby or []
extras = extras or {}
need_groupby = bool(metrics is not None or groupby)
metrics = metrics or []
# For backward compatibility
if granularity not in self.dttm_cols:
@@ -965,19 +983,47 @@ class SqlaTable( # pylint: disable=too-many-public-methods,too-many-instance-at
main_metric_expr, label = literal_column("COUNT(*)"), "ccount"
main_metric_expr = self.make_sqla_column_compatible(main_metric_expr, label)
select_exprs: List[Column] = []
groupby_exprs_sans_timestamp = OrderedDict()
# To ensure correct handling of the ORDER BY labeling we need to reference the
# metric instance if defined in the SELECT clause.
metrics_exprs_by_label = {
m.name: m for m in metrics_exprs # pylint: disable=protected-access
}
assert extras is not None
# Since orderby may use adhoc metrics, too; we need to process them first
orderby_exprs: List[ColumnElement] = []
for orig_col, ascending in orderby:
col: Union[Metric, ColumnElement] = orig_col
if isinstance(col, dict):
if utils.is_adhoc_metric(col):
# add adhoc sort by column to columns_by_name if not exists
col = self.adhoc_metric_to_sqla(col, columns_by_name)
need_groupby = True
elif col in columns_by_name:
col = columns_by_name[col].get_sqla_col()
elif col in metrics_by_name:
col = metrics_by_name[col].get_sqla_col()
need_groupby = True
elif col in metrics_exprs_by_label:
col = metrics_exprs_by_label[col]
if isinstance(col, ColumnElement):
orderby_exprs.append(col)
else:
# Could not convert a column reference to valid ColumnElement
raise QueryObjectValidationError(
_("Unknown column used in orderby: %(col)", col=orig_col)
)
select_exprs: List[Union[Column, Label]] = []
groupby_exprs_sans_timestamp = OrderedDict()
# filter out the pseudo column __timestamp from columns
columns = columns or []
columns = [col for col in columns if col != utils.DTTM_ALIAS]
if metrics or groupby:
if need_groupby:
# dedup columns while preserving order
columns = groupby or columns
select_exprs = []
for selected in columns:
# if groupby field/expr equals granularity field/expr
if selected == granularity:
@@ -1005,6 +1051,13 @@ class SqlaTable( # pylint: disable=too-many-public-methods,too-many-instance-at
groupby_exprs_with_timestamp = OrderedDict(groupby_exprs_sans_timestamp.items())
if granularity:
if granularity not in columns_by_name:
raise QueryObjectValidationError(
_(
'Time column "%(col)s" does not exist in dataset',
col=granularity,
)
)
dttm_col = columns_by_name[granularity]
time_grain = extras.get("time_grain_sqla")
time_filters = []
@@ -1031,12 +1084,7 @@ class SqlaTable( # pylint: disable=too-many-public-methods,too-many-instance-at
)
select_exprs += metrics_exprs
labels_expected = [
c._df_label_expected # pylint: disable=protected-access
for c in select_exprs
]
labels_expected = [c.name for c in select_exprs]
select_exprs = db_engine_spec.make_select_compatible(
groupby_exprs_with_timestamp.values(), select_exprs
)
@@ -1149,26 +1197,13 @@ class SqlaTable( # pylint: disable=too-many-public-methods,too-many-instance-at
qry = qry.where(and_(*where_clause_and))
qry = qry.having(and_(*having_clause_and))
# To ensure correct handling of the ORDER BY labeling we need to reference the
# metric instance if defined in the SELECT clause.
metrics_exprs_by_label = {
m._label: m for m in metrics_exprs # pylint: disable=protected-access
}
for col, ascending in orderby:
for col, (orig_col, ascending) in zip(orderby_exprs, orderby):
if (
db_engine_spec.allows_alias_in_orderby
and col.name in metrics_exprs_by_label
):
col = Label(col.name, metrics_exprs_by_label[col.name])
direction = asc if ascending else desc
if utils.is_adhoc_metric(col):
col = self.adhoc_metric_to_sqla(col, columns_by_name)
elif col in columns_by_name:
col = columns_by_name[col].get_sqla_col()
elif col in metrics_by_name:
col = metrics_by_name[col].get_sqla_col()
if isinstance(col, Label):
label = col._label # pylint: disable=protected-access
if label in metrics_exprs_by_label:
col = metrics_exprs_by_label[label]
qry = qry.order_by(direction(col))
if row_limit:
@@ -1182,7 +1217,7 @@ class SqlaTable( # pylint: disable=too-many-public-methods,too-many-instance-at
and not time_groupby_inline
and groupby
):
if self.database.db_engine_spec.allows_joins:
if db_engine_spec.allows_joins:
# some sql dialects require for order by expressions
# to also be in the select clause -- others, e.g. vertica,
# require a unique inner alias
@@ -1287,7 +1322,7 @@ class SqlaTable( # pylint: disable=too-many-public-methods,too-many-instance-at
timeseries_limit_metric: Metric,
metrics_by_name: Dict[str, SqlMetric],
columns_by_name: Dict[str, TableColumn],
) -> Optional[Column]:
) -> Column:
if utils.is_adhoc_metric(timeseries_limit_metric):
assert isinstance(timeseries_limit_metric, dict)
ob = self.adhoc_metric_to_sqla(timeseries_limit_metric, columns_by_name)
@@ -1300,7 +1335,6 @@ class SqlaTable( # pylint: disable=too-many-public-methods,too-many-instance-at
raise QueryObjectValidationError(
_("Metric '%(metric)s' does not exist", metric=timeseries_limit_metric)
)
return ob
def _get_top_groups( # pylint: disable=no-self-use
@@ -1326,27 +1360,33 @@ class SqlaTable( # pylint: disable=too-many-public-methods,too-many-instance-at
errors = None
error_message = None
def mutator(df: pd.DataFrame) -> None:
def assign_column_label(df: pd.DataFrame) -> Optional[pd.DataFrame]:
"""
Some engines change the case or generate bespoke column names, either by
default or due to lack of support for aliasing. This function ensures that
the column names in the DataFrame correspond to what is expected by
the viz components.
:param df: Original DataFrame returned by the engine
"""
Sometimes a query may also contain only order by columns that are not used
as metrics or groupby columns, but need to present in the SQL `select`,
filtering by `labels_expected` make sure we only return columns users want.
:param df: Original DataFrame returned by the engine
:return: Mutated DataFrame
"""
labels_expected = query_str_ext.labels_expected
if df is not None and not df.empty:
if len(df.columns) != len(labels_expected):
if len(df.columns) < len(labels_expected):
raise QueryObjectValidationError(
f"For {sql}, df.columns: {df.columns}"
f" differs from {labels_expected}"
_("Db engine did not return all queried columns")
)
if len(df.columns) > len(labels_expected):
df = df.iloc[:, 0 : len(labels_expected)]
df.columns = labels_expected
return df
try:
df = self.database.get_df(sql, self.schema, mutator)
df = self.database.get_df(sql, self.schema, mutator=assign_column_label)
except Exception as ex: # pylint: disable=broad-except
df = pd.DataFrame()
status = utils.QueryStatus.FAILED
superset/db_engine_specs/base.py
+2 -1
View File
@@ -232,7 +232,8 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods
time_secondary_columns = False
allows_joins = True
allows_subqueries = True
allows_column_aliases = True
allows_alias_in_select = True
allows_alias_in_orderby = True
allows_sql_comments = True
force_column_alias_quotes = False
arraysize = 0
superset/db_engine_specs/pinot.py
+2 -1
View File
@@ -26,7 +26,8 @@ class PinotEngineSpec(BaseEngineSpec): # pylint: disable=abstract-method
engine_name = "Apache Pinot"
allows_subqueries = False
allows_joins = False
allows_column_aliases = False
allows_alias_in_select = False
allows_alias_in_orderby = False
# Pinot does its own conversion below
_time_grain_expressions: Dict[Optional[str], str] = {
superset/models/core.py
+1 -1
View File
@@ -394,7 +394,7 @@ class Database(
)
df = result_set.to_pandas_df()
if mutator:
mutator(df)
df = mutator(df)
for col, coltype in df.dtypes.to_dict().items():
if coltype == numpy.object_ and needs_conversion(df[col]):
superset/typing.py
+3 -1
View File
@@ -30,7 +30,9 @@ FilterValue = Union[float, int, str]
FilterValues = Union[FilterValue, List[FilterValue], Tuple[FilterValue]]
FormData = Dict[str, Any]
Granularity = Union[str, Dict[str, Union[str, float]]]
Metric = Union[Dict[str, str], str]
AdhocMetric = Dict[str, Any]
Metric = Union[AdhocMetric, str]
OrderBy = Tuple[Metric, bool]
QueryObjectDict = Dict[str, Any]
VizData = Optional[Union[List[Any], Dict[Any, Any]]]
VizPayload = Dict[str, Any]
superset/utils/core.py
+1 -1
View File
@@ -1243,7 +1243,7 @@ def backend() -> str:
def is_adhoc_metric(metric: Metric) -> bool:
return isinstance(metric, dict)
return isinstance(metric, dict) and "expressionType" in metric
def get_metric_name(metric: Metric) -> str:
superset/utils/pandas_postprocessing.py
+2 -1
View File
@@ -15,6 +15,7 @@
# specific language governing permissions and limitations
# under the License.
import logging
from decimal import Decimal
from functools import partial
from typing import Any, Callable, Dict, List, Optional, Set, Tuple, Union
@@ -577,7 +578,7 @@ def contribution(
:return: DataFrame with contributions.
"""
contribution_df = df.copy()
numeric_df = contribution_df.select_dtypes(include="number")
numeric_df = contribution_df.select_dtypes(include=["number", Decimal])
# verify column selections
if columns:
numeric_columns = numeric_df.columns.tolist()
superset/viz.py
+4 -2
View File
@@ -519,7 +519,8 @@ class BaseViz:
+ (query_obj.get("groupby") or [])
+ utils.get_column_names_from_metrics(
cast(
List[Union[str, Dict[str, Any]]], query_obj.get("metrics"),
List[Union[str, Dict[str, Any]]],
query_obj.get("metrics") or [],
)
)
if col not in self.datasource.column_names
@@ -1666,7 +1667,8 @@ class HistogramViz(BaseViz):
self.columns = numeric_columns
d["columns"] = numeric_columns + self.groupby
# override groupby entry to avoid aggregation
d["groupby"] = []
d["groupby"] = None
d["metrics"] = None
return d
def labelify(self, keys: Union[List[str], str], column: str) -> str:
tests/fixtures/birth_names_dashboard.py
Vendored
+1 -1
View File
@@ -127,7 +127,7 @@ def _get_dataframe(database: Database) -> DataFrame:
data = _get_birth_names_data()
df = pd.DataFrame.from_dict(data)
if database.backend == "presto":
df.ds = df.ds.dt.strftime("%Y-%m-%d %H:%M%:%S")
df.ds = df.ds.dt.strftime("%Y-%m-%d %H:%M:%S")
return df
tests/model_tests.py
+1 -1
View File
@@ -421,7 +421,7 @@ class TestSqlaTableModel(SupersetTestCase):
tbl = self.get_table_by_name("birth_names")
query_obj = dict(
groupby=[],
metrics=[],
metrics=None,
filter=[],
is_timeseries=False,
columns=["name"],
tests/security_tests.py
+5 -12
View File
@@ -15,19 +15,17 @@
# specific language governing permissions and limitations
# under the License.
# isort:skip_file
import datetime
import inspect
import re
import unittest
from unittest.mock import Mock, patch
import pandas as pd
from unittest.mock import Mock, patch
from typing import Any, Dict
import prison
import pytest
import random
from flask import current_app, g
from sqlalchemy import Float, Date, String
from superset.models.dashboard import Dashboard
@@ -42,11 +40,6 @@ from superset.sql_parse import Table
from superset.utils.core import get_example_database
from .base_tests import SupersetTestCase
from .dashboard_utils import (
create_table_for_dashboard,
create_slice,
create_dashboard,
)
from tests.fixtures.birth_names_dashboard import load_birth_names_dashboard_with_slices
from tests.fixtures.energy_dashboard import load_energy_table_with_slice
from tests.fixtures.public_role import (
@@ -1037,9 +1030,9 @@ class TestRowLevelSecurity(SupersetTestCase):
"""
rls_entry = None
query_obj = dict(
query_obj: Dict[str, Any] = dict(
groupby=[],
metrics=[],
metrics=None,
filter=[],
is_timeseries=False,
columns=["value"],