You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "mihaibudiu (via GitHub)" <gi...@apache.org> on 2023/11/07 00:18:21 UTC

[PR] [CALCITE-6071] RexCall should carry source position information for runtime error reporting [calcite]

mihaibudiu opened a new pull request, #3506:
URL: https://github.com/apache/calcite/pull/3506

   This PR adds source position (`SqlParserPos`) information to two kinds of nodes:
   - RexCall
   - AggregateCall
   
   Not all RexCall nodes and AggregateCall operations need to have meaningful source position information. The most useful nodes are the ones that could lead to runtime errors. So operations like addition (which can overflow) or casts (which can fail when the target type is too narrow) need to carry such information. Nodes where the operation cannot fail dynamically (e.g., MAX, comparisons, or IS_NULL) may have the source position set to ZERO.
   
   In this PR I have carefully audited the entire code base and tried to thread the flow of the position information everywhere, but there are a few places where I couldn't figure out where it should come from. There are, for example, a few casts inserted by the rewrite rules which have no direct correspondence to source constructs. Such casts have now a TODO comment wondering about their runtime safety.
   
   In the process I have also removed a few `@Deprecated` functions which would have required a signature change. I figured that since they are deprecated there is no point to update them, since no one can call the variant with the new signature.
   
   For methods where there are many calls where the position information may legitimately be ZERO I have left variants of the original methods without source position information (e.g., see RexBuilder.makeCall)
   
   Currently there are no uses of this information, but I have inspected the IR with a debugger after optimizations to check that the information is present. A future commit (part of this PR or another one) should add an example runtime error that displays this information. This is not trivial to do for the Calcite Linq4j-based executor, so I haven't done it yet.
   
   None of this should affect the behavior of any of the compiled programs, but I noticed that it's easy to mess up something in practice a few times. Currently no tests need to change.


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6071] RexCall should carry source position information for runtime error reporting [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu merged PR #3506:
URL: https://github.com/apache/calcite/pull/3506


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6071] RexCall should carry source position information for runtime error reporting [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #3506:
URL: https://github.com/apache/calcite/pull/3506#issuecomment-2364189710

   I have resolved the conflicts, rebased and optimistically squashed the commits too


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6071] RexCall should carry source position information for runtime error reporting [calcite]

Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on code in PR #3506:
URL: https://github.com/apache/calcite/pull/3506#discussion_r1384199791


##########
core/src/main/java/org/apache/calcite/rel/core/AggregateCall.java:
##########
@@ -129,58 +111,8 @@ private AggregateCall(SqlAggFunction aggFunction, boolean distinct,
 
   //~ Methods ----------------------------------------------------------------
 
-  @Deprecated // to be removed before 2.0
-  public static AggregateCall create(SqlAggFunction aggFunction,
-      boolean distinct, List<Integer> argList, int groupCount, RelNode input,
-      @Nullable RelDataType type, @Nullable String name) {
-    return create(aggFunction, distinct, false, false,
-        ImmutableList.of(), argList, -1,
-        null, RelCollations.EMPTY, groupCount, input, type, name);
-  }
-
-  @Deprecated // to be removed before 2.0
-  public static AggregateCall create(SqlAggFunction aggFunction,
-      boolean distinct, List<Integer> argList, int filterArg, int groupCount,
-      RelNode input, @Nullable RelDataType type, @Nullable String name) {
-    return create(aggFunction, distinct, false, false,
-        ImmutableList.of(), argList, filterArg,
-        null, RelCollations.EMPTY, groupCount, input, type, name);
-  }
-
-  @Deprecated // to be removed before 2.0
-  public static AggregateCall create(SqlAggFunction aggFunction,
-      boolean distinct, boolean approximate, List<Integer> argList,
-      int filterArg, int groupCount,
-      RelNode input, @Nullable RelDataType type, @Nullable String name) {
-    return create(aggFunction, distinct, approximate, false,
-        ImmutableList.of(), argList,
-        filterArg, null, RelCollations.EMPTY, groupCount, input, type, name);
-  }
-
-  @Deprecated // to be removed before 2.0
-  public static AggregateCall create(SqlAggFunction aggFunction,
-      boolean distinct, boolean approximate, List<Integer> argList,
-      int filterArg, RelCollation collation, int groupCount,
-      RelNode input, @Nullable RelDataType type, @Nullable String name) {
-    return create(aggFunction, distinct, approximate, false,
-        ImmutableList.of(), argList, filterArg,
-        null, collation, groupCount, input, type, name);
-  }
-
-  @Deprecated // to be removed before 2.0
-  public static AggregateCall create(SqlAggFunction aggFunction,
-      boolean distinct, boolean approximate, boolean ignoreNulls,
-      List<Integer> argList, int filterArg,
-      @Nullable ImmutableBitSet distinctKeys, RelCollation collation,
-      int groupCount,
-      RelNode input, @Nullable RelDataType type, @Nullable String name) {
-    return create(aggFunction, distinct, approximate, ignoreNulls,
-        ImmutableList.of(), argList, filterArg,
-        distinctKeys, collation, groupCount, input, type, name);
-  }
-

Review Comment:
   Sorry, we can't remove deprecated stuff.



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6071] RexCall should carry source position information for runtime error reporting [calcite]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3506:
URL: https://github.com/apache/calcite/pull/3506#issuecomment-1816933312

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3506)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3506&resolved=false&types=BUG) [![D](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/D-16px.png 'D')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3506&resolved=false&types=BUG) [2 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3506&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3506&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3506&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3506&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3506&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3506&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3506&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3506&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3506&resolved=false&types=CODE_SMELL) [13 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3506&resolved=false&types=CODE_SMELL)
   
   [![73.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '73.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3506&metric=new_coverage&view=list) [73.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3506&metric=new_coverage&view=list)  
   [![0.8%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.8%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3506&metric=new_duplicated_lines_density&view=list) [0.8% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3506&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6071] RexCall should carry source position information for runtime error reporting [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #3506:
URL: https://github.com/apache/calcite/pull/3506#issuecomment-2285458271

   I really would like to merge this PR, but no one wants to review it.
   Since it doesn't change any of the functionality of Calcite (only adds a new orthogonal feature) the risk is low.
   But it is a bit tedious, I agree.


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6071] RexCall should carry source position information for runtime error reporting [calcite]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3506:
URL: https://github.com/apache/calcite/pull/3506#issuecomment-2270139462

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3506) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [14 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3506&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3506&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3506&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [69.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3506&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.5% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3506&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3506)
   
   


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6071] RexCall should carry source position information for runtime error reporting [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #3506:
URL: https://github.com/apache/calcite/pull/3506#issuecomment-2364166185

   During development I verified that there are no missing cases by deleting the old functions without source position and re-enabling them only after I checked that the existing ones cover all cases.
   I will rebase. The tests should pass.


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6071] RexCall should carry source position information for runtime error reporting [calcite]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3506:
URL: https://github.com/apache/calcite/pull/3506#issuecomment-1843676938

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3506)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3506&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3506&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3506&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3506&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3506&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3506&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3506&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3506&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3506&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3506&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3506&resolved=false&types=CODE_SMELL) [13 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3506&resolved=false&types=CODE_SMELL)
   
   [![73.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '73.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3506&metric=new_coverage&view=list) [73.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3506&metric=new_coverage&view=list)  
   [![0.8%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.8%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3506&metric=new_duplicated_lines_density&view=list) [0.8% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3506&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6071] RexCall should carry source position information for runtime error reporting [calcite]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3506:
URL: https://github.com/apache/calcite/pull/3506#issuecomment-2364236234

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3506) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [12 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3506&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3506&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3506&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [71.4% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3506&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.5% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3506&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3506)
   
   


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6071] RexCall should carry source position information for runtime error reporting [calcite]

Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on code in PR #3506:
URL: https://github.com/apache/calcite/pull/3506#discussion_r1384199193


##########
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableMergeUnionRule.java:
##########
@@ -125,6 +125,7 @@ public EnumerableMergeUnionRule(Config config) {
           RexNode node = fields.get(i);
           if (fieldsRequiringCast.get(i)) {
             final RelDataType targetType = unionFieldList.get(i).getType();
+            // TODO: can this cast fail?

Review Comment:
   Don't add a TODO. Either do it or log a bug.



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6071] RexCall should carry source position information for runtime error reporting [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #3506:
URL: https://github.com/apache/calcite/pull/3506#discussion_r1397731791


##########
core/src/main/java/org/apache/calcite/rel/core/AggregateCall.java:
##########
@@ -129,58 +111,8 @@ private AggregateCall(SqlAggFunction aggFunction, boolean distinct,
 
   //~ Methods ----------------------------------------------------------------
 
-  @Deprecated // to be removed before 2.0
-  public static AggregateCall create(SqlAggFunction aggFunction,
-      boolean distinct, List<Integer> argList, int groupCount, RelNode input,
-      @Nullable RelDataType type, @Nullable String name) {
-    return create(aggFunction, distinct, false, false,
-        ImmutableList.of(), argList, -1,
-        null, RelCollations.EMPTY, groupCount, input, type, name);
-  }
-
-  @Deprecated // to be removed before 2.0
-  public static AggregateCall create(SqlAggFunction aggFunction,
-      boolean distinct, List<Integer> argList, int filterArg, int groupCount,
-      RelNode input, @Nullable RelDataType type, @Nullable String name) {
-    return create(aggFunction, distinct, false, false,
-        ImmutableList.of(), argList, filterArg,
-        null, RelCollations.EMPTY, groupCount, input, type, name);
-  }
-
-  @Deprecated // to be removed before 2.0
-  public static AggregateCall create(SqlAggFunction aggFunction,
-      boolean distinct, boolean approximate, List<Integer> argList,
-      int filterArg, int groupCount,
-      RelNode input, @Nullable RelDataType type, @Nullable String name) {
-    return create(aggFunction, distinct, approximate, false,
-        ImmutableList.of(), argList,
-        filterArg, null, RelCollations.EMPTY, groupCount, input, type, name);
-  }
-
-  @Deprecated // to be removed before 2.0
-  public static AggregateCall create(SqlAggFunction aggFunction,
-      boolean distinct, boolean approximate, List<Integer> argList,
-      int filterArg, RelCollation collation, int groupCount,
-      RelNode input, @Nullable RelDataType type, @Nullable String name) {
-    return create(aggFunction, distinct, approximate, false,
-        ImmutableList.of(), argList, filterArg,
-        null, collation, groupCount, input, type, name);
-  }
-
-  @Deprecated // to be removed before 2.0
-  public static AggregateCall create(SqlAggFunction aggFunction,
-      boolean distinct, boolean approximate, boolean ignoreNulls,
-      List<Integer> argList, int filterArg,
-      @Nullable ImmutableBitSet distinctKeys, RelCollation collation,
-      int groupCount,
-      RelNode input, @Nullable RelDataType type, @Nullable String name) {
-    return create(aggFunction, distinct, approximate, ignoreNulls,
-        ImmutableList.of(), argList, filterArg,
-        distinctKeys, collation, groupCount, input, type, name);
-  }
-

Review Comment:
   I have put all the deprecated functions back



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6071] RexCall should carry source position information for runtime error reporting [calcite]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3506:
URL: https://github.com/apache/calcite/pull/3506#issuecomment-2270178115

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3506) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [13 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3506&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3506&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3506&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [71.3% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3506&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.5% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3506&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3506)
   
   


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6071] RexCall should carry source position information for runtime error reporting [calcite]

Posted by "asolimando (via GitHub)" <gi...@apache.org>.
asolimando commented on PR #3506:
URL: https://github.com/apache/calcite/pull/3506#issuecomment-2364252052

   > I have resolved the conflicts, rebased and optimistically squashed the commits too
   
   As soon as the last tests finish successfully you can go ahead and merge. Thanks for your great work @mihaibudiu!


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6071] RexCall should carry source position information for runtime error reporting [calcite]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3506:
URL: https://github.com/apache/calcite/pull/3506#issuecomment-2048206751

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3506) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [12 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3506&resolved=false&inNewCodePeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3506&metric=new_accepted_issues&view=list)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3506&resolved=false&inNewCodePeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [73.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3506&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.8% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3506&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3506)
   
   


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6071] RexCall should carry source position information for runtime error reporting [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #3506:
URL: https://github.com/apache/calcite/pull/3506#issuecomment-1843690716

   My plan is to merge #3522 first, then try to use this infrastructure for one of these runtime cast failures to give a better error message at runtime. But I hope that the part that I have done so far can be reviewed on its own as well. It's not guaranteed to be complete, but we will only know that when we try to use it.
   


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org