paul-rogers commented on code in PR #13173:
URL: https://github.com/apache/druid/pull/13173#discussion_r991528258
##########
sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidOuterQueryRel.java:
##########
@@ -116,12 +114,20 @@ public DruidQuery toDruidQuery(final boolean
finalizeAggregations)
@Override
public DruidQuery toDruidQueryForExplaining()
{
+ final RowSignature sourceRowSignature;
+ if (sourceRel instanceof DruidRel) {
+ final DruidQuery subQuery = ((DruidRel)
sourceRel).toDruidQueryForExplaining();
+ sourceRowSignature = subQuery.getOutputRowSignature();
+ } else {
+ // fallback for cases other than group by inner query with order by with
alias on __time column
+ sourceRowSignature = RowSignatures.fromRelDataType(
+ sourceRel.getRowType().getFieldNames(),
+ sourceRel.getRowType()
Review Comment:
If aliasing is used, Calcite should ensure that the signature of the inner
`SELECT` provides the alias names. If not, we broke something since this is
pretty basic SQL functionality.
If, on the other hand, we want the column name `__time`, even if the user
said `__time AS t`, then we're using non-standard SQL behavior. We can unpack
the alias to work back to the base column if we need to know that this is the
`__time` column. However, we should not need to: the outer query should not, in
general, special-case `__time`. Our implementation might, given that native
queries are focused on the event use case.
Suppose we have to know that `t` is really `__time`. The proper thing to do
is to check if the column is an alias. If so, get the base column as above. It
may also be possible to attach traits to the column. Columns an have traits
such as their cardinality, etc. We might be able to add a trait saying the
column kind (`__time`, dimension, measure or whatever.)
##########
sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidOuterQueryRel.java:
##########
@@ -116,12 +114,20 @@ public DruidQuery toDruidQuery(final boolean
finalizeAggregations)
@Override
public DruidQuery toDruidQueryForExplaining()
{
+ final RowSignature sourceRowSignature;
+ if (sourceRel instanceof DruidRel) {
+ final DruidQuery subQuery = ((DruidRel)
sourceRel).toDruidQueryForExplaining();
+ sourceRowSignature = subQuery.getOutputRowSignature();
+ } else {
+ // fallback for cases other than group by inner query with order by with
alias on __time column
+ sourceRowSignature = RowSignatures.fromRelDataType(
+ sourceRel.getRowType().getFieldNames(),
+ sourceRel.getRowType()
Review Comment:
Another concern. If we return as the row signature the signature of the
source, we may miss computed columns:
```sql
SELECT * FROM
SELECT __time AS t, myCol as c, a + b AS d FROM foo
```
Here, the signature of the inner query is `(t TIMESTAMP, c VARCHAR, d
BIGINT)`, say. If we get the source signature (based on `foo`), we would omit
the computed column `d`, which is likely to cause cascading issues.
Let's be sure to test this case to validate the workaround.
--
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]
