[ 
https://issues.apache.org/jira/browse/CALCITE-1800?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16020677#comment-16020677
 ] 

MinJi Kim commented on CALCITE-1800:
------------------------------------

It looks like my previous suggestion is not going to work.  In 
RelToSqlConverterTest.testSelectQueryWithGroupByHaving3(), we have the 
following query plan:

{code}
LogicalProject(product_id=[$0], EXPR$1=[$1])
  LogicalFilter(condition=[>($0, 100)])
    LogicalProject(product_id=[$0], EXPR$1=[$1])
      LogicalFilter(condition=[>($2, 1)])
        LogicalAggregate(group=[{0}], EXPR$1=[MIN($1)], agg#1=[COUNT()])
          LogicalProject(product_id=[$1], store_id=[$19])
            LogicalJoin(condition=[=($1, $15)], joinType=[inner])
              LogicalTableScan(table=[[foodmart, product]])
              LogicalTableScan(table=[[foodmart, sales_fact_1997]])
{code}

The subtree below "LogicalFilter(condition=[>($0, 100)])" generates the 
following sql query, which I believe is correct.  Now, with the additional 
filter, "LogicalFilter(condition=[>($0, 100)])", we need to make sure that a 
new AliasContext is generated.

{code}
SELECT `product`.`product_id`, MIN(`sales_fact_1997`.`store_id`)
FROM `foodmart`.`product`
INNER JOIN `foodmart`.`sales_fact_1997` ON `product`.`product_id` = 
`sales_fact_1997`.`product_id`
GROUP BY `product`.`product_id`
HAVING COUNT(*) > 1
{code}

In this case, selectList is null since this is a filter, and we fall through 
and never generate a new AliasContext if we use my previously proposed fix.  
So, actually we do need the if-else-clause as before.  I do think that we need 
to be a bit more specific as to when we enter the if-else-clause.  We not only 
need check that we used the new alias (i.e. needNew is true and neededAlias != 
null), but also check that our aliases is of size 1.  Since we could have 
aliases = [<t1, rowType1>, <t2, rowType2>], with neededAlias = t1 (although I 
believe this should not happen in our code currently, but I think if our 
aliases context has two elements, "t1" and "t2", and we want to re-name the 
entire subquery as "t1", that should be a new AliasContext with a single 
element).

{code}
      } else {
        boolean qualified =
            !dialect.hasImplicitTableAlias() || aliases.size() > 1;
        // basically, we did a subSelect() since needNew is set and neededAlias 
is not null
        // now, we need to make sure that we need to update the alias context.
        // if our aliases map has a single element:  <neededAlias, rowType>,
        // then we don't need to rewrite the alias but otherwise, it should be 
updated.
        if (needNew  // if we have renamed
            && neededAlias != null  // can't rename to empty
            && aliases.size() != 1  // Even if "t1" is in aliases, if size is > 
1 then we should rename.
            && !aliases.containsKey(neededAlias)) {
          newContext = aliasContext(ImmutableMap.of(neededAlias, 
rel.getRowType()), qualified);
        } else {
          newContext = aliasContext(aliases, qualified);
        }
      }
      return new Builder(rel, clauseList, select, newContext, aliases);  // 
<--- I don't recall why I added the needNew check here, and I don't think it is 
necessary.
    }
{code}

[~julianhyde] and [~victor.ba...@gmail.com] does this sound reasonable? With 
above change, all tests seem to pass too.

> JDBC adapter fails to SELECT FROM a UNION query 
> ------------------------------------------------
>
>                 Key: CALCITE-1800
>                 URL: https://issues.apache.org/jira/browse/CALCITE-1800
>             Project: Calcite
>          Issue Type: Bug
>          Components: core
>    Affects Versions: 1.9.0
>            Reporter: Viktor Batytskyi
>            Assignee: Julian Hyde
>             Fix For: 1.13.0
>
>   Original Estimate: 2h
>  Remaining Estimate: 2h
>
> Relational representation of the following query fails to be converter to sql 
> string in Calcite:
> {code}
> select sum(case when "product_id"=0 then "net_weight" else 0 end) as 
> net_weight
> from ( select "product_id", "net_weight" from "product"  
> union all
> select "product_id", 0 as "net_weight" from "sales_fact_1997" ) t0
> {code}
> The conversion fails with:
> {code}
> java.lang.RuntimeException: While invoking method 'public 
> org.apache.calcite.rel.rel2sql.SqlImplementor$Result 
> org.apache.calcite.rel.rel2sql.RelToSqlConverter.visit(org.apache.calcite.rel.core.Aggregate)'
>       at org.apache.calcite.util.ReflectUtil$2.invoke(ReflectUtil.java:528)
>       at 
> org.apache.calcite.rel.rel2sql.RelToSqlConverter.dispatch(RelToSqlConverter.java:96)
>       at 
> org.apache.calcite.rel.rel2sql.RelToSqlConverter.visitChild(RelToSqlConverter.java:100)
>       at 
> org.apache.calcite.rel.rel2sql.RelToSqlConverterTest$Sql.ok(RelToSqlConverterTest.java:1559)
>       at 
> org.apache.calcite.rel.rel2sql.RelToSqlConverterTest.testUnionWrappedInASelect(RelToSqlConverterTest.java:654)
>       at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>       at 
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>       at 
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>       at java.lang.reflect.Method.invoke(Method.java:498)
>       at 
> org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
>       at 
> org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
>       at 
> org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
>       at 
> org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
>       at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
>       at 
> org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
>       at 
> org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
>       at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
>       at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
>       at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
>       at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
>       at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
>       at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
>       at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
>       at 
> com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:117)
>       at 
> com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:42)
>       at 
> com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:262)
>       at 
> com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:84)
>       at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>       at 
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>       at 
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>       at java.lang.reflect.Method.invoke(Method.java:498)
>       at com.intellij.rt.execution.application.AppMain.main(AppMain.java:147)
> Caused by: java.lang.reflect.InvocationTargetException
>       at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>       at 
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>       at 
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>       at java.lang.reflect.Method.invoke(Method.java:498)
>       at org.apache.calcite.util.ReflectUtil$2.invoke(ReflectUtil.java:525)
>       ... 31 more
> Caused by: java.lang.RuntimeException: While invoking method 'public 
> org.apache.calcite.rel.rel2sql.SqlImplementor$Result 
> org.apache.calcite.rel.rel2sql.RelToSqlConverter.visit(org.apache.calcite.rel.core.Project)'
>       at org.apache.calcite.util.ReflectUtil$2.invoke(ReflectUtil.java:528)
>       at 
> org.apache.calcite.rel.rel2sql.RelToSqlConverter.dispatch(RelToSqlConverter.java:96)
>       at 
> org.apache.calcite.rel.rel2sql.RelToSqlConverter.visitChild(RelToSqlConverter.java:100)
>       at 
> org.apache.calcite.rel.rel2sql.RelToSqlConverter.visit(RelToSqlConverter.java:179)
>       ... 36 more
> Caused by: java.lang.reflect.InvocationTargetException
>       at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>       at 
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>       at 
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>       at java.lang.reflect.Method.invoke(Method.java:498)
>       at org.apache.calcite.util.ReflectUtil$2.invoke(ReflectUtil.java:525)
>       ... 39 more
> Caused by: java.lang.AssertionError: field ordinal 0 out of range 
> {t1=RecordType(DOUBLE $f0)}
>       at 
> org.apache.calcite.rel.rel2sql.SqlImplementor$AliasContext.field(SqlImplementor.java:889)
>       at 
> org.apache.calcite.rel.rel2sql.SqlImplementor$Context.toSql(SqlImplementor.java:521)
> {code}
> Test case for the issue (RelToSqlConverterTest):
> {code}
>   @Test public void testUnionWrappedInASelect() {
>     String query =
>               "select sum(case when \"product_id\"=0 then \"net_weight\" else 
> 0 end) as net_weight from ( "
>             + "select \"product_id\", \"net_weight\" from \"product\" "
>             + " union all "
>             + "select \"product_id\", 0 as \"net_weight\" from 
> \"sales_fact_1997\" "
>             + ") t0";
>     String expected =
>               "SELECT SUM(CASE WHEN \"product_id\" = 0 THEN \"net_weight\" 
> ELSE 0 END) AS \"NET_WEIGHT\"\n"
>             + "FROM (SELECT \"product_id\", \"net_weight\"\n"
>             + "FROM \"foodmart\".\"product\"\n"
>             + "UNION ALL\n"
>             + "SELECT \"product_id\", 0 AS \"net_weight\"\n"
>             + "FROM \"foodmart\".\"sales_fact_1997\") AS \"t1\"";
>     sql(query).ok(expected);
>   }
> {code}
> The issue is caused by the fix: CALCITE-1332
> The issue should be resolved, as the case is pretty common and the failing 
> sql is fairly simple.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Reply via email to