This is an automated email from the ASF dual-hosted git repository.
johnbodley pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push:
new 2b3e7fe [sqla] Adding check for invalid filter columns (#7888)
2b3e7fe is described below
commit 2b3e7fe4de816d11444fafce3c40a0cc13d0944a
Author: John Bodley <[email protected]>
AuthorDate: Thu Jul 18 08:51:41 2019 -0700
[sqla] Adding check for invalid filter columns (#7888)
---
superset/connectors/sqla/models.py | 79 ++++++++++++++++++++------------------
tests/model_tests.py | 42 ++++++++++++++++++++
2 files changed, 83 insertions(+), 38 deletions(-)
diff --git a/superset/connectors/sqla/models.py
b/superset/connectors/sqla/models.py
index a3f5690..c9eed47 100644
--- a/superset/connectors/sqla/models.py
+++ b/superset/connectors/sqla/models.py
@@ -649,7 +649,7 @@ class SqlaTable(Model, BaseDatasource):
elif m in metrics_dict:
metrics_exprs.append(metrics_dict.get(m).get_sqla_col())
else:
- raise Exception(_("Metric '{}' is not valid".format(m)))
+ raise Exception(_("Metric '%(metric)s' does not exist",
metric=m))
if metrics_exprs:
main_metric_expr = metrics_exprs[0]
else:
@@ -721,43 +721,46 @@ class SqlaTable(Model, BaseDatasource):
if not all([flt.get(s) for s in ["col", "op"]]):
continue
col = flt["col"]
+
+ if col not in cols:
+ raise Exception(_("Column '%(column)s' does not exist",
column=col))
+
op = flt["op"]
- col_obj = cols.get(col)
- if col_obj:
- is_list_target = op in ("in", "not in")
- eq = self.filter_values_handler(
- flt.get("val"),
- target_column_is_numeric=col_obj.is_num,
- is_list_target=is_list_target,
- )
- if op in ("in", "not in"):
- cond = col_obj.get_sqla_col().in_(eq)
- if "<NULL>" in eq:
- cond = or_(cond, col_obj.get_sqla_col() == None) #
noqa
- if op == "not in":
- cond = ~cond
- where_clause_and.append(cond)
- else:
- if col_obj.is_num:
- eq = utils.string_to_num(flt["val"])
- if op == "==":
- where_clause_and.append(col_obj.get_sqla_col() == eq)
- elif op == "!=":
- where_clause_and.append(col_obj.get_sqla_col() != eq)
- elif op == ">":
- where_clause_and.append(col_obj.get_sqla_col() > eq)
- elif op == "<":
- where_clause_and.append(col_obj.get_sqla_col() < eq)
- elif op == ">=":
- where_clause_and.append(col_obj.get_sqla_col() >= eq)
- elif op == "<=":
- where_clause_and.append(col_obj.get_sqla_col() <= eq)
- elif op == "LIKE":
-
where_clause_and.append(col_obj.get_sqla_col().like(eq))
- elif op == "IS NULL":
- where_clause_and.append(col_obj.get_sqla_col() ==
None) # noqa
- elif op == "IS NOT NULL":
- where_clause_and.append(col_obj.get_sqla_col() !=
None) # noqa
+ col_obj = cols[col]
+ is_list_target = op in ("in", "not in")
+ eq = self.filter_values_handler(
+ flt.get("val"),
+ target_column_is_numeric=col_obj.is_num,
+ is_list_target=is_list_target,
+ )
+ if op in ("in", "not in"):
+ cond = col_obj.get_sqla_col().in_(eq)
+ if "<NULL>" in eq:
+ cond = or_(cond, col_obj.get_sqla_col() == None) # noqa
+ if op == "not in":
+ cond = ~cond
+ where_clause_and.append(cond)
+ else:
+ if col_obj.is_num:
+ eq = utils.string_to_num(flt["val"])
+ if op == "==":
+ where_clause_and.append(col_obj.get_sqla_col() == eq)
+ elif op == "!=":
+ where_clause_and.append(col_obj.get_sqla_col() != eq)
+ elif op == ">":
+ where_clause_and.append(col_obj.get_sqla_col() > eq)
+ elif op == "<":
+ where_clause_and.append(col_obj.get_sqla_col() < eq)
+ elif op == ">=":
+ where_clause_and.append(col_obj.get_sqla_col() >= eq)
+ elif op == "<=":
+ where_clause_and.append(col_obj.get_sqla_col() <= eq)
+ elif op == "LIKE":
+ where_clause_and.append(col_obj.get_sqla_col().like(eq))
+ elif op == "IS NULL":
+ where_clause_and.append(col_obj.get_sqla_col() == None) #
noqa
+ elif op == "IS NOT NULL":
+ where_clause_and.append(col_obj.get_sqla_col() != None) #
noqa
if extras:
where = extras.get("where")
if where:
@@ -877,7 +880,7 @@ class SqlaTable(Model, BaseDatasource):
ob = timeseries_limit_metric.get_sqla_col()
else:
raise Exception(
- _("Metric '{}' is not valid".format(timeseries_limit_metric))
+ _("Metric '%(metric)s' does not exist",
metric=timeseries_limit_metric)
)
return ob
diff --git a/tests/model_tests.py b/tests/model_tests.py
index fda941b..febf198 100644
--- a/tests/model_tests.py
+++ b/tests/model_tests.py
@@ -272,3 +272,45 @@ class SqlaTableModelTestCase(SupersetTestCase):
self.assertIn("--COMMENT", sql)
app.config["SQL_QUERY_MUTATOR"] = None
+
+ def test_query_with_non_existent_metrics(self):
+ tbl = self.get_table_by_name("birth_names")
+
+ query_obj = dict(
+ groupby=[],
+ metrics=["invalid"],
+ filter=[],
+ is_timeseries=False,
+ columns=["name"],
+ granularity=None,
+ from_dttm=None,
+ to_dttm=None,
+ is_prequery=False,
+ extras={},
+ )
+
+ with self.assertRaises(Exception) as context:
+ tbl.get_query_str(query_obj)
+
+ self.assertTrue("Metric 'invalid' does not exist", context.exception)
+
+ def test_query_with_non_existent_filter_columns(self):
+ tbl = self.get_table_by_name("birth_names")
+
+ query_obj = dict(
+ groupby=[],
+ metrics=["count"],
+ filter=[{"col": "invalid", "op": "==", "val": "male"}],
+ is_timeseries=False,
+ columns=["name"],
+ granularity=None,
+ from_dttm=None,
+ to_dttm=None,
+ is_prequery=False,
+ extras={},
+ )
+
+ with self.assertRaises(Exception) as context:
+ tbl.get_query_str(query_obj)
+
+ self.assertTrue("Column 'invalid' does not exist", context.exception)
