diff --git a/superset-frontend/src/explore/components/controls/CollectionControl/index.tsx b/superset-frontend/src/explore/components/controls/CollectionControl/index.tsx index 0ad904731fa..369642f4142 100644 --- a/superset-frontend/src/explore/components/controls/CollectionControl/index.tsx +++ b/superset-frontend/src/explore/components/controls/CollectionControl/index.tsx @@ -188,7 +188,9 @@ function CollectionControl({ // Two items can collide when keyAccessor returns falsy and the index // fallback is used — breaking dnd-kit reordering and React reconciliation. // Assign a stable nanoid per item ref when no key is available. - const generatedIdsRef = useRef>(new WeakMap()); + const generatedIdsRef = useRef>( + new WeakMap(), + ); const itemIds = useMemo( () => value.map(item => { diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py index 78fc2e78afa..a98c87dd227 100644 --- a/superset/db_engine_specs/base.py +++ b/superset/db_engine_specs/base.py @@ -557,13 +557,11 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods # if True, database will be listed as option in the upload file form supports_file_upload = True - # Optional override for the RLS method used by ``get_rls_method``. When set, - # the engine spec opts into a specific strategy regardless of the - # ``allows_subqueries`` / ``allows_alias_in_select`` defaults. Use - # ``RLSMethod.AS_PREDICATE_SPLICE`` for engines whose sqlglot dialect can - # parse but not faithfully regenerate the SQL — splice mode rewrites the - # original query string instead of round-tripping through the generator. - rls_method: RLSMethod | None = None + # RLS strategy for this engine spec. Override in engine-specific classes as + # needed (for example ``RLSMethod.AS_PREDICATE`` for engines that don't + # support subquery-based RLS, or ``RLSMethod.AS_PREDICATE_SPLICE`` for + # engines where sqlglot generation is not faithful). + rls_method = RLSMethod.AS_SUBQUERY # Is the DB engine spec able to change the default schema? This requires implementing # noqa: E501 # a custom `adjust_engine_params` method. @@ -626,29 +624,6 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods else cls.encrypted_extra_sensitive_fields ) - @classmethod - def get_rls_method(cls) -> RLSMethod: - """ - Returns the RLS method to be used for this engine. - - There are three ways to insert RLS: replacing the table with a subquery - that has the RLS (safest, but not supported in all databases), appending - the RLS to the ``WHERE`` clause via AST transformation, or splicing the - RLS into the original SQL string (preserves dialect-specific syntax that - the sqlglot generator would otherwise transpile). - - Engine specs can opt into a specific strategy by setting the class-level - ``rls_method`` attribute; otherwise the choice falls back to subquery - when supported, and predicate otherwise. - """ - if cls.rls_method is not None: - return cls.rls_method - return ( - RLSMethod.AS_SUBQUERY - if cls.allows_subqueries and cls.allows_alias_in_select - else RLSMethod.AS_PREDICATE - ) - @classmethod def is_oauth2_enabled(cls) -> bool: return ( diff --git a/superset/db_engine_specs/couchbase.py b/superset/db_engine_specs/couchbase.py index 9f4b4b82256..8e238906390 100644 --- a/superset/db_engine_specs/couchbase.py +++ b/superset/db_engine_specs/couchbase.py @@ -35,6 +35,7 @@ from superset.db_engine_specs.base import ( DatabaseCategory, ) from superset.errors import ErrorLevel, SupersetError, SupersetErrorType +from superset.sql.parse import RLSMethod from superset.utils.network import is_hostname_valid, is_port_open @@ -82,6 +83,7 @@ class CouchbaseEngineSpec(BasicParametersMixin, BaseEngineSpec): default_driver = "couchbase" allows_joins = False allows_subqueries = False + rls_method = RLSMethod.AS_PREDICATE sqlalchemy_uri_placeholder = ( "couchbase://user:password@host[:port]?truststorepath=value?ssl=value" ) diff --git a/superset/db_engine_specs/pinot.py b/superset/db_engine_specs/pinot.py index 7942df79efe..b5b671c3451 100644 --- a/superset/db_engine_specs/pinot.py +++ b/superset/db_engine_specs/pinot.py @@ -20,6 +20,7 @@ from sqlalchemy.types import TypeEngine from superset.constants import TimeGrain from superset.db_engine_specs.base import BaseEngineSpec, DatabaseCategory +from superset.sql.parse import RLSMethod class PinotEngineSpec(BaseEngineSpec): @@ -30,6 +31,7 @@ class PinotEngineSpec(BaseEngineSpec): allows_joins = False allows_alias_in_select = False allows_alias_in_orderby = False + rls_method = RLSMethod.AS_PREDICATE # pinotdb only sets cursor.description when the response contains # columnDataTypes, which Pinot omits for zero-row results. diff --git a/superset/db_engine_specs/solr.py b/superset/db_engine_specs/solr.py index 03dca6c2b66..4f08e94ecac 100644 --- a/superset/db_engine_specs/solr.py +++ b/superset/db_engine_specs/solr.py @@ -16,6 +16,7 @@ # under the License. from superset.db_engine_specs.base import BaseEngineSpec, DatabaseCategory +from superset.sql.parse import RLSMethod class SolrEngineSpec(BaseEngineSpec): # pylint: disable=abstract-method @@ -27,6 +28,7 @@ class SolrEngineSpec(BaseEngineSpec): # pylint: disable=abstract-method time_groupby_inline = False allows_joins = False allows_subqueries = False + rls_method = RLSMethod.AS_PREDICATE metadata = { "description": "Apache Solr is an open-source enterprise search platform.", diff --git a/superset/utils/rls.py b/superset/utils/rls.py index 456b589e365..e7b989f349a 100644 --- a/superset/utils/rls.py +++ b/superset/utils/rls.py @@ -46,7 +46,7 @@ def apply_rls( # - append the RLS to the ``WHERE`` clause via AST transformation # - splice the RLS into the original SQL string (preserves dialect-specific # syntax that the sqlglot generator would otherwise transpile) - method = database.db_engine_spec.get_rls_method() + method = database.db_engine_spec.rls_method # In splice mode predicates stay as raw SQL strings and are inserted verbatim # into the source query — re-parsing them would force a generator round-trip diff --git a/tests/unit_tests/db_engine_specs/test_base.py b/tests/unit_tests/db_engine_specs/test_base.py index 1b58809b3b2..5d64d9f283d 100644 --- a/tests/unit_tests/db_engine_specs/test_base.py +++ b/tests/unit_tests/db_engine_specs/test_base.py @@ -1285,42 +1285,6 @@ def test_start_oauth2_dance_falls_back_to_url_for(mocker: MockerFixture) -> None assert error.extra["redirect_uri"] == fallback_uri -def test_get_rls_method_default_subquery() -> None: - """ - By default, an engine that supports subqueries and aliases-in-select - uses the safer subquery RLS strategy. - """ - - class _Spec(BaseEngineSpec): - allows_subqueries = True - allows_alias_in_select = True - - assert _Spec.get_rls_method() == RLSMethod.AS_SUBQUERY - - -def test_get_rls_method_default_predicate_when_no_subqueries() -> None: - """ - Engines without subquery / alias-in-select support fall back to the - AST predicate strategy. - """ - - class _Spec(BaseEngineSpec): - allows_subqueries = False - allows_alias_in_select = True - - assert _Spec.get_rls_method() == RLSMethod.AS_PREDICATE - - -def test_get_rls_method_class_attribute_override() -> None: - """ - Setting ``rls_method`` on an engine spec opts the engine into a specific - strategy regardless of the subquery/alias defaults — used by engines whose - sqlglot dialect can parse but not faithfully regenerate SQL. - """ - - class _SpliceSpec(BaseEngineSpec): - allows_subqueries = True - allows_alias_in_select = True - rls_method = RLSMethod.AS_PREDICATE_SPLICE - - assert _SpliceSpec.get_rls_method() == RLSMethod.AS_PREDICATE_SPLICE +def test_default_rls_method_is_subquery() -> None: + """Base engine spec defaults to subquery-based RLS.""" + assert BaseEngineSpec.rls_method == RLSMethod.AS_SUBQUERY diff --git a/tests/unit_tests/sql/parse_tests.py b/tests/unit_tests/sql/parse_tests.py index 79a9cec6953..a8de0dcbe39 100644 --- a/tests/unit_tests/sql/parse_tests.py +++ b/tests/unit_tests/sql/parse_tests.py @@ -2754,14 +2754,17 @@ def test_rls_predicate_splice_inserts_before_comments(sql: str, expected: str) - "sql, engine, expected", [ ( - "SELECT * FROM some_table QUALIFY row_number() OVER (PARTITION BY id ORDER BY ts DESC) = 1", + "SELECT * FROM some_table QUALIFY row_number() OVER " + "(PARTITION BY id ORDER BY ts DESC) = 1", "snowflake", - "SELECT * FROM some_table WHERE tenant_id = 42 QUALIFY row_number() OVER (PARTITION BY id ORDER BY ts DESC) = 1", + "SELECT * FROM some_table WHERE tenant_id = 42 " + "QUALIFY row_number() OVER (PARTITION BY id ORDER BY ts DESC) = 1", ), ( "SELECT sum(v) OVER () FROM some_table WINDOW w AS (PARTITION BY id)", "postgresql", - "SELECT sum(v) OVER () FROM some_table WHERE tenant_id = 42 WINDOW w AS (PARTITION BY id)", + "SELECT sum(v) OVER () FROM some_table WHERE tenant_id = 42 " + "WINDOW w AS (PARTITION BY id)", ), ], )