sadpandajoe commented on code in PR #45793:
URL: https://github.com/apache/superset/pull/45793#discussion_r4399184040
##########
superset/db_engine_specs/clickhouse.py:
##########
@@ -72,6 +73,22 @@ class ClickHouseQueryStoppedBeforeDispatchError(Exception):
"""
+class AggregateFunctionState(types.UserDefinedType):
+ cache_ok = True
+
+ def get_col_spec(self, **kw: Any) -> str:
+ return "AggregateFunction"
+
+ def coerce_compared_value(self, op: Any, value: Any) ->
types.TypeEngine[Any]:
+ return types.NullType().coerce_compared_value(op, value)
+
+ def column_expression(self, colexpr: ColumnElement) -> ColumnElement:
+ element = colexpr.element if isinstance(colexpr, Label) else colexpr
+ if isinstance(element, ColumnClause) and not element.is_literal:
+ return func.finalizeAggregation(colexpr)
Review Comment:
The wrapper is applied while the statement compiles, after the
shadowing-alias rename has already looked at the plain `rtt` column. That
leaves `finalizeAggregation(rtt) AS rtt` in the SELECT list, and ClickHouse
resolves `rtt` in the other select expressions to that alias. For a dataset
like the one in the new calculated-column test, a Drill to detail that selects
both `rtt` and `rtt_p50` compiles to `SELECT finalizeAggregation(rtt) AS rtt,
(finalizeAggregation(rtt)) AS rtt_p50__`, so the second call receives the
already-finalized value and ClickHouse rejects it (and a merge metric like
`quantileTimingMerge(0.5)(rtt)` selected next to `rtt` would be fed the scalar
the same way). The new tests select only one of the two columns, so they don't
catch this. Should the finalized projection get a non-shadowing alias (or be
renamed the way `rename_shadowing_aliases` does), and could a case with both
columns selected be added?
--
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]
