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 2026/04/13 12:54:48 UTC

[Impala-ASF-CR] IMPALA-14886: Add trace information into Calcite Join rule

Steve Carlin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/24183


Change subject: IMPALA-14886: Add trace information into Calcite Join rule
......................................................................

IMPALA-14886: Add trace information into Calcite Join rule

This commit adds some trace level logging into the join optimizer
rule to give more detail into how it makes the decisions of join
ordering.

Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
---
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaCost.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaRelMdNonCumulativeCost.java
3 files changed, 85 insertions(+), 2 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/24183/1
-- 
To view, visit http://gerrit.cloudera.org:8080/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 1
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>

[Impala-ASF-CR] IMPALA-14886: Add trace information for Calcite.

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24183 )

Change subject: IMPALA-14886: Add trace information for Calcite.
......................................................................


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/24183/8/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java:

http://gerrit.cloudera.org:8080/#/c/24183/8/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java@2214
PS8, Line 2214:       Preconditions.checkState(rel.getInputs().size() == 1);
> This would assert for RelNodes that have 2 inputs that are not covered by p
I think this is ok for now. Those RelNodes are removed in the CalciteRelNodeConverter.  If they exist, there are gonna be problems in a lot of other places as well which would also need to be fixed.



-- 
To view, visit http://gerrit.cloudera.org:8080/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 8
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 May 2026 02:31:35 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-14886: Add trace information into Calcite Join rule

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24183 )

Change subject: IMPALA-14886: Add trace information into Calcite Join rule
......................................................................


Patch Set 1:

(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/24183/1/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java:

http://gerrit.cloudera.org:8080/#/c/24183/1/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java@114
PS1, Line 114:   protected static final Logger LOG = LoggerFactory.getLogger(ImpalaLoptOptimizeJoinRule.class.getName());
line too long (106 > 90)



-- 
To view, visit http://gerrit.cloudera.org:8080/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 1
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 13 Apr 2026 12:56:59 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-14886: Add trace information for Calcite.

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Joe McDonnell, Michael Smith, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/24183

to look at the new patch set (#5).

Change subject: IMPALA-14886: Add trace information for Calcite.
......................................................................

IMPALA-14886: Add trace information for Calcite.

This commit adds some trace level logging into the join optimizer
rule to give more detail into how it makes the decisions of join
ordering.

Also, the RuleEventLogger is now added in trace mode which shows
which rules get attempted and applied. Within Calcite, the RuleEventLogger
logs at DEBUG level, but it is quite verbose, so the TRACE wrapper
within CalciteOptimizer needs to be applied to see the information.

Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
---
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaRelMdNonCumulativeCost.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java
3 files changed, 120 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/24183/5
-- 
To view, visit http://gerrit.cloudera.org:8080/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>

[Impala-ASF-CR] IMPALA-14886: Add trace information for Calcite.

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Joe McDonnell, Michael Smith, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/24183

to look at the new patch set (#7).

Change subject: IMPALA-14886: Add trace information for Calcite.
......................................................................

IMPALA-14886: Add trace information for Calcite.

This commit adds some trace level logging into the join optimizer
rule to give more detail into how it makes the decisions of join
ordering.

Also, the RuleEventLogger is now added in trace mode which shows
which rules get attempted and applied. Within Calcite, the RuleEventLogger
logs at DEBUG level, but it is quite verbose, so the TRACE wrapper
within CalciteOptimizer needs to be applied to see the information.

Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
---
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaRelMdNonCumulativeCost.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java
4 files changed, 92 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/24183/7
-- 
To view, visit http://gerrit.cloudera.org:8080/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 7
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>

[Impala-ASF-CR] IMPALA-14886: Add trace information for Calcite.

Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/24183 )

Change subject: IMPALA-14886: Add trace information for Calcite.
......................................................................


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/24183/8/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java:

http://gerrit.cloudera.org:8080/#/c/24183/8/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java@2214
PS8, Line 2214:       Preconditions.checkState(rel.getInputs().size() == 1);
This would assert for RelNodes that have 2 inputs that are not covered by previous checks above.  For instance Intersect, Minus,  Correlate .. these all have 2 inputs.  We should handle those.



-- 
To view, visit http://gerrit.cloudera.org:8080/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 8
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 May 2026 00:44:07 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-14886: Add trace information into Calcite Join rule

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24183 )

Change subject: IMPALA-14886: Add trace information into Calcite Join rule
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/22118/ : 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/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 1
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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-Comment-Date: Mon, 13 Apr 2026 13:16:25 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14886: Add trace information for Calcite.

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Joe McDonnell, Michael Smith, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/24183

to look at the new patch set (#8).

Change subject: IMPALA-14886: Add trace information for Calcite.
......................................................................

IMPALA-14886: Add trace information for Calcite.

This commit adds some trace level logging into the join optimizer
rule to give more detail into how it makes the decisions of join
ordering.

Also, the RuleEventLogger is now added in trace mode which shows
which rules get attempted and applied. Within Calcite, the RuleEventLogger
logs at DEBUG level, but it is quite verbose, so the TRACE wrapper
within CalciteOptimizer needs to be applied to see the information.

Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
---
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaRelMdNonCumulativeCost.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java
4 files changed, 98 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/24183/8
-- 
To view, visit http://gerrit.cloudera.org:8080/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 8
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Steve Carlin <sc...@cloudera.com>

[Impala-ASF-CR] IMPALA-14886: Add trace information for Calcite.

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/24183 )

Change subject: IMPALA-14886: Add trace information for Calcite.
......................................................................


Patch Set 8: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/24183/8/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java:

http://gerrit.cloudera.org:8080/#/c/24183/8/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java@2201
PS8, Line 2201:       return getTableName((TableScan) rel);
Could be ((TableScan) rel).getTable().toString(), but I accept "<unknown table name>" may display nicer for non-CalciteTables.



-- 
To view, visit http://gerrit.cloudera.org:8080/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 8
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 May 2026 18:10:18 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-14886: Add trace information for Calcite.

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/24183 )

Change subject: IMPALA-14886: Add trace information for Calcite.
......................................................................


Patch Set 5:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/24183/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java:

http://gerrit.cloudera.org:8080/#/c/24183/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java@1012
PS5, Line 1012:         LOG.trace("cost for pushdown tree is " + costPushDown);
nit: could avoid isTraceEnabled with slf4j substitution

  Log.trace("cost ... is {}", costPushDown);


http://gerrit.cloudera.org:8080/#/c/24183/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java@1853
PS5, Line 1853:       if (LOG.isTraceEnabled()) {
This is a pointless check.


http://gerrit.cloudera.org:8080/#/c/24183/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaRelMdNonCumulativeCost.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaRelMdNonCumulativeCost.java:

http://gerrit.cloudera.org:8080/#/c/24183/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaRelMdNonCumulativeCost.java@106
PS5, Line 106:       LOG.trace("calculating join cost, left row count = " + leftRCount +
Use substitution and remove isTraceEnabled. We only use the guards to prevent expensive operations that are only needed for logging.


http://gerrit.cloudera.org:8080/#/c/24183/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaRelMdNonCumulativeCost.java@137
PS5, Line 137:           getTableName(scan) + ", cardinality = " +
How about overriding toString?



-- 
To view, visit http://gerrit.cloudera.org:8080/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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-Comment-Date: Fri, 01 May 2026 19:03:16 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-14886: Add trace information into Calcite Join rule

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24183 )

Change subject: IMPALA-14886: Add trace information into Calcite Join rule
......................................................................


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/24183/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java:

http://gerrit.cloudera.org:8080/#/c/24183/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java@114
PS2, Line 114:   protected static final Logger LOG = LoggerFactory.getLogger(ImpalaLoptOptimizeJoinRule.class.getName());
line too long (106 > 90)



-- 
To view, visit http://gerrit.cloudera.org:8080/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 13 Apr 2026 13:03:19 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-14886: Add trace information for Calcite.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24183 )

Change subject: IMPALA-14886: Add trace information for Calcite.
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/22125/ : 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/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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-Comment-Date: Mon, 13 Apr 2026 17:08:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14886: Add trace information for Calcite.

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24183 )

Change subject: IMPALA-14886: Add trace information for Calcite.
......................................................................


Patch Set 7:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/24183/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java:

http://gerrit.cloudera.org:8080/#/c/24183/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java@1005
PS7, Line 1005:           getJoinTableString(pushDownTree.getJoinTree()));
> This is a spot I'd still use isTraceEnabled, because it's doing additional 
Brain isn't on right today.  Of course. :)


http://gerrit.cloudera.org:8080/#/c/24183/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java@1010
PS7, Line 1010:       LOG.trace("calculating top tree {}", getJoinTableString(topTree.getJoinTree()));
> Same here.
Done


http://gerrit.cloudera.org:8080/#/c/24183/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java@1038
PS7, Line 1038:       LOG.trace(getJoinTableString(pushDownTree.getJoinTree()));
> Same here. Might as well guard the whole block.
Done


http://gerrit.cloudera.org:8080/#/c/24183/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java@1827
PS7, Line 1827:     LOG.trace("Checking for swap inputs");
> And here.
Done


http://gerrit.cloudera.org:8080/#/c/24183/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaRelMdNonCumulativeCost.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaRelMdNonCumulativeCost.java:

http://gerrit.cloudera.org:8080/#/c/24183/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaRelMdNonCumulativeCost.java@170
PS7, Line 170:       LOG.trace("calculating aggregate cost " + "row count = " + rCount +
> This could also use substitution instead of string concatenation.
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 7
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 May 2026 18:01:29 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-14886: Add trace information for Calcite.

Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/24183 )

Change subject: IMPALA-14886: Add trace information for Calcite.
......................................................................


Patch Set 8: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/24183/8/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java:

http://gerrit.cloudera.org:8080/#/c/24183/8/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java@2214
PS8, Line 2214:       Preconditions.checkState(rel.getInputs().size() == 1);
> I think this is ok for now. Those RelNodes are removed in the CalciteRelNod
Ack.  Marking this done.



-- 
To view, visit http://gerrit.cloudera.org:8080/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 8
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 May 2026 05:32:35 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-14886: Add trace information for Calcite.

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Steve Carlin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/24183 )

Change subject: IMPALA-14886: Add trace information for Calcite.
......................................................................

IMPALA-14886: Add trace information for Calcite.

This commit adds some trace level logging into the join optimizer
rule to give more detail into how it makes the decisions of join
ordering.

Also, the RuleEventLogger is now added in trace mode which shows
which rules get attempted and applied. Within Calcite, the RuleEventLogger
logs at DEBUG level, but it is quite verbose, so the TRACE wrapper
within CalciteOptimizer needs to be applied to see the information.

Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Reviewed-on: http://gerrit.cloudera.org:8080/24183
Reviewed-by: Michael Smith <mi...@cloudera.com>
Reviewed-by: Aman Sinha <am...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaRelMdNonCumulativeCost.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java
4 files changed, 98 insertions(+), 1 deletion(-)

Approvals:
  Michael Smith: Looks good to me, but someone else must approve
  Aman Sinha: Looks good to me, approved
  Impala Public Jenkins: Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 9
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Steve Carlin <sc...@cloudera.com>

[Impala-ASF-CR] IMPALA-14886: Add trace information for Calcite.

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/24183 )

Change subject: IMPALA-14886: Add trace information for Calcite.
......................................................................


Patch Set 7:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/24183/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java:

http://gerrit.cloudera.org:8080/#/c/24183/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java@1005
PS7, Line 1005:           getJoinTableString(pushDownTree.getJoinTree()));
This is a spot I'd still use isTraceEnabled, because it's doing additional calculation that LOG.trace can't skip.


http://gerrit.cloudera.org:8080/#/c/24183/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java@1010
PS7, Line 1010:       LOG.trace("calculating top tree {}", getJoinTableString(topTree.getJoinTree()));
Same here.


http://gerrit.cloudera.org:8080/#/c/24183/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java@1038
PS7, Line 1038:       LOG.trace(getJoinTableString(pushDownTree.getJoinTree()));
Same here. Might as well guard the whole block.


http://gerrit.cloudera.org:8080/#/c/24183/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java@1827
PS7, Line 1827:     LOG.trace("Checking for swap inputs");
And here.


http://gerrit.cloudera.org:8080/#/c/24183/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaRelMdNonCumulativeCost.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaRelMdNonCumulativeCost.java:

http://gerrit.cloudera.org:8080/#/c/24183/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaRelMdNonCumulativeCost.java@170
PS7, Line 170:       LOG.trace("calculating aggregate cost " + "row count = " + rCount +
This could also use substitution instead of string concatenation.



-- 
To view, visit http://gerrit.cloudera.org:8080/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 7
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 May 2026 16:45:10 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-14886: Add trace information into Calcite Join rule

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24183 )

Change subject: IMPALA-14886: Add trace information into Calcite Join rule
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/22119/ : 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/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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-Comment-Date: Mon, 13 Apr 2026 13:23:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14886: Add trace information into Calcite Join rule

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24183 )

Change subject: IMPALA-14886: Add trace information into Calcite Join rule
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/22120/ : 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/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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-Comment-Date: Mon, 13 Apr 2026 13:25:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14886: Add trace information for Calcite.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24183 )

Change subject: IMPALA-14886: Add trace information for Calcite.
......................................................................


Patch Set 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/22307/ : 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/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 6
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 May 2026 16:28:33 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14886: Add trace information for Calcite.

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Joe McDonnell, Michael Smith, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/24183

to look at the new patch set (#6).

Change subject: IMPALA-14886: Add trace information for Calcite.
......................................................................

IMPALA-14886: Add trace information for Calcite.

This commit adds some trace level logging into the join optimizer
rule to give more detail into how it makes the decisions of join
ordering.

Also, the RuleEventLogger is now added in trace mode which shows
which rules get attempted and applied. Within Calcite, the RuleEventLogger
logs at DEBUG level, but it is quite verbose, so the TRACE wrapper
within CalciteOptimizer needs to be applied to see the information.

Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
---
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaRelMdNonCumulativeCost.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java
3 files changed, 96 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/24183/6
-- 
To view, visit http://gerrit.cloudera.org:8080/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 6
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>

[Impala-ASF-CR] IMPALA-14886: Add trace information for Calcite.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24183 )

Change subject: IMPALA-14886: Add trace information for Calcite.
......................................................................


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/24183/4/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java:

http://gerrit.cloudera.org:8080/#/c/24183/4/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java@114
PS4, Line 114:   protected static final Logger LOG = LoggerFactory.getLogger(ImpalaLoptOptimizeJoinRule.class.getName());
line too long (106 > 90)



-- 
To view, visit http://gerrit.cloudera.org:8080/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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-Comment-Date: Mon, 13 Apr 2026 16:48:19 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-14886: Add trace information into Calcite Join rule

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/24183

to look at the new patch set (#3).

Change subject: IMPALA-14886: Add trace information into Calcite Join rule
......................................................................

IMPALA-14886: Add trace information into Calcite Join rule

This commit adds some trace level logging into the join optimizer
rule to give more detail into how it makes the decisions of join
ordering.

Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
---
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaRelMdNonCumulativeCost.java
2 files changed, 113 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/24183/3
-- 
To view, visit http://gerrit.cloudera.org:8080/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-14886: Add trace information for Calcite.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24183 )

Change subject: IMPALA-14886: Add trace information for Calcite.
......................................................................


Patch Set 7:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/22309/ : 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/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 7
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 May 2026 16:37:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14886: Add trace information for Calcite.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24183 )

Change subject: IMPALA-14886: Add trace information for Calcite.
......................................................................


Patch Set 8:

Build Failed 

https://jenkins.impala.io/job/gerrit-code-review-checks/22313/ : Initial code review checks failed. See linked job for details on the failure.


-- 
To view, visit http://gerrit.cloudera.org:8080/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 8
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 May 2026 18:22:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14886: Add trace information for Calcite.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24183 )

Change subject: IMPALA-14886: Add trace information for Calcite.
......................................................................


Patch Set 8:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/13500/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 8
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 May 2026 12:18:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14886: Add trace information for Calcite.

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Joe McDonnell, Michael Smith, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/24183

to look at the new patch set (#4).

Change subject: IMPALA-14886: Add trace information for Calcite.
......................................................................

IMPALA-14886: Add trace information for Calcite.

This commit adds some trace level logging into the join optimizer
rule to give more detail into how it makes the decisions of join
ordering.

Also, the RuleEventLogger is now added in trace mode which shows
which rules get attempted and applied. Within Calcite, the RuleEventLogger
logs at DEBUG level, but it is quite verbose, so the TRACE wrapper
within CalciteOptimizer needs to be applied to see the information.

Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
---
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaRelMdNonCumulativeCost.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java
3 files changed, 119 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/24183/4
-- 
To view, visit http://gerrit.cloudera.org:8080/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>

[Impala-ASF-CR] IMPALA-14886: Add trace information into Calcite Join rule

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24183 )

Change subject: IMPALA-14886: Add trace information into Calcite Join rule
......................................................................


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/24183/3/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java:

http://gerrit.cloudera.org:8080/#/c/24183/3/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java@114
PS3, Line 114:   protected static final Logger LOG = LoggerFactory.getLogger(ImpalaLoptOptimizeJoinRule.class.getName());
line too long (106 > 90)



-- 
To view, visit http://gerrit.cloudera.org:8080/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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-Comment-Date: Mon, 13 Apr 2026 13:05:33 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-14886: Add trace information for Calcite.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24183 )

Change subject: IMPALA-14886: Add trace information for Calcite.
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/22285/ : 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/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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-Comment-Date: Thu, 30 Apr 2026 19:07:07 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14886: Add trace information for Calcite.

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24183 )

Change subject: IMPALA-14886: Add trace information for Calcite.
......................................................................


Patch Set 5:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/24183/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java:

http://gerrit.cloudera.org:8080/#/c/24183/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java@1012
PS5, Line 1012:         LOG.trace("cost for pushdown tree is " + costPushDown);
> nit: could avoid isTraceEnabled with slf4j substitution
Done


http://gerrit.cloudera.org:8080/#/c/24183/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java@1853
PS5, Line 1853:       if (LOG.isTraceEnabled()) {
> This is a pointless check.
Done


http://gerrit.cloudera.org:8080/#/c/24183/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaRelMdNonCumulativeCost.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaRelMdNonCumulativeCost.java:

http://gerrit.cloudera.org:8080/#/c/24183/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaRelMdNonCumulativeCost.java@106
PS5, Line 106:       LOG.trace("calculating join cost, left row count = " + leftRCount +
> Use substitution and remove isTraceEnabled. We only use the guards to preve
Done


http://gerrit.cloudera.org:8080/#/c/24183/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaRelMdNonCumulativeCost.java@137
PS5, Line 137:           getTableName(scan) + ", cardinality = " +
> How about overriding toString?
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 May 2026 16:13:57 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-14886: Add trace information for Calcite.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24183 )

Change subject: IMPALA-14886: Add trace information for Calcite.
......................................................................


Patch Set 8: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 8
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 May 2026 16:34:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14886: Add trace information into Calcite Join rule

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/24183

to look at the new patch set (#2).

Change subject: IMPALA-14886: Add trace information into Calcite Join rule
......................................................................

IMPALA-14886: Add trace information into Calcite Join rule

This commit adds some trace level logging into the join optimizer
rule to give more detail into how it makes the decisions of join
ordering.

Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
---
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaLoptOptimizeJoinRule.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaRelMdNonCumulativeCost.java
2 files changed, 111 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/24183/2
-- 
To view, visit http://gerrit.cloudera.org:8080/24183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0d05dba235e7f6973fd8ea44dfe98d271835c9aa
Gerrit-Change-Number: 24183
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>