aminghadersohi commented on code in PR #45681:
URL: https://github.com/apache/superset/pull/45681#discussion_r4382219941
##########
superset/models/helpers.py:
##########
@@ -3994,6 +4019,156 @@ def get_rendered_sql(
def text(self, clause: str) -> TextClause:
return self.db_engine_spec.get_text_clause(clause)
+ def _get_rls_schema(self) -> str:
+ """Resolve RLS tables with the same schema policy as SQL
expressions."""
+ return get_rls_schema(
+ self.database,
+ self.catalog,
+ self.schema,
+ self.sql or "",
+ self.template_params_dict,
+ )
+
+ def _get_query_sql_expressions(
+ self, query_obj: QueryObjectDict, template_processor:
BaseTemplateProcessor
+ ) -> list[str]:
+ """Find SQL-bearing chart fields without building a chart or its
prequeries."""
+ # Real column names take precedence over verbose names in filter
lookup.
+ columns = {col.verbose_name: col.expression for col in self.columns}
+ columns.update({col.column_name: col.expression for col in
self.columns})
+ metrics = {metric.metric_name: metric.expression for metric in
self.metrics}
+ expressions: list[str] = []
+
+ def add(value: Any, metric: bool = False) -> None:
+ """Resolve stored references in the namespace used by the query
sink."""
+ if isinstance(value, str):
+ references = metrics if metric else columns
+ expressions.append(references.get(value) or value)
+ elif isinstance(value, dict):
+ if sql := value.get("sqlExpression"):
+ sql = template_processor.process_template(sql)
+ # Adhoc columns can name a saved calculated column after
+ # rendering. SQL metrics use their SQL literally instead.
+ expressions.append(
+ sql if "expressionType" in value else columns.get(sql)
or sql
Review Comment:
I missed the duplicate verbose-name case in my earlier reply. Fixed in
dc56dcc7f9d2a66b952e2c552be45331ca92d39b: filter execution and the RLS
cache-key collector share one lookup helper, preferring a real column name,
then the first matching verbose name.
Regression test:
`tests/unit_tests/connectors/sqla/virtual_dataset_connections_test.py::test_duplicate_verbose_filter_cache_keys_are_user_scoped`
runs two users with different RLS rules against SQLite and checks actual
results, full cache keys and exact predicates. With the incoming-head helper: 2
failed, 2 passed; after: all 4 passed. This includes the protected-first
collision, reversed order and real-name precedence controls. Docker validation:
3,010 tests passed; the 79-case connection/regression subset passed, retaining
0 connections for known-schema cache hits, 1 for inspector discovery and 0
series prequeries. Full branch-file pre-commit, including mypy, passed.
Siblings checked:
- superset/models/helpers.py:4032 — fixed in
dc56dcc7f9d2a66b952e2c552be45331ca92d39b
- superset/models/helpers.py:4100 — fixed in
dc56dcc7f9d2a66b952e2c552be45331ca92d39b
- superset/models/helpers.py:5692 — fixed in
dc56dcc7f9d2a66b952e2c552be45331ca92d39b
- superset/models/helpers.py:4051 — not affected: columns/GROUP BY use real
names only, with GROUP BY overriding columns; both maps have identical
precedence. Saved real column names are unique per table, unlike verbose names.
- superset/connectors/sqla/models.py:2402 — not affected: adhoc columns and
x-axis references render and strip before real-name lookup, without
verbose-name fallback; the 18 two-user saved-column cases pass.
- superset/connectors/sqla/models.py:2286 — not affected: SQL metrics remain
literal; SIMPLE metrics resolve their nested column_name, while saved metrics
use metric_name, not verbose_name. Namespace tests pass.
- superset/models/helpers.py:5331 — not affected: ORDER BY uses selected
metric labels, then adhoc labels, saved metric names and real column names;
selected expressions are already collected. Shadowed-reference and label tests
pass.
- superset/models/helpers.py:5700 — not affected: filter saved-metric and
adhoc-label fallbacks follow the shared column lookup; inline adhoc filters and
__timestamp retain their existing paths. Sibling-field and alias tests pass.
- superset/models/helpers.py:4168 — not affected: granularity and optional
primary-time filtering use the same effective real-name temporal columns as
execution; both time-column test groups pass.
- superset/connectors/sqla/models.py:2542 — not affected: active
series-limit metrics use the metric namespace; inactive-series and no-prequery
tests pass.
- superset/models/sql_lab.py:425 — not affected: query-backed metadata has
no saved expressions or verbose labels and uses the same expression collector;
Query cases pass.
- superset/common/query_context_processor.py:461 — not affected: chart API
and async consumers delegate to the same datasource collector, with no separate
name lookup.
- superset/semantic_layers/models.py:886 — not affected: separate cache
implementation that does not inherit ExploreMixin.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
