[jira] [Comment Edited] (CALCITE-3300) Add a star identifier as operand to "SqlCountAggFunction" if no operands given while in the method "createCall"

["Wang Weidong (Jira)" <ji...@apache.org>]

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

Wang Weidong edited comment on CALCITE-3300 at 9/3/19 12:11 PM:
----------------------------------------------------------------

[~julianhyde] 
 * "I don't think you should make {{SqlCountAggFunction}} "fix itself"; some caller might conceivably *really* want zero arguments; the bug is in {{RelToSqlConverter}} and that is the place I think you should fix."
 ** It may be better to fix this bug in {{RelToSqlConverter, but the problem is that we cannot distinguish whether "count" with no arguments means "count star" or not. With fixing in }}{{SqlCountAggFunction, }}{{"count" with no arguments only means "count star". How do you think about this? }}{{}}
 * Please use {{assertThat}} rather than {{assertEquals}} to {{assertTrue}}
 ** I've found that there is almost {{assertThat everywhere, but in some situations, it's obviously more convenient to use }}{{assertEquals or }}{{assertTrue, why is }}{{assertThat the only alternative? }}{{}}
 * Are the other changes to {{RelToSqlConverterTest}} necessary?
 ** That is because the I need to get SqlNode to check whether there is "count star" but I did not found any available methods or classes to use.


was (Author: wangweidong):
[~julianhyde] 
 * "I don't think you should make {{SqlCountAggFunction}} "fix itself"; some caller might conceivably *really* want zero arguments; the bug is in {{RelToSqlConverter}} and that is the place I think you should fix."
 ** It may be better to fix this bug in {{RelToSqlConverter, but the problem is that we cannot distinguish whether "count" with no arguments means "count star" or not. With fixing }}{{SqlCountAggFunction in, }}{{"count" with no arguments only means "count star". How do you think about this? }}{{}}
 * Please use {{assertThat}} rather than {{assertEquals}} to {{assertTrue}}
 ** I've found that there is almost {{assertThat everywhere, but in some situations, it's obviously more convenient to use }}{{assertEquals or }}{{assertTrue, why is }}{{assertThat the only alternative? }}{{}}
 * Are the other changes to {{RelToSqlConverterTest}} necessary?
 ** That is because the I need to get SqlNode to check whether there is "count star" but I did not found any available methods or classes to use.

> Add a star identifier as operand to "SqlCountAggFunction" if no operands given while in the method "createCall"
> ---------------------------------------------------------------------------------------------------------------
>
>                 Key: CALCITE-3300
>                 URL: https://issues.apache.org/jira/browse/CALCITE-3300
>             Project: Calcite
>          Issue Type: Bug
>          Components: core
>    Affects Versions: 1.21.0
>            Reporter: Wang Weidong
>            Assignee: Wang Weidong
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> h4. Procedure to reproduce the problem:
>  - create table {{~^t^~}}
>  - parse query  select count(*) from t to SqlNode
>  - convert SqlNode to RelNode
>  - convert RelNode to SqlNode
>  - validate the result SqlNode
> test code is like this
> {code:java}
> // code placeholder
> @Test
> public void testSelectCountAll() throws Exception {
>   try (Statement s = parserContext.getStatement()) {
>     s.execute("create table if not exists t (i int not null)");
>     String sql = "select count(*) from t";
>     SqlNode sqlNode = parserContext.parseStmt(sql);
>     parserContext.getSqlValidator().validate(sqlNode);
>     RelNode relNode = parserContext.getSqlToRelConverter().convertQuery(sqlNode, true, true).rel;
>     SqlNode sqlNodeNew = toSqlNode(relNode);
>     parserContext.getSqlValidator().validate(sqlNodeNew);
>   }
> }
> {code}
> Finally we will get an exception as follow:
> {code:java}
> // code placeholder
> org.apache.calcite.runtime.CalciteContextException: At line 0, column 0: Invalid number of arguments to function 'COUNT'. Was expecting 1 argumentsorg.apache.calcite.runtime.CalciteContextException: At line 0, column 0: Invalid number of arguments to function 'COUNT'. Was expecting 1 arguments at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62) at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45) at java.lang.reflect.Constructor.newInstance(Constructor.java:423) at org.apache.calcite.runtime.Resources$ExInstWithCause.ex(Resources.java:463) at org.apache.calcite.sql.SqlUtil.newContextException(SqlUtil.java:783) at org.apache.calcite.sql.SqlUtil.newContextException(SqlUtil.java:768) at org.apache.calcite.sql.validate.SqlValidatorImpl.newValidationError(SqlValidatorImpl.java:4753) at org.apache.calcite.sql.validate.SqlValidatorImpl.handleUnresolvedFunction(SqlValidatorImpl.java:1699) at org.apache.calcite.sql.SqlFunction.deriveType(SqlFunction.java:270) at org.apache.calcite.sql.SqlFunction.deriveType(SqlFunction.java:215) at org.apache.calcite.sql.fun.SqlCountAggFunction.deriveType(SqlCountAggFunction.java:83) at org.apache.calcite.sql.validate.SqlValidatorImpl$DeriveTypeVisitor.visit(SqlValidatorImpl.java:5477) at org.apache.calcite.sql.validate.SqlValidatorImpl$DeriveTypeVisitor.visit(SqlValidatorImpl.java:5464) at org.apache.calcite.sql.SqlCall.accept(SqlCall.java:138) at org.apache.calcite.sql.validate.SqlValidatorImpl.deriveTypeImpl(SqlValidatorImpl.java:1629) at org.apache.calcite.sql.validate.SqlValidatorImpl.deriveType(SqlValidatorImpl.java:1614) at org.apache.calcite.sql.validate.SqlValidatorImpl.expandSelectItem(SqlValidatorImpl.java:457) at org.apache.calcite.sql.validate.SqlValidatorImpl.validateSelectList(SqlValidatorImpl.java:4017) at org.apache.calcite.sql.validate.SqlValidatorImpl.validateSelect(SqlValidatorImpl.java:3280) at org.apache.calcite.sql.validate.SelectNamespace.validateImpl(SelectNamespace.java:60) at org.apache.calcite.sql.validate.AbstractNamespace.validate(AbstractNamespace.java:84) at org.apache.calcite.sql.validate.SqlValidatorImpl.validateNamespace(SqlValidatorImpl.java:967) at org.apache.calcite.sql.validate.SqlValidatorImpl.validateQuery(SqlValidatorImpl.java:943) at org.apache.calcite.sql.SqlSelect.validate(SqlSelect.java:225) at org.apache.calcite.sql.validate.SqlValidatorImpl.validateScopedExpression(SqlValidatorImpl.java:918) at org.apache.calcite.sql.validate.SqlValidatorImpl.validate(SqlValidatorImpl.java:628) at org.apache.calcite.test.OptimizeTest.testSelectCountAll(OptimizeTest.java:220) 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.rules.ExpectedException$ExpectedExceptionStatement.evaluate(ExpectedException.java:239) at org.junit.rules.RunRules.evaluate(RunRules.java:20) 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:68) at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47) at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242) at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)Caused by: org.apache.calcite.sql.validate.SqlValidatorException: Invalid number of arguments to function 'COUNT'. Was expecting 1 arguments at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62) at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45) at java.lang.reflect.Constructor.newInstance(Constructor.java:423) at org.apache.calcite.runtime.Resources$ExInstWithCause.ex(Resources.java:463) at org.apache.calcite.runtime.Resources$ExInst.ex(Resources.java:572) ... 47 more{code}
>  
> After debugging, I found that the *main causes* of this exception are as follows.
> First, while converting RelNode to SqlNode, in the method {{~^org.apache.calcite.rel.rel2sql.SqlImplementor.Context#toSql^~}}
> [https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java#L869],
> the operandList is empty because that there is with no argument in aggCall.
> Second,  while validating  the result SqlNode, in the method  org.apache.calcite.sql.fun.SqlCountAggFunction#deriveType
> [https://github.com/apache/calcite/blob/8327e674e7f0a768d124fa37fd75cda4b8a35bb6/core/src/main/java/org/apache/calcite/sql/fun/SqlCountAggFunction.java#L79],
> the result of function call {{call.isCountStar() is false}}, because there is no operand in {{call. While actually this is a count star call.}}
>  
> *How to solve this problem?*  
> I think we can solve it either in the first or second step, but solving it in the second step is more appropriate because SqlNode converted from RelNode should be same with the result parsed from sql string possibly.
>  



--
This message was sent by Atlassian Jira
(v8.3.2#803003)