Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions superset/models/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2899,6 +2899,17 @@ def validate_expression(
tp = self.get_template_processor()
processed_expression = self._process_expression_template(expression, tp)

# Apply the same parsing policy used for stored adhoc column and
# metric expressions (single statement, no set operations, and no
# sub-queries unless ALLOW_ADHOC_SUBQUERY is enabled), so expression
# validation follows one policy across the query pipeline. Imported
# locally to avoid a circular import with the connectors package.
from superset.connectors.sqla.models import validate_stored_expression

validate_stored_expression(
self.database, self.catalog, self.schema or "", processed_expression
)

# Build validation query
tbl, cte = self.get_from_clause(tp)
validation_query = self._build_validation_query(
Expand Down
42 changes: 37 additions & 5 deletions tests/unit_tests/models/test_validate_expression.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ def setup_method(self):
self.table.schema = "test_schema"
self.table.catalog = None
self.table.database = MagicMock()
self.table.database.backend = "sqlite"
self.table.database.db_engine_spec = MagicMock()
self.table.database.db_engine_spec.make_sqla_column_compatible = lambda x, _: x
self.table.columns = []
Expand Down Expand Up @@ -105,18 +106,17 @@ def test_validate_having_expression(self, mock_execute):

@patch("superset.connectors.sqla.models.SqlaTable._execute_validation_query")
def test_validate_invalid_expression(self, mock_execute):
"""Test validation of invalid SQL expressions"""
# Mock _execute_validation_query to raise an exception
mock_execute.side_effect = Exception("Invalid SQL syntax")

"""Unparseable SQL is rejected by the shared expression parser before the
validation query is built or executed."""
result = self.table.validate_expression(
expression="INVALID SQL HERE",
expression_type=SqlExpressionType.COLUMN,
)

assert result["valid"] is False
assert len(result["errors"]) == 1
assert "Invalid SQL syntax" in result["errors"][0]["message"]
assert result["errors"][0]["message"]
mock_execute.assert_not_called()

@patch("superset.connectors.sqla.models.SqlaTable._execute_validation_query")
def test_validate_having_with_non_aggregated_column(self, mock_execute):
Expand Down Expand Up @@ -152,6 +152,38 @@ def test_validate_empty_expression(self, mock_execute):
# The actual error message will come from the exception
assert "empty" in result["errors"][0]["message"].lower()

@patch("superset.models.helpers.is_feature_enabled", return_value=False)
Comment thread
sha174n marked this conversation as resolved.
@patch("superset.connectors.sqla.models.SqlaTable._execute_validation_query")
def test_validate_expression_rejects_subquery(
self, mock_execute: MagicMock, mock_ff: MagicMock
) -> None:
"""A sub-query expression is rejected by the same validate_adhoc_subquery
gate used for stored adhoc expressions, before any validation query is
built or run (with ALLOW_ADHOC_SUBQUERY off, the default). Locks in that
expression validation never executes the sub-query."""
result = self.table.validate_expression(
expression="(SELECT 1) IS NOT NULL OR 1 = 1",
expression_type=SqlExpressionType.WHERE,
)

assert result["valid"] is False
mock_execute.assert_not_called()

@patch("superset.models.helpers.is_feature_enabled", return_value=False)
@patch("superset.connectors.sqla.models.SqlaTable._execute_validation_query")
def test_validate_expression_rejects_set_operation(
self, mock_execute: MagicMock, mock_ff: MagicMock
) -> None:
"""A set-operation expression is rejected before the validation query is
built or run, matching the stored-adhoc-expression policy."""
result = self.table.validate_expression(
expression="1 UNION SELECT 1",
expression_type=SqlExpressionType.WHERE,
)

assert result["valid"] is False
mock_execute.assert_not_called()

@patch("superset.connectors.sqla.models.SqlaTable._execute_validation_query")
def test_validate_expression_with_rls(self, mock_execute):
"""Test that RLS filters are applied during validation"""
Expand Down
Loading