You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Steve Carlin (Code Review)" <ge...@cloudera.org> on 2025/10/08 13:50:00 UTC
[Impala-ASF-CR] IMPALA-13695: Calcite planner: fix for ndv with 2 args
Steve Carlin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/23513
Change subject: IMPALA-13695: Calcite planner: fix for ndv with 2 args
......................................................................
IMPALA-13695: Calcite planner: fix for ndv with 2 args
The NDV function was crashing when called with the "scale" arg. This
requires special processing which exists in FunctionCallExpr.
The validation for this is now done in ImpalaNdvFunction
and the special calculation is done within ImpalaAggRel
This also fixes ndv for varchar types. The aggregation call
within CoerceNodes was not differentiating between varchar
and string. A cast to string function is needed in order
to run the ndv function on a varchar column.
Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
---
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaCustomOperatorTable.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaNdvFunction.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaAggRel.java
M testdata/workloads/functional-query/queries/QueryTest/calcite.test
6 files changed, 104 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/23513/2
--
To view, visit http://gerrit.cloudera.org:8080/23513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
Gerrit-Change-Number: 23513
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
[Impala-ASF-CR] IMPALA-13695: Calcite planner: fix for ndv with 2 args
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23513 )
Change subject: IMPALA-13695: Calcite planner: fix for ndv with 2 args
......................................................................
Patch Set 4: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/12587/
--
To view, visit http://gerrit.cloudera.org:8080/23513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
Gerrit-Change-Number: 23513
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Oct 2025 21:07:42 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13695: Calcite planner: fix for ndv with 2 args
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23513 )
Change subject: IMPALA-13695: Calcite planner: fix for ndv with 2 args
......................................................................
Patch Set 4:
(1 comment)
gerrit-auto-critic failed. You can reproduce it locally using command:
python3 bin/jenkins/critique-gerrit-review.py --dryrun
To run it, you might need a virtual env with Python3's venv installed.
http://gerrit.cloudera.org:8080/#/c/23513/4/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaAggRel.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaAggRel.java:
http://gerrit.cloudera.org:8080/#/c/23513/4/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaAggRel.java@358
PS4, Line 358: Function fn = FunctionResolver.getExactFunction(aggFunction.getName(), aggFunction.getKind(),
line too long (97 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/23513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
Gerrit-Change-Number: 23513
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Oct 2025 16:22:31 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13695: Calcite planner: fix for ndv with 2 args
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23513 )
Change subject: IMPALA-13695: Calcite planner: fix for ndv with 2 args
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/23513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
Gerrit-Change-Number: 23513
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Oct 2025 23:26:28 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13695: Calcite planner: fix for ndv with 2 args
Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Steve Carlin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/23513 )
Change subject: IMPALA-13695: Calcite planner: fix for ndv with 2 args
......................................................................
IMPALA-13695: Calcite planner: fix for ndv with 2 args
The NDV function was crashing when called with the "scale" arg. This
requires special processing which exists in FunctionCallExpr.
The validation for this is now done in ImpalaNdvFunction
and the special calculation is done within ImpalaAggRel
This also fixes ndv for varchar types. The aggregation call
within CoerceNodes was not differentiating between varchar
and string. A cast to string function is needed in order
to run the ndv function on a varchar column.
Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
Reviewed-on: http://gerrit.cloudera.org:8080/23513
Reviewed-by: Riza Suminto <ri...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaCustomOperatorTable.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaNdvFunction.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaAggRel.java
M testdata/workloads/functional-query/queries/QueryTest/calcite.test
6 files changed, 122 insertions(+), 7 deletions(-)
Approvals:
Riza Suminto: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/23513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
Gerrit-Change-Number: 23513
Gerrit-PatchSet: 6
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
[Impala-ASF-CR] IMPALA-13695: Calcite planner: fix for ndv with 2 args
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/23513 )
Change subject: IMPALA-13695: Calcite planner: fix for ndv with 2 args
......................................................................
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/23513/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java:
http://gerrit.cloudera.org:8080/#/c/23513/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java@651
PS2, Line 651: int maxVal = Integer.MAX_VALUE;
The comparison is being with Integer.MAX_VALUE which is 2^31 - 1 but the comment above says -1. Why not just compare with -1 ?
http://gerrit.cloudera.org:8080/#/c/23513/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaNdvFunction.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaNdvFunction.java:
http://gerrit.cloudera.org:8080/#/c/23513/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaNdvFunction.java@35
PS2, Line 35: if (callBinding.getOperandCount() == 1) {
Here it checks for 1 or more arguments. What if it is 0 args ? e.g NDV() .. is that rejected before getting here ?
Also, what if it is more than 2 arguments ?
http://gerrit.cloudera.org:8080/#/c/23513/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaNdvFunction.java@41
PS2, Line 41: ndv
nit: SQL functions should preferably be upper case in error messages or logs in order to distinguish from rest of the text. I doubt we follow this everywhere but it's a good convention.
http://gerrit.cloudera.org:8080/#/c/23513/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaNdvFunction.java@46
PS2, Line 46: ndv
nit: same as above.
--
To view, visit http://gerrit.cloudera.org:8080/23513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
Gerrit-Change-Number: 23513
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 15 Oct 2025 18:10:22 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13695: Calcite planner: fix for ndv with 2 args
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23513 )
Change subject: IMPALA-13695: Calcite planner: fix for ndv with 2 args
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/20707/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/23513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
Gerrit-Change-Number: 23513
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Oct 2025 15:34:13 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13695: Calcite planner: fix for ndv with 2 args
Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Fang-Yu Rao, Riza Suminto, Joe McDonnell, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/23513
to look at the new patch set (#4).
Change subject: IMPALA-13695: Calcite planner: fix for ndv with 2 args
......................................................................
IMPALA-13695: Calcite planner: fix for ndv with 2 args
The NDV function was crashing when called with the "scale" arg. This
requires special processing which exists in FunctionCallExpr.
The validation for this is now done in ImpalaNdvFunction
and the special calculation is done within ImpalaAggRel
This also fixes ndv for varchar types. The aggregation call
within CoerceNodes was not differentiating between varchar
and string. A cast to string function is needed in order
to run the ndv function on a varchar column.
Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
---
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaCustomOperatorTable.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaNdvFunction.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaAggRel.java
M testdata/workloads/functional-query/queries/QueryTest/calcite.test
6 files changed, 121 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/23513/4
--
To view, visit http://gerrit.cloudera.org:8080/23513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
Gerrit-Change-Number: 23513
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
[Impala-ASF-CR] IMPALA-13695: Calcite planner: fix for ndv with 2 args
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23513 )
Change subject: IMPALA-13695: Calcite planner: fix for ndv with 2 args
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/12587/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/23513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
Gerrit-Change-Number: 23513
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Oct 2025 16:20:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13695: Calcite planner: fix for ndv with 2 args
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/23513 )
Change subject: IMPALA-13695: Calcite planner: fix for ndv with 2 args
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/23513/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java:
http://gerrit.cloudera.org:8080/#/c/23513/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java@651
PS2, Line 651: int maxVal = Integer.MAX_VALUE;
> The comparison is being with Integer.MAX_VALUE which is 2^31 - 1 but the c
Pending a response on this comment, the patch looks good to me.
--
To view, visit http://gerrit.cloudera.org:8080/23513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
Gerrit-Change-Number: 23513
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Fri, 17 Oct 2025 22:56:59 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13695: Calcite planner: fix for ndv with 2 args
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23513 )
Change subject: IMPALA-13695: Calcite planner: fix for ndv with 2 args
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/20640/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/23513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
Gerrit-Change-Number: 23513
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Oct 2025 14:15:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13695: Calcite planner: fix for ndv with 2 args
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/23513 )
Change subject: IMPALA-13695: Calcite planner: fix for ndv with 2 args
......................................................................
Patch Set 3: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/23513/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaNdvFunction.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaNdvFunction.java:
http://gerrit.cloudera.org:8080/#/c/23513/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaNdvFunction.java@39
PS2, Line 39:
: if (operandCount == 1) {
: return true;
: }
:
: BigDecimal bd = callBinding.getOperandLiteralValue(1, BigDecimal.class);
: if (bd == null) {
: throw new IllegalArgumentException("Error in NDV function, " +
: "second parameter needs to be an integer.");
: }
> Done
Done
--
To view, visit http://gerrit.cloudera.org:8080/23513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
Gerrit-Change-Number: 23513
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Oct 2025 15:13:19 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13695: Calcite planner: fix for ndv with 2 args
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/23513 )
Change subject: IMPALA-13695: Calcite planner: fix for ndv with 2 args
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/23513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
Gerrit-Change-Number: 23513
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Oct 2025 17:29:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13695: Calcite planner: fix for ndv with 2 args
Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Fang-Yu Rao, Riza Suminto, Joe McDonnell, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/23513
to look at the new patch set (#3).
Change subject: IMPALA-13695: Calcite planner: fix for ndv with 2 args
......................................................................
IMPALA-13695: Calcite planner: fix for ndv with 2 args
The NDV function was crashing when called with the "scale" arg. This
requires special processing which exists in FunctionCallExpr.
The validation for this is now done in ImpalaNdvFunction
and the special calculation is done within ImpalaAggRel
This also fixes ndv for varchar types. The aggregation call
within CoerceNodes was not differentiating between varchar
and string. A cast to string function is needed in order
to run the ndv function on a varchar column.
Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
---
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaCustomOperatorTable.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaNdvFunction.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaAggRel.java
M testdata/workloads/functional-query/queries/QueryTest/calcite.test
6 files changed, 121 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/23513/3
--
To view, visit http://gerrit.cloudera.org:8080/23513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
Gerrit-Change-Number: 23513
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-13695: Calcite planner: fix for ndv with 2 args
Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/23513 )
Change subject: IMPALA-13695: Calcite planner: fix for ndv with 2 args
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/23513/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java:
http://gerrit.cloudera.org:8080/#/c/23513/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java@651
PS2, Line 651: int maxVal = Integer.MAX_VALUE;
> The comparison is being with Integer.MAX_VALUE which is 2^31 - 1 but the c
The norm for Calcite is to use MAX_VALUE for the precision of strings. Changed the comment rather than the value.
http://gerrit.cloudera.org:8080/#/c/23513/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java@651
PS2, Line 651: int maxVal = Integer.MAX_VALUE;
> Pending a response on this comment, the patch looks good to me.
Whoops, missed this one, fixed
--
To view, visit http://gerrit.cloudera.org:8080/23513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
Gerrit-Change-Number: 23513
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Oct 2025 16:20:32 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13695: Calcite planner: fix for ndv with 2 args
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/23513 )
Change subject: IMPALA-13695: Calcite planner: fix for ndv with 2 args
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/23513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
Gerrit-Change-Number: 23513
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Oct 2025 18:27:45 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13695: Calcite planner: fix for ndv with 2 args
Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Fang-Yu Rao, Riza Suminto, Joe McDonnell, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/23513
to look at the new patch set (#5).
Change subject: IMPALA-13695: Calcite planner: fix for ndv with 2 args
......................................................................
IMPALA-13695: Calcite planner: fix for ndv with 2 args
The NDV function was crashing when called with the "scale" arg. This
requires special processing which exists in FunctionCallExpr.
The validation for this is now done in ImpalaNdvFunction
and the special calculation is done within ImpalaAggRel
This also fixes ndv for varchar types. The aggregation call
within CoerceNodes was not differentiating between varchar
and string. A cast to string function is needed in order
to run the ndv function on a varchar column.
Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
---
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaCustomOperatorTable.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaNdvFunction.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaAggRel.java
M testdata/workloads/functional-query/queries/QueryTest/calcite.test
6 files changed, 122 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/23513/5
--
To view, visit http://gerrit.cloudera.org:8080/23513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
Gerrit-Change-Number: 23513
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
[Impala-ASF-CR] IMPALA-13695: Calcite planner: fix for ndv with 2 args
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23513 )
Change subject: IMPALA-13695: Calcite planner: fix for ndv with 2 args
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/20731/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/23513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
Gerrit-Change-Number: 23513
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Oct 2025 18:38:38 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13695: Calcite planner: fix for ndv with 2 args
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/23513 )
Change subject: IMPALA-13695: Calcite planner: fix for ndv with 2 args
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/23513/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaNdvFunction.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaNdvFunction.java:
http://gerrit.cloudera.org:8080/#/c/23513/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaNdvFunction.java@39
PS2, Line 39: BigDecimal bd = callBinding.getOperandLiteralValue(1, BigDecimal.class);
: if (bd == null) {
: throw new IllegalArgumentException("Error in ndv function, " +
: "second parameter needs to be an integer.");
: }
:
: if (bd.intValue() < 1 || bd.intValue() > 10) {
: throw new IllegalArgumentException("Error in ndv function, " +
: "second parameter needs to be between 1 and 10.");
: }
Can you add tests that trigger this error in calcite.test please?
--
To view, visit http://gerrit.cloudera.org:8080/23513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
Gerrit-Change-Number: 23513
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Mon, 13 Oct 2025 16:56:23 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13695: Calcite planner: fix for ndv with 2 args
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23513 )
Change subject: IMPALA-13695: Calcite planner: fix for ndv with 2 args
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/12588/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/23513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
Gerrit-Change-Number: 23513
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Oct 2025 18:39:59 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13695: Calcite planner: fix for ndv with 2 args
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23513 )
Change subject: IMPALA-13695: Calcite planner: fix for ndv with 2 args
......................................................................
Patch Set 2:
(1 comment)
gerrit-auto-critic failed. You can reproduce it locally using command:
python3 bin/jenkins/critique-gerrit-review.py --dryrun
To run it, you might need a virtual env with Python3's venv installed.
http://gerrit.cloudera.org:8080/#/c/23513/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaAggRel.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaAggRel.java:
http://gerrit.cloudera.org:8080/#/c/23513/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaAggRel.java@358
PS2, Line 358: Function fn = FunctionResolver.getExactFunction(aggFunction.getName(), aggFunction.getKind(),
line too long (97 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/23513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
Gerrit-Change-Number: 23513
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Oct 2025 13:52:16 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13695: Calcite planner: fix for ndv with 2 args
Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/23513 )
Change subject: IMPALA-13695: Calcite planner: fix for ndv with 2 args
......................................................................
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/23513/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaNdvFunction.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaNdvFunction.java:
http://gerrit.cloudera.org:8080/#/c/23513/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaNdvFunction.java@35
PS2, Line 35: if (callBinding.getOperandCount() == 1) {
> Here it checks for 1 or more arguments. What if it is 0 args ? e.g NDV() .
Well, heh, it threw an exception, but not a clean one.
Fixed the code.
http://gerrit.cloudera.org:8080/#/c/23513/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaNdvFunction.java@41
PS2, Line 41: ndv
> nit: SQL functions should preferably be upper case in error messages or log
Done
http://gerrit.cloudera.org:8080/#/c/23513/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaNdvFunction.java@46
PS2, Line 46: ndv
> nit: same as above.
Done
http://gerrit.cloudera.org:8080/#/c/23513/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaNdvFunction.java@39
PS2, Line 39: BigDecimal bd = callBinding.getOperandLiteralValue(1, BigDecimal.class);
: if (bd == null) {
: throw new IllegalArgumentException("Error in ndv function, " +
: "second parameter needs to be an integer.");
: }
:
: if (bd.intValue() < 1 || bd.intValue() > 10) {
: throw new IllegalArgumentException("Error in ndv function, " +
: "second parameter needs to be between 1 and 10.");
: }
> Can you add tests that trigger this error in calcite.test please?
Done
--
To view, visit http://gerrit.cloudera.org:8080/23513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
Gerrit-Change-Number: 23513
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Oct 2025 15:09:15 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13695: Calcite planner: fix for ndv with 2 args
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23513 )
Change subject: IMPALA-13695: Calcite planner: fix for ndv with 2 args
......................................................................
Patch Set 3:
(1 comment)
gerrit-auto-critic failed. You can reproduce it locally using command:
python3 bin/jenkins/critique-gerrit-review.py --dryrun
To run it, you might need a virtual env with Python3's venv installed.
http://gerrit.cloudera.org:8080/#/c/23513/3/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaAggRel.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaAggRel.java:
http://gerrit.cloudera.org:8080/#/c/23513/3/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaAggRel.java@358
PS3, Line 358: Function fn = FunctionResolver.getExactFunction(aggFunction.getName(), aggFunction.getKind(),
line too long (97 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/23513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
Gerrit-Change-Number: 23513
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Oct 2025 15:09:42 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13695: Calcite planner: fix for ndv with 2 args
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23513 )
Change subject: IMPALA-13695: Calcite planner: fix for ndv with 2 args
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/20726/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/23513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82419f77e043e9975865a042ffb8db75a26931f7
Gerrit-Change-Number: 23513
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Oct 2025 16:47:09 +0000
Gerrit-HasComments: No