aminghadersohi commented on code in PR #45681:
URL: https://github.com/apache/superset/pull/45681#discussion_r4382113715
##########
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:
Correction to my reply above: at 2723978ee47c6fe5d34c7db14b0b60997cf59f49
the adhoc-column fix holds, but the filter sibling is not fully aligned with
execution.
`_get_query_sql_expressions` resolves a verbose-name filter through
`filter_columns` (superset/models/helpers.py:4041), a dict that keeps the last
column with that verbose name. Execution uses the first one (`next(c for c in
self.columns if c.verbose_name == flt_col)`, superset/models/helpers.py:5690).
If two columns share a verbose name, the first being a saved calculated column
such as `(SELECT MAX(col1) FROM physical)` and the second a plain column, a
filter on that verbose name executes the subquery with each user's RLS on
`physical`, but the collector adds no predicate. Two users with different RLS
rules then share a cache key. I reproduced this at the head on SQLite: the
executed SQL contains each user's predicate, and `get_extra_cache_keys` returns
`[]` for both.
What checks out: the 18 `test_adhoc_saved_column_cache_keys_are_user_scoped`
cases (15 fail at dba4383d1c39acf2968a79c793a89b852978cc9d, all pass at the
head). Tab- and newline-padded references, BASE_AXIS and isColumnReference
columns, adhoc and label-referenced filters, label ORDER BY, SIMPLE metrics on
saved columns, saved-metric filters, series-limit metrics, granularity and
`__timestamp` filters all contribute the predicates that execution applies.
Not fixed: verbose-name filter resolution when several columns share a
verbose name. The collector takes the last match and execution takes the first.
--
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]
