This is an automated email from the ASF dual-hosted git repository. maximebeauchemin pushed a commit to branch fix_bool_snowflake in repository https://gitbox.apache.org/repos/asf/superset.git
commit 1610c6f298e88d03eb27125ec82102409c515914 Author: Maxime Beauchemin <maximebeauche...@gmail.com> AuthorDate: Wed Jul 16 17:43:04 2025 -0700 fix: proper handling of boolean filters with snowflake --- superset/db_engine_specs/base.py | 68 ++++++++++++++++++++++ superset/db_engine_specs/snowflake.py | 3 + superset/models/helpers.py | 42 +++++++------ tests/unit_tests/db_engine_specs/test_snowflake.py | 38 ++++++++++++ 4 files changed, 133 insertions(+), 18 deletions(-) diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py index e3ffdd335d..ec314b94c5 100644 --- a/superset/db_engine_specs/base.py +++ b/superset/db_engine_specs/base.py @@ -377,6 +377,10 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods disallow_uri_query_params: dict[str, set[str]] = {} # A Dict of query parameters that will always be used on every connection # by driver name + + # Whether to use equality operators (= true/false) instead of IS operators + # for boolean filters. Some databases like Snowflake don't support IS true/false + use_equality_for_boolean_filters = False enforce_uri_query_params: dict[str, dict[str, Any]] = {} force_column_alias_quotes = False @@ -1202,6 +1206,70 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods """ return None + @classmethod + def handle_boolean_filter(cls, sqla_col: Any, op: str, value: bool) -> Any: + """ + Handle boolean filter operations with engine-specific logic. + + By default, uses SQLAlchemy's IS operator (column IS true/false). + Engines that don't support IS for boolean values can override + use_equality_for_boolean_filters to use equality operators instead. + + :param sqla_col: SQLAlchemy column element + :param op: Filter operator (IS_TRUE or IS_FALSE) + :param value: Boolean value (True or False) + :return: SQLAlchemy expression for the boolean filter + """ + if cls.use_equality_for_boolean_filters: + return sqla_col == value + else: + return sqla_col.is_(value) + + @classmethod + def handle_null_filter(cls, sqla_col: Any, op: str) -> Any: + """ + Handle null/not null filter operations. + + :param sqla_col: SQLAlchemy column element + :param op: Filter operator (IS_NULL or IS_NOT_NULL) + :return: SQLAlchemy expression for the null filter + """ + from superset.utils import core as utils + + if op == utils.FilterOperator.IS_NULL.value: + return sqla_col.is_(None) + elif op == utils.FilterOperator.IS_NOT_NULL.value: + return sqla_col.isnot(None) + else: + raise ValueError(f"Invalid null filter operator: {op}") + + @classmethod + def handle_comparison_filter(cls, sqla_col: Any, op: str, value: Any) -> Any: + """ + Handle comparison filter operations (=, !=, >, <, >=, <=). + + :param sqla_col: SQLAlchemy column element + :param op: Filter operator + :param value: Filter value + :return: SQLAlchemy expression for the comparison filter + """ + from superset.utils import core as utils + + if op == utils.FilterOperator.EQUALS.value: + return sqla_col == value + elif op == utils.FilterOperator.NOT_EQUALS.value: + return sqla_col != value + elif op == utils.FilterOperator.GREATER_THAN.value: + return sqla_col > value + elif op == utils.FilterOperator.LESS_THAN.value: + return sqla_col < value + elif op == utils.FilterOperator.GREATER_THAN_OR_EQUALS.value: + return sqla_col >= value + elif op == utils.FilterOperator.LESS_THAN_OR_EQUALS.value: + return sqla_col <= value + else: + raise ValueError(f"Invalid comparison filter operator: {op}") + @classmethod def handle_cursor(cls, cursor: Any, query: Query) -> None: """Handle a live cursor between the execute and fetchall calls diff --git a/superset/db_engine_specs/snowflake.py b/superset/db_engine_specs/snowflake.py index 7e353f006d..5b064c22c7 100644 --- a/superset/db_engine_specs/snowflake.py +++ b/superset/db_engine_specs/snowflake.py @@ -83,6 +83,9 @@ class SnowflakeEngineSpec(PostgresBaseEngineSpec): force_column_alias_quotes = True max_column_name_length = 256 + # Snowflake doesn't support IS true/false syntax, use = true/false instead + use_equality_for_boolean_filters = True + parameters_schema = SnowflakeParametersSchema() default_driver = "snowflake" sqlalchemy_uri_placeholder = "snowflake://" diff --git a/superset/models/helpers.py b/superset/models/helpers.py index 930b84aa34..74a95c717a 100644 --- a/superset/models/helpers.py +++ b/superset/models/helpers.py @@ -1853,14 +1853,21 @@ class ExploreMixin: # pylint: disable=too-many-public-methods if op == utils.FilterOperator.NOT_IN.value: cond = ~cond where_clause_and.append(cond) - elif op == utils.FilterOperator.IS_NULL.value: - where_clause_and.append(sqla_col.is_(None)) - elif op == utils.FilterOperator.IS_NOT_NULL.value: - where_clause_and.append(sqla_col.isnot(None)) + elif op in { + utils.FilterOperator.IS_NULL.value, + utils.FilterOperator.IS_NOT_NULL.value, + }: + where_clause_and.append( + db_engine_spec.handle_null_filter(sqla_col, op) + ) elif op == utils.FilterOperator.IS_TRUE.value: - where_clause_and.append(sqla_col.is_(True)) + where_clause_and.append( + db_engine_spec.handle_boolean_filter(sqla_col, op, True) + ) elif op == utils.FilterOperator.IS_FALSE.value: - where_clause_and.append(sqla_col.is_(False)) + where_clause_and.append( + db_engine_spec.handle_boolean_filter(sqla_col, op, False) + ) else: if ( op @@ -1876,18 +1883,17 @@ class ExploreMixin: # pylint: disable=too-many-public-methods "with comparison operators" ) ) - if op == utils.FilterOperator.EQUALS.value: - where_clause_and.append(sqla_col == eq) - elif op == utils.FilterOperator.NOT_EQUALS.value: - where_clause_and.append(sqla_col != eq) - elif op == utils.FilterOperator.GREATER_THAN.value: - where_clause_and.append(sqla_col > eq) - elif op == utils.FilterOperator.LESS_THAN.value: - where_clause_and.append(sqla_col < eq) - elif op == utils.FilterOperator.GREATER_THAN_OR_EQUALS.value: - where_clause_and.append(sqla_col >= eq) - elif op == utils.FilterOperator.LESS_THAN_OR_EQUALS.value: - where_clause_and.append(sqla_col <= eq) + if op in { + utils.FilterOperator.EQUALS.value, + utils.FilterOperator.NOT_EQUALS.value, + utils.FilterOperator.GREATER_THAN.value, + utils.FilterOperator.LESS_THAN.value, + utils.FilterOperator.GREATER_THAN_OR_EQUALS.value, + utils.FilterOperator.LESS_THAN_OR_EQUALS.value, + }: + where_clause_and.append( + db_engine_spec.handle_comparison_filter(sqla_col, op, eq) + ) elif op in { utils.FilterOperator.ILIKE.value, utils.FilterOperator.LIKE.value, diff --git a/tests/unit_tests/db_engine_specs/test_snowflake.py b/tests/unit_tests/db_engine_specs/test_snowflake.py index 68c611c5c0..1b576a319f 100644 --- a/tests/unit_tests/db_engine_specs/test_snowflake.py +++ b/tests/unit_tests/db_engine_specs/test_snowflake.py @@ -352,6 +352,44 @@ def test_mask_encrypted_extra_no_fields() -> None: ) +def test_handle_boolean_filter() -> None: + """ + Test that Snowflake uses equality operators for boolean filters instead of IS. + """ + from sqlalchemy import Boolean, Column + + from superset.db_engine_specs.snowflake import SnowflakeEngineSpec + + # Create a mock SQLAlchemy column + bool_col = Column("test_col", Boolean) + + # Test IS_TRUE filter + result_true = SnowflakeEngineSpec.handle_boolean_filter(bool_col, "IS TRUE", True) + # The result should be a equality comparison, not an IS comparison + assert ( + str(result_true.compile(compile_kwargs={"literal_binds": True})) + == "test_col = true" + ) + + # Test IS_FALSE filter + result_false = SnowflakeEngineSpec.handle_boolean_filter( + bool_col, "IS FALSE", False + ) + assert ( + str(result_false.compile(compile_kwargs={"literal_binds": True})) + == "test_col = false" + ) + + +def test_use_equality_for_boolean_filters_property() -> None: + """ + Test that Snowflake has the use_equality_for_boolean_filters property set to True. + """ + from superset.db_engine_specs.snowflake import SnowflakeEngineSpec + + assert SnowflakeEngineSpec.use_equality_for_boolean_filters is True + + def test_unmask_encrypted_extra() -> None: """ Test that the private keys can be reused from the previous `encrypted_extra`.