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/07/15 01:52:17 UTC

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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


Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................

IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

This commit supports the Iceberg time travel feature for queries.
Iceberg uses either the "FOR SYSTEM_TIME AS OF" or "FOR
SYSTEM_VERSION AS OF" sql phrase to retrieve data (with its proper
schema) at a given time.

Calcite does provide some code to support snapshots with its
SqlSnapshotNode (validation time) and LogicalSnapshot (optimization
time) objects. However, the Calcite implementation did not quite
fit the architecture used by Impala.

The SqlSnapshot object has a timestamp type "Period" and directly
creates a LogicalSnapshot RelNode on top of a LogicalTableScan. The
reason this didn't work for Impala is because:
  - Iceberg works off a long value of snapshotId. While a timestamp
    can be provided at query time, sometimes it is the snapshotId
    itself that is provided through the SYSTEM_VERSION feature.
  - Impala creates a pseudo-catalog table at analysis time called
    ImpalaTimeTravelTable. This table holds the schema at the given
    time instance for the table. The problem for Calcite is that this
    schema needs to be known at validation time to handle validation
    steps like expanding a select star operation. When Calcite calls
    back into Impala, it needs a unique table name to fetch this.

Some creative engineering was needed to handled these issues. The
architecture here is most likely safe from Calcite upgrades.

Highlights of these changes include:

- In the Parser.jj file, the Impala TimeTravelSpec object is created.
A JavaCCParserUtils utility class helps grab the special token after
the "AS OF" keyword. The Impala CUP parser has the code which parses
the time travel SQL. So some code was added to the CUP parser which
allows the object to be created, located in the ".cup" file and the
Parser java file.

- The SqlSnapshot Calcite class was extended (ImpalaSnapshotSqlNode
to hold onto the Impala specific TimeTravelSpec. The Period variable
is ignored. The tableRef name contains the table name concatenated
with "_tt_<TimeTravelSpec hash code>" to generate a unique table name
that will be found in the CalciteCatalogReader in the validation
stage.

- The CalciteMetadataHandler.TableVisitor is still called after the
parsing and before the validation. It walks through the SqlNode tree.
It still tracks all the tables as before. When it encounters the
ImpalaSnapshotSqlNode, it keeps a map of a table with a list of
TimeTravelSpecs which need a catalog table object. For normal tables
without a TimeTravelSpec, the list will contain a "null" for the
TimeTravelSpec. Duplicates are ok here as will be discussed in the
next bullet point.

- While there may be duplicates, the TableName objects duplicates
are removed via the TableVisitor.getTableNames() by the caller of
the visitor.  These unique table names eventually get into the
StatementMetadataLoader class.

- The CalciteMetadataHandler.populateCalciteSchema is a callback
from the Impala framework called by the StatementMetadataLoader
before the Calcite validation step. In this phase, the tables get
loaded from catalogd. At this point, the Calcite schema gets loaded
with all the catalogd objects. For time travel objects, as mentioned
earlier, an IcebergTimeTravelTable object gets created. There may
be some TimeTravel tables that are duplicates, but it is not worth
the coding headache to make them unique since they only exist at
analysis time. The unique time travel name uses the same TimeTravelSpec
hash code in the ImpalaSnapshotSqlNode object which is how Calcite
can fetch the correct schema.

- At Logical node creation time, a LogicalSnapshot node is created.
This is not needed by Impala since the LogicalTableScan object will
contain the CalciteIcebergTable which contains the
IcebergTimeTravelTable, so it is removed from the logical RelNode
tree via the RemoveSnapshotRule.

- The existing ScanNodeHelper interface has a new getTimeTravelSpec
method which allows the IcebergScanPlanner to fetch the
TimeTravelSpec.

Testing: This was run against the time_travel e2e test in
test_iceberg.py and manually checked to ensure that there was never
any fallback. In a later commit, there will be tests to ensure that
fallback does not happen.

Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
---
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/Parser.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelper.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelperImpl.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M java/calcite-planner/src/main/codegen/templates/Parser.jj
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/RemoveSnapshotRule.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteIcebergTable.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/service/CalciteAnalysisDriver.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteParsedStatement.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/util/JavaCCParserUtils.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/validate/ImpalaSnapshotSqlNode.java
19 files changed, 471 insertions(+), 61 deletions(-)



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

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

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Zoltan Borok-Nagy, Peter Rozsa, Joe McDonnell, Michael Smith, Noemi Pap-Takacs, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................

IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

This commit supports the Iceberg time travel feature for queries.
Iceberg uses either the "FOR SYSTEM_TIME AS OF" or "FOR
SYSTEM_VERSION AS OF" sql phrase to retrieve data (with its proper
schema) at a given time.

Calcite does provide some code to support snapshots with its
SqlSnapshotNode (validation time) and LogicalSnapshot (optimization
time) objects. However, the Calcite implementation did not quite
fit the architecture used by Impala.

The SqlSnapshot object has a timestamp type "Period" and directly
creates a LogicalSnapshot RelNode on top of a LogicalTableScan. The
reason this didn't work for Impala is because:
  - Iceberg works off a long value of snapshotId. While a timestamp
    can be provided at query time, sometimes it is the snapshotId
    itself that is provided through the SYSTEM_VERSION feature.
  - Impala creates a pseudo-catalog table at analysis time called
    ImpalaTimeTravelTable. This table holds the schema at the given
    time instance for the table. The problem for Calcite is that this
    schema needs to be known at validation time to handle validation
    steps like expanding a select star operation. When Calcite calls
    back into Impala, it needs a unique table name to fetch this.

Some creative engineering was needed to handled these issues. The
architecture here is most likely safe from Calcite upgrades.

Highlights of these changes include:

- In the Parser.jj file, the Impala TimeTravelSpec object is created.
A JavaCCParserUtils utility class helps grab the special token after
the "AS OF" keyword. The Impala CUP parser has the code which parses
the time travel SQL. So some code was added to the CUP parser which
allows the object to be created, located in the ".cup" file and the
Parser java file.

- The SqlSnapshot Calcite class was extended (ImpalaSnapshotSqlNode
to hold onto the Impala specific TimeTravelSpec. The Period variable
is ignored. The tableRef name contains the table name concatenated
with "_tt_<TimeTravelSpec hash code>" to generate a unique table name
that will be found in the CalciteCatalogReader in the validation
stage.

- The CalciteMetadataHandler.TableVisitor is still called after the
parsing and before the validation. It walks through the SqlNode tree.
It still tracks all the tables as before. When it encounters the
ImpalaSnapshotSqlNode, it keeps a map of a table with a list of
TimeTravelSpecs which need a catalog table object. For normal tables
without a TimeTravelSpec, the list will contain a "null" for the
TimeTravelSpec. Duplicates are ok here as will be discussed in the
next bullet point.

- While there may be duplicates, the TableName objects duplicates
are removed via the TableVisitor.getTableNames() by the caller of
the visitor.  These unique table names eventually get into the
StatementMetadataLoader class.

- The CalciteMetadataHandler.populateCalciteSchema is a callback
from the Impala framework called by the StatementMetadataLoader
before the Calcite validation step. In this phase, the tables get
loaded from catalogd. At this point, the Calcite schema gets loaded
with all the catalogd objects. For time travel objects, as mentioned
earlier, an IcebergTimeTravelTable object gets created. There may
be some TimeTravel tables that are duplicates, but it is not worth
the coding headache to make them unique since they only exist at
analysis time. The unique time travel name uses the same TimeTravelSpec
hash code in the ImpalaSnapshotSqlNode object which is how Calcite
can fetch the correct schema.

- At Logical node creation time, a LogicalSnapshot node is created.
This is not needed by Impala since the LogicalTableScan object will
contain the CalciteIcebergTable which contains the
IcebergTimeTravelTable, so it is removed from the logical RelNode
tree via the RemoveSnapshotRule.

- The existing ScanNodeHelper interface has a new getTimeTravelSpec
method which allows the IcebergScanPlanner to fetch the
TimeTravelSpec.

Testing: This was run against the time_travel e2e test in
test_iceberg.py and manually checked to ensure that there was never
any fallback. In a later commit, there will be tests to ensure that
fallback does not happen.

Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
---
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/Parser.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelper.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelperImpl.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M java/calcite-planner/src/main/codegen/templates/Parser.jj
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/RemoveSnapshotRule.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteIcebergTable.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/service/CalciteAnalysisDriver.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteParsedStatement.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/util/JavaCCParserUtils.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/validate/ImpalaSnapshotSqlNode.java
19 files changed, 467 insertions(+), 61 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/24577/10
-- 
To view, visit http://gerrit.cloudera.org:8080/24577
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 10
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 21:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 21
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Aug 2026 21:07:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 15:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 15
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Jul 2026 12:44:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 20: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/13872/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 20
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Aug 2026 18:36:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Zoltan Borok-Nagy, Peter Rozsa, Joe McDonnell, Michael Smith, Noemi Pap-Takacs, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................

IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

This commit supports the Iceberg time travel feature for queries.
Iceberg uses either the "FOR SYSTEM_TIME AS OF" or "FOR
SYSTEM_VERSION AS OF" sql phrase to retrieve data (with its proper
schema) at a given time.

Calcite does provide some code to support snapshots with its
SqlSnapshotNode (validation time) and LogicalSnapshot (optimization
time) objects. However, the Calcite implementation did not quite
fit the architecture used by Impala.

The SqlSnapshot object has a timestamp type "Period" and directly
creates a LogicalSnapshot RelNode on top of a LogicalTableScan. The
reason this didn't work for Impala is because:
  - Iceberg works off a long value of snapshotId. While a timestamp
    can be provided at query time, sometimes it is the snapshotId
    itself that is provided through the SYSTEM_VERSION feature.
  - Impala creates a pseudo-catalog table at analysis time called
    ImpalaTimeTravelTable. This table holds the schema at the given
    time instance for the table. The problem for Calcite is that this
    schema needs to be known at validation time to handle validation
    steps like expanding a select star operation. When Calcite calls
    back into Impala, it needs a unique table name to fetch this.

Some creative engineering was needed to handled these issues. The
architecture here is most likely safe from Calcite upgrades.

Highlights of these changes include:

- In the Parser.jj file, the Impala TimeTravelSpec object is created.
A JavaCCParserUtils utility class helps grab the special token after
the "AS OF" keyword. The Impala CUP parser has the code which parses
the time travel SQL. So some code was added to the CUP parser which
allows the object to be created, located in the ".cup" file and the
Parser java file.

- The SqlSnapshot Calcite class was extended (ImpalaSnapshotSqlNode
to hold onto the Impala specific TimeTravelSpec. The Period variable
is ignored. The tableRef name contains the table name concatenated
with "_tt_<TimeTravelSpec hash code>" to generate a unique table name
that will be found in the CalciteCatalogReader in the validation
stage.

- The CalciteMetadataHandler.TableVisitor is still called after the
parsing and before the validation. It walks through the SqlNode tree.
It still tracks all the tables as before. When it encounters the
ImpalaSnapshotSqlNode, it keeps a map of a table with a list of
TimeTravelSpecs which need a catalog table object. For normal tables
without a TimeTravelSpec, the list will contain a "null" for the
TimeTravelSpec. Duplicates are ok here as will be discussed in the
next bullet point.

- While there may be duplicates, the TableName objects duplicates
are removed via the TableVisitor.getTableNames() by the caller of
the visitor.  These unique table names eventually get into the
StatementMetadataLoader class.

- The CalciteMetadataHandler.populateCalciteSchema is a callback
from the Impala framework called by the StatementMetadataLoader
before the Calcite validation step. In this phase, the tables get
loaded from catalogd. At this point, the Calcite schema gets loaded
with all the catalogd objects. For time travel objects, as mentioned
earlier, an IcebergTimeTravelTable object gets created. There may
be some TimeTravel tables that are duplicates, but it is not worth
the coding headache to make them unique since they only exist at
analysis time. The unique time travel name uses the same TimeTravelSpec
hash code in the ImpalaSnapshotSqlNode object which is how Calcite
can fetch the correct schema.

- At Logical node creation time, a LogicalSnapshot node is created.
This is not needed by Impala since the LogicalTableScan object will
contain the CalciteIcebergTable which contains the
IcebergTimeTravelTable, so it is removed from the logical RelNode
tree via the RemoveSnapshotRule.

- The existing ScanNodeHelper interface has a new getTimeTravelSpec
method which allows the IcebergScanPlanner to fetch the
TimeTravelSpec.

Testing: This was run against the time_travel e2e test in
test_iceberg.py and manually checked to ensure that there was never
any fallback. In a later commit, there will be tests to ensure that
fallback does not happen.

Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
---
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/Parser.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelper.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelperImpl.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M java/calcite-planner/src/main/codegen/templates/Parser.jj
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/RemoveSnapshotRule.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteIcebergTable.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/service/CalciteAnalysisDriver.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteParsedStatement.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/util/JavaCCParserUtils.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/validate/ImpalaSnapshotSqlNode.java
19 files changed, 481 insertions(+), 62 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/24577/11
-- 
To view, visit http://gerrit.cloudera.org:8080/24577
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 11
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 10:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 10
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sun, 19 Jul 2026 00:08:15 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 12:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 12
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sun, 19 Jul 2026 16:20:29 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 19: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/13869/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 19
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Aug 2026 04:56:02 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 7:

(3 comments)

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/24577/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java:

http://gerrit.cloudera.org:8080/#/c/24577/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java@36
PS7, Line 36: import org.apache.commons.lang3.tuple.Pair;
unused import: org.apache.commons.lang3.tuple.Pair


http://gerrit.cloudera.org:8080/#/c/24577/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/validate/ImpalaSnapshotSqlNode.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/validate/ImpalaSnapshotSqlNode.java:

http://gerrit.cloudera.org:8080/#/c/24577/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/validate/ImpalaSnapshotSqlNode.java@28
PS7, Line 28: import org.slf4j.Logger;
unused import: org.slf4j.Logger


http://gerrit.cloudera.org:8080/#/c/24577/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/validate/ImpalaSnapshotSqlNode.java@29
PS7, Line 29: import org.slf4j.LoggerFactory;
unused import: org.slf4j.LoggerFactory



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
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: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 15 Jul 2026 01:54:29 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 7:

Build Failed 

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
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: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 15 Jul 2026 02:05:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Aleksandr Efimov, Zoltan Borok-Nagy, Peter Rozsa, Joe McDonnell, Michael Smith, Noemi Pap-Takacs, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................

IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

This commit supports the Iceberg time travel feature for queries.
Iceberg uses either the "FOR SYSTEM_TIME AS OF" or "FOR
SYSTEM_VERSION AS OF" sql phrase to retrieve data (with its proper
schema) at a given time.

Calcite does provide some code to support snapshots with its
SqlSnapshotNode (validation time) and LogicalSnapshot (optimization
time) objects. However, the Calcite implementation did not quite
fit the architecture used by Impala.

The SqlSnapshot object has a timestamp type "Period" and directly
creates a LogicalSnapshot RelNode on top of a LogicalTableScan. The
reason this didn't work for Impala is because:
  - Iceberg works off a long value of snapshotId. While a timestamp
    can be provided at query time, sometimes it is the snapshotId
    itself that is provided through the SYSTEM_VERSION feature.
  - Impala creates a pseudo-catalog table at analysis time called
    IcebergTimeTravelTable. This table holds the schema at the given
    time instance for the table. The problem for Calcite is that this
    schema needs to be known at validation time to handle validation
    steps like expanding a select star operation. When Calcite calls
    back into Impala, it needs a unique table name to fetch this.

Some creative engineering was needed to handled these issues. The
architecture here is most likely safe from Calcite upgrades.

Highlights of these changes include:

- In the Parser.jj file, the Impala TimeTravelSpec object is created.
A JavaCCParserUtils utility class helps grab the special token after
the "AS OF" keyword. The Impala CUP parser has the code which parses
the time travel SQL. So some code was added to the CUP parser which
allows the object to be created, located in the ".cup" file and the
Parser java file.

- The SqlSnapshot Calcite class was extended (ImpalaSnapshotSqlNode
to hold onto the Impala specific TimeTravelSpec. The Period variable
is ignored. The tableRef name contains the table name concatenated
with "_tt_<TimeTravelSpec hash code>" to generate a unique table name
that will be found in the CalciteCatalogReader in the validation
stage.

- The CalciteMetadataHandler.TableVisitor is still called after the
parsing and before the validation. It walks through the SqlNode tree.
It still tracks all the tables as before. When it encounters the
ImpalaSnapshotSqlNode, it keeps a map of a table with a list of
TimeTravelSpecs which need a catalog table object. For normal tables
without a TimeTravelSpec, the list will contain a "null" for the
TimeTravelSpec. Duplicates are ok here as will be discussed in the
next bullet point.

- While there may be duplicates, the TableName objects duplicates
are removed via the TableVisitor.getTableNames() by the caller of
the visitor.  These unique table names eventually get into the
StatementMetadataLoader class.

- The CalciteMetadataHandler.populateCalciteSchema is a callback
from the Impala framework called by the StatementMetadataLoader
before the Calcite validation step. In this phase, the tables get
loaded from catalogd. At this point, the Calcite schema gets loaded
with all the catalogd objects. For time travel objects, as mentioned
earlier, an IcebergTimeTravelTable object gets created. There may
be some TimeTravel tables that are duplicates, but it is not worth
the coding headache to make them unique since they only exist at
analysis time. The unique time travel name uses the same TimeTravelSpec
hash code in the ImpalaSnapshotSqlNode object which is how Calcite
can fetch the correct schema.

- At Logical node creation time, a LogicalSnapshot node is created.
This is not needed by Impala since the LogicalTableScan object will
contain the CalciteIcebergTable which contains the
IcebergTimeTravelTable, so it is removed from the logical RelNode
tree via the RemoveSnapshotRule.

- The existing ScanNodeHelper interface has a new getTimeTravelSpec
method which allows the IcebergScanPlanner to fetch the
TimeTravelSpec.

Testing: This was run against the time_travel e2e test in
test_iceberg.py and manually checked to ensure that there was never
any fallback. In a later commit, there will be tests to ensure that
fallback does not happen.

Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
---
M common/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/Parser.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelper.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelperImpl.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M java/calcite-planner/src/main/codegen/templates/Parser.jj
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/RemoveSnapshotRule.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteIcebergTable.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/service/CalciteAnalysisDriver.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteParsedStatement.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/util/JavaCCParserUtils.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/validate/ImpalaSnapshotSqlNode.java
A java/calcite-planner/src/test/java/org/apache/impala/calcite/util/JavaCCParserUtilTest.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-scan-metrics-with-deletes.test
22 files changed, 632 insertions(+), 72 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/24577/21
-- 
To view, visit http://gerrit.cloudera.org:8080/24577
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 21
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 20:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 20
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Aug 2026 13:46:35 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Aleksandr Efimov, Zoltan Borok-Nagy, Peter Rozsa, Joe McDonnell, Michael Smith, Noemi Pap-Takacs, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................

IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

This commit supports the Iceberg time travel feature for queries.
Iceberg uses either the "FOR SYSTEM_TIME AS OF" or "FOR
SYSTEM_VERSION AS OF" sql phrase to retrieve data (with its proper
schema) at a given time.

Calcite does provide some code to support snapshots with its
SqlSnapshotNode (validation time) and LogicalSnapshot (optimization
time) objects. However, the Calcite implementation did not quite
fit the architecture used by Impala.

The SqlSnapshot object has a timestamp type "Period" and directly
creates a LogicalSnapshot RelNode on top of a LogicalTableScan. The
reason this didn't work for Impala is because:
  - Iceberg works off a long value of snapshotId. While a timestamp
    can be provided at query time, sometimes it is the snapshotId
    itself that is provided through the SYSTEM_VERSION feature.
  - Impala creates a pseudo-catalog table at analysis time called
    IcebergTimeTravelTable. This table holds the schema at the given
    time instance for the table. The problem for Calcite is that this
    schema needs to be known at validation time to handle validation
    steps like expanding a select star operation. When Calcite calls
    back into Impala, it needs a unique table name to fetch this.

Some creative engineering was needed to handled these issues. The
architecture here is most likely safe from Calcite upgrades.

Highlights of these changes include:

- In the Parser.jj file, the Impala TimeTravelSpec object is created.
A JavaCCParserUtils utility class helps grab the special token after
the "AS OF" keyword. The Impala CUP parser has the code which parses
the time travel SQL. So some code was added to the CUP parser which
allows the object to be created, located in the ".cup" file and the
Parser java file.

- The SqlSnapshot Calcite class was extended (ImpalaSnapshotSqlNode
to hold onto the Impala specific TimeTravelSpec. The Period variable
is ignored. The tableRef name contains the table name concatenated
with "_tt_<TimeTravelSpec hash code>" to generate a unique table name
that will be found in the CalciteCatalogReader in the validation
stage.

- The CalciteMetadataHandler.TableVisitor is still called after the
parsing and before the validation. It walks through the SqlNode tree.
It still tracks all the tables as before. When it encounters the
ImpalaSnapshotSqlNode, it keeps a map of a table with a list of
TimeTravelSpecs which need a catalog table object. For normal tables
without a TimeTravelSpec, the list will contain a "null" for the
TimeTravelSpec. Duplicates are ok here as will be discussed in the
next bullet point.

- While there may be duplicates, the TableName objects duplicates
are removed via the TableVisitor.getTableNames() by the caller of
the visitor.  These unique table names eventually get into the
StatementMetadataLoader class.

- The CalciteMetadataHandler.populateCalciteSchema is a callback
from the Impala framework called by the StatementMetadataLoader
before the Calcite validation step. In this phase, the tables get
loaded from catalogd. At this point, the Calcite schema gets loaded
with all the catalogd objects. For time travel objects, as mentioned
earlier, an IcebergTimeTravelTable object gets created. There may
be some TimeTravel tables that are duplicates, but it is not worth
the coding headache to make them unique since they only exist at
analysis time. The unique time travel name uses the same TimeTravelSpec
hash code in the ImpalaSnapshotSqlNode object which is how Calcite
can fetch the correct schema.

- At Logical node creation time, a LogicalSnapshot node is created.
This is not needed by Impala since the LogicalTableScan object will
contain the CalciteIcebergTable which contains the
IcebergTimeTravelTable, so it is removed from the logical RelNode
tree via the RemoveSnapshotRule.

- The existing ScanNodeHelper interface has a new getTimeTravelSpec
method which allows the IcebergScanPlanner to fetch the
TimeTravelSpec.

Testing: This was run against the time_travel e2e test in
test_iceberg.py and manually checked to ensure that there was never
any fallback. In a later commit, there will be tests to ensure that
fallback does not happen.

Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
---
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/Parser.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelper.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelperImpl.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M java/calcite-planner/src/main/codegen/templates/Parser.jj
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/RemoveSnapshotRule.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteIcebergTable.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/service/CalciteAnalysisDriver.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteParsedStatement.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/util/JavaCCParserUtils.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/validate/ImpalaSnapshotSqlNode.java
A java/calcite-planner/src/test/java/org/apache/impala/calcite/util/JavaCCParserUtilTest.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-scan-metrics-with-deletes.test
M tests/query_test/test_iceberg.py
23 files changed, 628 insertions(+), 73 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/24577/17
-- 
To view, visit http://gerrit.cloudera.org:8080/24577
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 17
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Aleksandr Efimov, Zoltan Borok-Nagy, Peter Rozsa, Joe McDonnell, Michael Smith, Noemi Pap-Takacs, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................

IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

This commit supports the Iceberg time travel feature for queries.
Iceberg uses either the "FOR SYSTEM_TIME AS OF" or "FOR
SYSTEM_VERSION AS OF" sql phrase to retrieve data (with its proper
schema) at a given time.

Calcite does provide some code to support snapshots with its
SqlSnapshotNode (validation time) and LogicalSnapshot (optimization
time) objects. However, the Calcite implementation did not quite
fit the architecture used by Impala.

The SqlSnapshot object has a timestamp type "Period" and directly
creates a LogicalSnapshot RelNode on top of a LogicalTableScan. The
reason this didn't work for Impala is because:
  - Iceberg works off a long value of snapshotId. While a timestamp
    can be provided at query time, sometimes it is the snapshotId
    itself that is provided through the SYSTEM_VERSION feature.
  - Impala creates a pseudo-catalog table at analysis time called
    IcebergTimeTravelTable. This table holds the schema at the given
    time instance for the table. The problem for Calcite is that this
    schema needs to be known at validation time to handle validation
    steps like expanding a select star operation. When Calcite calls
    back into Impala, it needs a unique table name to fetch this.

Some creative engineering was needed to handled these issues. The
architecture here is most likely safe from Calcite upgrades.

Highlights of these changes include:

- In the Parser.jj file, the Impala TimeTravelSpec object is created.
A JavaCCParserUtils utility class helps grab the special token after
the "AS OF" keyword. The Impala CUP parser has the code which parses
the time travel SQL. So some code was added to the CUP parser which
allows the object to be created, located in the ".cup" file and the
Parser java file.

- The SqlSnapshot Calcite class was extended (ImpalaSnapshotSqlNode
to hold onto the Impala specific TimeTravelSpec. The Period variable
is ignored. The tableRef name contains the table name concatenated
with "_tt_<TimeTravelSpec hash code>" to generate a unique table name
that will be found in the CalciteCatalogReader in the validation
stage.

- The CalciteMetadataHandler.TableVisitor is still called after the
parsing and before the validation. It walks through the SqlNode tree.
It still tracks all the tables as before. When it encounters the
ImpalaSnapshotSqlNode, it keeps a map of a table with a list of
TimeTravelSpecs which need a catalog table object. For normal tables
without a TimeTravelSpec, the list will contain a "null" for the
TimeTravelSpec. Duplicates are ok here as will be discussed in the
next bullet point.

- While there may be duplicates, the TableName objects duplicates
are removed via the TableVisitor.getTableNames() by the caller of
the visitor.  These unique table names eventually get into the
StatementMetadataLoader class.

- The CalciteMetadataHandler.populateCalciteSchema is a callback
from the Impala framework called by the StatementMetadataLoader
before the Calcite validation step. In this phase, the tables get
loaded from catalogd. At this point, the Calcite schema gets loaded
with all the catalogd objects. For time travel objects, as mentioned
earlier, an IcebergTimeTravelTable object gets created. There may
be some TimeTravel tables that are duplicates, but it is not worth
the coding headache to make them unique since they only exist at
analysis time. The unique time travel name uses the same TimeTravelSpec
hash code in the ImpalaSnapshotSqlNode object which is how Calcite
can fetch the correct schema.

- At Logical node creation time, a LogicalSnapshot node is created.
This is not needed by Impala since the LogicalTableScan object will
contain the CalciteIcebergTable which contains the
IcebergTimeTravelTable, so it is removed from the logical RelNode
tree via the RemoveSnapshotRule.

- The existing ScanNodeHelper interface has a new getTimeTravelSpec
method which allows the IcebergScanPlanner to fetch the
TimeTravelSpec.

Testing: This was run against the time_travel e2e test in
test_iceberg.py and manually checked to ensure that there was never
any fallback. In a later commit, there will be tests to ensure that
fallback does not happen.

Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
---
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/Parser.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelper.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelperImpl.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M java/calcite-planner/src/main/codegen/templates/Parser.jj
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/RemoveSnapshotRule.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteIcebergTable.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/service/CalciteAnalysisDriver.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteParsedStatement.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/util/JavaCCParserUtils.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/validate/ImpalaSnapshotSqlNode.java
A java/calcite-planner/src/test/java/org/apache/impala/calcite/util/JavaCCParserUtilTest.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-scan-metrics-with-deletes.test
M tests/query_test/test_iceberg.py
23 files changed, 637 insertions(+), 73 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/24577/18
-- 
To view, visit http://gerrit.cloudera.org:8080/24577
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 18
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: restore
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 12
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 19:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 19
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Aug 2026 23:39:20 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 19:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 19
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Aug 2026 00:09:05 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 15:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/24577/15/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java:

http://gerrit.cloudera.org:8080/#/c/24577/15/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java@152
PS15, Line 152:             String timeTravelTableKey = lowerCaseTableName + "_tt_" + tts.hashCode();
TimeTravelSpec doesn't override equals()/hashCode() (nor does StmtNode), so this is the identity hash, and the synthetic name isn't reproducible: the same query text produces a different table key on every compilation. getQualifiedName() puts this string into the RelOptTable qualified name, so it lands in RelNode digests and logical-plan dumps - anything comparing plans textually (debug logs, future plan tests, materialization matching by qualified name) sees a fresh name each run.

A second effect: two references to the same table with an equal spec in one query (self-join `t FOR SYSTEM_VERSION AS OF 123 join t FOR SYSTEM_VERSION AS OF 123`) create two distinct spec objects, hence two unrelated Calcite tables, and the containsKey() dedup in addTimeTravelTable() can effectively never fire since equal specs never share a key.

Could TimeTravelSpec get equals/hashCode over (kind, asOfVersion, asOfMicros), or the key be derived from the resolved snapshot id / travel timestamp? A deterministic name would also help any external consumer of the logical plan, where stable table identity is part of the contract.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 15
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 17 Aug 2026 08:27:45 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 16:

(1 comment)

This time I read the whole patch rather than just the naming thread. The split between resolving the snapshot at validation time and having the planner read the spec off the helper reads nicely, and the non-Calcite path still behaves as before - SingleNodePlanner passes tblRef.getTimeTravelSpec() into ScanNodeHelperImpl and only IcebergScanPlanner consumes it. One question below, then I'm happy to +1.

http://gerrit.cloudera.org:8080/#/c/24577/16/java/calcite-planner/src/main/java/org/apache/impala/calcite/util/JavaCCParserUtils.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/util/JavaCCParserUtils.java:

http://gerrit.cloudera.org:8080/#/c/24577/16/java/calcite-planner/src/main/java/org/apache/impala/calcite/util/JavaCCParserUtils.java@67
PS16, Line 67:         currentAbsoluteIndex += lineLength + 1;
Small thing about the arithmetic here: JavaCC treats \r\n as a single line break (SimpleCharStream skips the line increment for the \n after a \r), but the string still holds two characters. So for a statement that arrives with CRLF, startIndex ends up one character short per preceding line and the extracted token shifts left.

The reason I mention it is that it can pass silently rather than fail: in a multi-line "... FOR SYSTEM_VERSION AS OF 12345", a one-character shift gives " 1234", which parses happily and picks a different snapshot.

Would normalizing the line endings first be enough, e.g. extracting from originalSource.replace("\r\n", "\n")? JavaCC's positions already treat CRLF as one break. And if something upstream normalizes the statement before SqlParser.create(), just ignore me.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 16
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Aug 2026 20:15:05 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 16:

Build Failed 

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 16
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Aug 2026 14:00:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 12
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Zoltan Borok-Nagy, Peter Rozsa, Joe McDonnell, Michael Smith, Noemi Pap-Takacs, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................

IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

This commit supports the Iceberg time travel feature for queries.
Iceberg uses either the "FOR SYSTEM_TIME AS OF" or "FOR
SYSTEM_VERSION AS OF" sql phrase to retrieve data (with its proper
schema) at a given time.

Calcite does provide some code to support snapshots with its
SqlSnapshotNode (validation time) and LogicalSnapshot (optimization
time) objects. However, the Calcite implementation did not quite
fit the architecture used by Impala.

The SqlSnapshot object has a timestamp type "Period" and directly
creates a LogicalSnapshot RelNode on top of a LogicalTableScan. The
reason this didn't work for Impala is because:
  - Iceberg works off a long value of snapshotId. While a timestamp
    can be provided at query time, sometimes it is the snapshotId
    itself that is provided through the SYSTEM_VERSION feature.
  - Impala creates a pseudo-catalog table at analysis time called
    ImpalaTimeTravelTable. This table holds the schema at the given
    time instance for the table. The problem for Calcite is that this
    schema needs to be known at validation time to handle validation
    steps like expanding a select star operation. When Calcite calls
    back into Impala, it needs a unique table name to fetch this.

Some creative engineering was needed to handled these issues. The
architecture here is most likely safe from Calcite upgrades.

Highlights of these changes include:

- In the Parser.jj file, the Impala TimeTravelSpec object is created.
A JavaCCParserUtils utility class helps grab the special token after
the "AS OF" keyword. The Impala CUP parser has the code which parses
the time travel SQL. So some code was added to the CUP parser which
allows the object to be created, located in the ".cup" file and the
Parser java file.

- The SqlSnapshot Calcite class was extended (ImpalaSnapshotSqlNode
to hold onto the Impala specific TimeTravelSpec. The Period variable
is ignored. The tableRef name contains the table name concatenated
with "_tt_<TimeTravelSpec hash code>" to generate a unique table name
that will be found in the CalciteCatalogReader in the validation
stage.

- The CalciteMetadataHandler.TableVisitor is still called after the
parsing and before the validation. It walks through the SqlNode tree.
It still tracks all the tables as before. When it encounters the
ImpalaSnapshotSqlNode, it keeps a map of a table with a list of
TimeTravelSpecs which need a catalog table object. For normal tables
without a TimeTravelSpec, the list will contain a "null" for the
TimeTravelSpec. Duplicates are ok here as will be discussed in the
next bullet point.

- While there may be duplicates, the TableName objects duplicates
are removed via the TableVisitor.getTableNames() by the caller of
the visitor.  These unique table names eventually get into the
StatementMetadataLoader class.

- The CalciteMetadataHandler.populateCalciteSchema is a callback
from the Impala framework called by the StatementMetadataLoader
before the Calcite validation step. In this phase, the tables get
loaded from catalogd. At this point, the Calcite schema gets loaded
with all the catalogd objects. For time travel objects, as mentioned
earlier, an IcebergTimeTravelTable object gets created. There may
be some TimeTravel tables that are duplicates, but it is not worth
the coding headache to make them unique since they only exist at
analysis time. The unique time travel name uses the same TimeTravelSpec
hash code in the ImpalaSnapshotSqlNode object which is how Calcite
can fetch the correct schema.

- At Logical node creation time, a LogicalSnapshot node is created.
This is not needed by Impala since the LogicalTableScan object will
contain the CalciteIcebergTable which contains the
IcebergTimeTravelTable, so it is removed from the logical RelNode
tree via the RemoveSnapshotRule.

- The existing ScanNodeHelper interface has a new getTimeTravelSpec
method which allows the IcebergScanPlanner to fetch the
TimeTravelSpec.

Testing: This was run against the time_travel e2e test in
test_iceberg.py and manually checked to ensure that there was never
any fallback. In a later commit, there will be tests to ensure that
fallback does not happen.

Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
---
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/Parser.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelper.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelperImpl.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M java/calcite-planner/src/main/codegen/templates/Parser.jj
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/RemoveSnapshotRule.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteIcebergTable.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/service/CalciteAnalysisDriver.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteParsedStatement.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/util/JavaCCParserUtils.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/validate/ImpalaSnapshotSqlNode.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
20 files changed, 488 insertions(+), 62 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/24577/13
-- 
To view, visit http://gerrit.cloudera.org:8080/24577
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 13
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 8:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
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: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 15 Jul 2026 02:38:39 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/24577 )

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 15: Code-Review+1

LGTM!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 15
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 31 Jul 2026 15:14:39 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 21:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 21
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Aug 2026 20:44:18 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 21: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 21
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 21 Aug 2026 01:36:06 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................

IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

This commit supports the Iceberg time travel feature for queries.
Iceberg uses either the "FOR SYSTEM_TIME AS OF" or "FOR
SYSTEM_VERSION AS OF" sql phrase to retrieve data (with its proper
schema) at a given time.

Calcite does provide some code to support snapshots with its
SqlSnapshotNode (validation time) and LogicalSnapshot (optimization
time) objects. However, the Calcite implementation did not quite
fit the architecture used by Impala.

The SqlSnapshot object has a timestamp type "Period" and directly
creates a LogicalSnapshot RelNode on top of a LogicalTableScan. The
reason this didn't work for Impala is because:
  - Iceberg works off a long value of snapshotId. While a timestamp
    can be provided at query time, sometimes it is the snapshotId
    itself that is provided through the SYSTEM_VERSION feature.
  - Impala creates a pseudo-catalog table at analysis time called
    IcebergTimeTravelTable. This table holds the schema at the given
    time instance for the table. The problem for Calcite is that this
    schema needs to be known at validation time to handle validation
    steps like expanding a select star operation. When Calcite calls
    back into Impala, it needs a unique table name to fetch this.

Some creative engineering was needed to handled these issues. The
architecture here is most likely safe from Calcite upgrades.

Highlights of these changes include:

- In the Parser.jj file, the Impala TimeTravelSpec object is created.
A JavaCCParserUtils utility class helps grab the special token after
the "AS OF" keyword. The Impala CUP parser has the code which parses
the time travel SQL. So some code was added to the CUP parser which
allows the object to be created, located in the ".cup" file and the
Parser java file.

- The SqlSnapshot Calcite class was extended (ImpalaSnapshotSqlNode
to hold onto the Impala specific TimeTravelSpec. The Period variable
is ignored. The tableRef name contains the table name concatenated
with "_tt_<TimeTravelSpec hash code>" to generate a unique table name
that will be found in the CalciteCatalogReader in the validation
stage.

- The CalciteMetadataHandler.TableVisitor is still called after the
parsing and before the validation. It walks through the SqlNode tree.
It still tracks all the tables as before. When it encounters the
ImpalaSnapshotSqlNode, it keeps a map of a table with a list of
TimeTravelSpecs which need a catalog table object. For normal tables
without a TimeTravelSpec, the list will contain a "null" for the
TimeTravelSpec. Duplicates are ok here as will be discussed in the
next bullet point.

- While there may be duplicates, the TableName objects duplicates
are removed via the TableVisitor.getTableNames() by the caller of
the visitor.  These unique table names eventually get into the
StatementMetadataLoader class.

- The CalciteMetadataHandler.populateCalciteSchema is a callback
from the Impala framework called by the StatementMetadataLoader
before the Calcite validation step. In this phase, the tables get
loaded from catalogd. At this point, the Calcite schema gets loaded
with all the catalogd objects. For time travel objects, as mentioned
earlier, an IcebergTimeTravelTable object gets created. There may
be some TimeTravel tables that are duplicates, but it is not worth
the coding headache to make them unique since they only exist at
analysis time. The unique time travel name uses the same TimeTravelSpec
hash code in the ImpalaSnapshotSqlNode object which is how Calcite
can fetch the correct schema.

- At Logical node creation time, a LogicalSnapshot node is created.
This is not needed by Impala since the LogicalTableScan object will
contain the CalciteIcebergTable which contains the
IcebergTimeTravelTable, so it is removed from the logical RelNode
tree via the RemoveSnapshotRule.

- The existing ScanNodeHelper interface has a new getTimeTravelSpec
method which allows the IcebergScanPlanner to fetch the
TimeTravelSpec.

Testing: This was run against the time_travel e2e test in
test_iceberg.py and manually checked to ensure that there was never
any fallback. In a later commit, there will be tests to ensure that
fallback does not happen.

Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Reviewed-on: http://gerrit.cloudera.org:8080/24577
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Steve Carlin <sc...@cloudera.com>
---
M common/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/Parser.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelper.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelperImpl.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M java/calcite-planner/src/main/codegen/templates/Parser.jj
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/RemoveSnapshotRule.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteIcebergTable.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/service/CalciteAnalysisDriver.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteParsedStatement.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/util/JavaCCParserUtils.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/validate/ImpalaSnapshotSqlNode.java
A java/calcite-planner/src/test/java/org/apache/impala/calcite/util/JavaCCParserUtilTest.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-scan-metrics-with-deletes.test
22 files changed, 632 insertions(+), 72 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Steve Carlin: Looks good to me, approved

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 22
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 21:

Carried over +2 from the two +1s given.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 21
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 21 Aug 2026 02:29:39 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

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

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

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................

IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

This commit supports the Iceberg time travel feature for queries.
Iceberg uses either the "FOR SYSTEM_TIME AS OF" or "FOR
SYSTEM_VERSION AS OF" sql phrase to retrieve data (with its proper
schema) at a given time.

Calcite does provide some code to support snapshots with its
SqlSnapshotNode (validation time) and LogicalSnapshot (optimization
time) objects. However, the Calcite implementation did not quite
fit the architecture used by Impala.

The SqlSnapshot object has a timestamp type "Period" and directly
creates a LogicalSnapshot RelNode on top of a LogicalTableScan. The
reason this didn't work for Impala is because:
  - Iceberg works off a long value of snapshotId. While a timestamp
    can be provided at query time, sometimes it is the snapshotId
    itself that is provided through the SYSTEM_VERSION feature.
  - Impala creates a pseudo-catalog table at analysis time called
    ImpalaTimeTravelTable. This table holds the schema at the given
    time instance for the table. The problem for Calcite is that this
    schema needs to be known at validation time to handle validation
    steps like expanding a select star operation. When Calcite calls
    back into Impala, it needs a unique table name to fetch this.

Some creative engineering was needed to handled these issues. The
architecture here is most likely safe from Calcite upgrades.

Highlights of these changes include:

- In the Parser.jj file, the Impala TimeTravelSpec object is created.
A JavaCCParserUtils utility class helps grab the special token after
the "AS OF" keyword. The Impala CUP parser has the code which parses
the time travel SQL. So some code was added to the CUP parser which
allows the object to be created, located in the ".cup" file and the
Parser java file.

- The SqlSnapshot Calcite class was extended (ImpalaSnapshotSqlNode
to hold onto the Impala specific TimeTravelSpec. The Period variable
is ignored. The tableRef name contains the table name concatenated
with "_tt_<TimeTravelSpec hash code>" to generate a unique table name
that will be found in the CalciteCatalogReader in the validation
stage.

- The CalciteMetadataHandler.TableVisitor is still called after the
parsing and before the validation. It walks through the SqlNode tree.
It still tracks all the tables as before. When it encounters the
ImpalaSnapshotSqlNode, it keeps a map of a table with a list of
TimeTravelSpecs which need a catalog table object. For normal tables
without a TimeTravelSpec, the list will contain a "null" for the
TimeTravelSpec. Duplicates are ok here as will be discussed in the
next bullet point.

- While there may be duplicates, the TableName objects duplicates
are removed via the TableVisitor.getTableNames() by the caller of
the visitor.  These unique table names eventually get into the
StatementMetadataLoader class.

- The CalciteMetadataHandler.populateCalciteSchema is a callback
from the Impala framework called by the StatementMetadataLoader
before the Calcite validation step. In this phase, the tables get
loaded from catalogd. At this point, the Calcite schema gets loaded
with all the catalogd objects. For time travel objects, as mentioned
earlier, an IcebergTimeTravelTable object gets created. There may
be some TimeTravel tables that are duplicates, but it is not worth
the coding headache to make them unique since they only exist at
analysis time. The unique time travel name uses the same TimeTravelSpec
hash code in the ImpalaSnapshotSqlNode object which is how Calcite
can fetch the correct schema.

- At Logical node creation time, a LogicalSnapshot node is created.
This is not needed by Impala since the LogicalTableScan object will
contain the CalciteIcebergTable which contains the
IcebergTimeTravelTable, so it is removed from the logical RelNode
tree via the RemoveSnapshotRule.

- The existing ScanNodeHelper interface has a new getTimeTravelSpec
method which allows the IcebergScanPlanner to fetch the
TimeTravelSpec.

Testing: This was run against the time_travel e2e test in
test_iceberg.py and manually checked to ensure that there was never
any fallback. In a later commit, there will be tests to ensure that
fallback does not happen.

Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
---
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/Parser.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelper.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelperImpl.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M java/calcite-planner/src/main/codegen/templates/Parser.jj
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/RemoveSnapshotRule.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteIcebergTable.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/service/CalciteAnalysisDriver.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteParsedStatement.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/util/JavaCCParserUtils.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/validate/ImpalaSnapshotSqlNode.java
19 files changed, 467 insertions(+), 61 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
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: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Zoltan Borok-Nagy, Peter Rozsa, Joe McDonnell, Michael Smith, Noemi Pap-Takacs, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................

IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

This commit supports the Iceberg time travel feature for queries.
Iceberg uses either the "FOR SYSTEM_TIME AS OF" or "FOR
SYSTEM_VERSION AS OF" sql phrase to retrieve data (with its proper
schema) at a given time.

Calcite does provide some code to support snapshots with its
SqlSnapshotNode (validation time) and LogicalSnapshot (optimization
time) objects. However, the Calcite implementation did not quite
fit the architecture used by Impala.

The SqlSnapshot object has a timestamp type "Period" and directly
creates a LogicalSnapshot RelNode on top of a LogicalTableScan. The
reason this didn't work for Impala is because:
  - Iceberg works off a long value of snapshotId. While a timestamp
    can be provided at query time, sometimes it is the snapshotId
    itself that is provided through the SYSTEM_VERSION feature.
  - Impala creates a pseudo-catalog table at analysis time called
    ImpalaTimeTravelTable. This table holds the schema at the given
    time instance for the table. The problem for Calcite is that this
    schema needs to be known at validation time to handle validation
    steps like expanding a select star operation. When Calcite calls
    back into Impala, it needs a unique table name to fetch this.

Some creative engineering was needed to handled these issues. The
architecture here is most likely safe from Calcite upgrades.

Highlights of these changes include:

- In the Parser.jj file, the Impala TimeTravelSpec object is created.
A JavaCCParserUtils utility class helps grab the special token after
the "AS OF" keyword. The Impala CUP parser has the code which parses
the time travel SQL. So some code was added to the CUP parser which
allows the object to be created, located in the ".cup" file and the
Parser java file.

- The SqlSnapshot Calcite class was extended (ImpalaSnapshotSqlNode
to hold onto the Impala specific TimeTravelSpec. The Period variable
is ignored. The tableRef name contains the table name concatenated
with "_tt_<TimeTravelSpec hash code>" to generate a unique table name
that will be found in the CalciteCatalogReader in the validation
stage.

- The CalciteMetadataHandler.TableVisitor is still called after the
parsing and before the validation. It walks through the SqlNode tree.
It still tracks all the tables as before. When it encounters the
ImpalaSnapshotSqlNode, it keeps a map of a table with a list of
TimeTravelSpecs which need a catalog table object. For normal tables
without a TimeTravelSpec, the list will contain a "null" for the
TimeTravelSpec. Duplicates are ok here as will be discussed in the
next bullet point.

- While there may be duplicates, the TableName objects duplicates
are removed via the TableVisitor.getTableNames() by the caller of
the visitor.  These unique table names eventually get into the
StatementMetadataLoader class.

- The CalciteMetadataHandler.populateCalciteSchema is a callback
from the Impala framework called by the StatementMetadataLoader
before the Calcite validation step. In this phase, the tables get
loaded from catalogd. At this point, the Calcite schema gets loaded
with all the catalogd objects. For time travel objects, as mentioned
earlier, an IcebergTimeTravelTable object gets created. There may
be some TimeTravel tables that are duplicates, but it is not worth
the coding headache to make them unique since they only exist at
analysis time. The unique time travel name uses the same TimeTravelSpec
hash code in the ImpalaSnapshotSqlNode object which is how Calcite
can fetch the correct schema.

- At Logical node creation time, a LogicalSnapshot node is created.
This is not needed by Impala since the LogicalTableScan object will
contain the CalciteIcebergTable which contains the
IcebergTimeTravelTable, so it is removed from the logical RelNode
tree via the RemoveSnapshotRule.

- The existing ScanNodeHelper interface has a new getTimeTravelSpec
method which allows the IcebergScanPlanner to fetch the
TimeTravelSpec.

Testing: This was run against the time_travel e2e test in
test_iceberg.py and manually checked to ensure that there was never
any fallback. In a later commit, there will be tests to ensure that
fallback does not happen.

Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
---
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/Parser.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelper.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelperImpl.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M java/calcite-planner/src/main/codegen/templates/Parser.jj
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/RemoveSnapshotRule.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteIcebergTable.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/service/CalciteAnalysisDriver.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteParsedStatement.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/util/JavaCCParserUtils.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/validate/ImpalaSnapshotSqlNode.java
19 files changed, 480 insertions(+), 62 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/24577/12
-- 
To view, visit http://gerrit.cloudera.org:8080/24577
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 12
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Zoltan Borok-Nagy, Peter Rozsa, Joe McDonnell, Michael Smith, Noemi Pap-Takacs, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................

IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

This commit supports the Iceberg time travel feature for queries.
Iceberg uses either the "FOR SYSTEM_TIME AS OF" or "FOR
SYSTEM_VERSION AS OF" sql phrase to retrieve data (with its proper
schema) at a given time.

Calcite does provide some code to support snapshots with its
SqlSnapshotNode (validation time) and LogicalSnapshot (optimization
time) objects. However, the Calcite implementation did not quite
fit the architecture used by Impala.

The SqlSnapshot object has a timestamp type "Period" and directly
creates a LogicalSnapshot RelNode on top of a LogicalTableScan. The
reason this didn't work for Impala is because:
  - Iceberg works off a long value of snapshotId. While a timestamp
    can be provided at query time, sometimes it is the snapshotId
    itself that is provided through the SYSTEM_VERSION feature.
  - Impala creates a pseudo-catalog table at analysis time called
    IcebergTimeTravelTable. This table holds the schema at the given
    time instance for the table. The problem for Calcite is that this
    schema needs to be known at validation time to handle validation
    steps like expanding a select star operation. When Calcite calls
    back into Impala, it needs a unique table name to fetch this.

Some creative engineering was needed to handled these issues. The
architecture here is most likely safe from Calcite upgrades.

Highlights of these changes include:

- In the Parser.jj file, the Impala TimeTravelSpec object is created.
A JavaCCParserUtils utility class helps grab the special token after
the "AS OF" keyword. The Impala CUP parser has the code which parses
the time travel SQL. So some code was added to the CUP parser which
allows the object to be created, located in the ".cup" file and the
Parser java file.

- The SqlSnapshot Calcite class was extended (ImpalaSnapshotSqlNode
to hold onto the Impala specific TimeTravelSpec. The Period variable
is ignored. The tableRef name contains the table name concatenated
with "_tt_<TimeTravelSpec hash code>" to generate a unique table name
that will be found in the CalciteCatalogReader in the validation
stage.

- The CalciteMetadataHandler.TableVisitor is still called after the
parsing and before the validation. It walks through the SqlNode tree.
It still tracks all the tables as before. When it encounters the
ImpalaSnapshotSqlNode, it keeps a map of a table with a list of
TimeTravelSpecs which need a catalog table object. For normal tables
without a TimeTravelSpec, the list will contain a "null" for the
TimeTravelSpec. Duplicates are ok here as will be discussed in the
next bullet point.

- While there may be duplicates, the TableName objects duplicates
are removed via the TableVisitor.getTableNames() by the caller of
the visitor.  These unique table names eventually get into the
StatementMetadataLoader class.

- The CalciteMetadataHandler.populateCalciteSchema is a callback
from the Impala framework called by the StatementMetadataLoader
before the Calcite validation step. In this phase, the tables get
loaded from catalogd. At this point, the Calcite schema gets loaded
with all the catalogd objects. For time travel objects, as mentioned
earlier, an IcebergTimeTravelTable object gets created. There may
be some TimeTravel tables that are duplicates, but it is not worth
the coding headache to make them unique since they only exist at
analysis time. The unique time travel name uses the same TimeTravelSpec
hash code in the ImpalaSnapshotSqlNode object which is how Calcite
can fetch the correct schema.

- At Logical node creation time, a LogicalSnapshot node is created.
This is not needed by Impala since the LogicalTableScan object will
contain the CalciteIcebergTable which contains the
IcebergTimeTravelTable, so it is removed from the logical RelNode
tree via the RemoveSnapshotRule.

- The existing ScanNodeHelper interface has a new getTimeTravelSpec
method which allows the IcebergScanPlanner to fetch the
TimeTravelSpec.

Testing: This was run against the time_travel e2e test in
test_iceberg.py and manually checked to ensure that there was never
any fallback. In a later commit, there will be tests to ensure that
fallback does not happen.

Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
---
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/Parser.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelper.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelperImpl.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M java/calcite-planner/src/main/codegen/templates/Parser.jj
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/RemoveSnapshotRule.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteIcebergTable.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/service/CalciteAnalysisDriver.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteParsedStatement.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/util/JavaCCParserUtils.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/validate/ImpalaSnapshotSqlNode.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-scan-metrics-with-deletes.test
M tests/query_test/test_iceberg.py
22 files changed, 518 insertions(+), 73 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/24577/15
-- 
To view, visit http://gerrit.cloudera.org:8080/24577
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 15
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/24577 )

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 14:

(9 comments)

Thanks for working on this!

http://gerrit.cloudera.org:8080/#/c/24577/14//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/24577/14//COMMIT_MSG@26
PS14, Line 26: ImpalaTimeTravelTable
nit: IcebergTimeTravelTable


http://gerrit.cloudera.org:8080/#/c/24577/14//COMMIT_MSG@47
PS14, Line 47: <TimeTravelSpec hash code>
If the tt table is visible to users via plans/query profiles, then probably we should use the snapshot id (asOfVersion_) / epoch (asOfMicros_).


http://gerrit.cloudera.org:8080/#/c/24577/14/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:

http://gerrit.cloudera.org:8080/#/c/24577/14/fe/src/main/cup/sql-parser.cup@385
PS14, Line 385: Object
Can you add comment why it is an Object?


http://gerrit.cloudera.org:8080/#/c/24577/14/fe/src/main/java/org/apache/impala/analysis/Parser.java
File fe/src/main/java/org/apache/impala/analysis/Parser.java:

http://gerrit.cloudera.org:8080/#/c/24577/14/fe/src/main/java/org/apache/impala/analysis/Parser.java@65
PS14, Line 65: timestamp
nit: timestamp or snapshot id?


http://gerrit.cloudera.org:8080/#/c/24577/14/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/RemoveSnapshotRule.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/RemoveSnapshotRule.java:

http://gerrit.cloudera.org:8080/#/c/24577/14/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/RemoveSnapshotRule.java@41
PS14, Line 41: onMatch
Should we check that Snapshot is created on top of an Iceberg table, and only remove it in this case?


http://gerrit.cloudera.org:8080/#/c/24577/14/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteIcebergTable.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteIcebergTable.java:

http://gerrit.cloudera.org:8080/#/c/24577/14/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteIcebergTable.java@135
PS14, Line 135: true
Should it only return true when timeTravelName_ != null?


http://gerrit.cloudera.org:8080/#/c/24577/14/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java:

http://gerrit.cloudera.org:8080/#/c/24577/14/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java@255
PS14, Line 255: localTableNames
unused


http://gerrit.cloudera.org:8080/#/c/24577/14/java/calcite-planner/src/main/java/org/apache/impala/calcite/util/JavaCCParserUtils.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/util/JavaCCParserUtils.java:

http://gerrit.cloudera.org:8080/#/c/24577/14/java/calcite-planner/src/main/java/org/apache/impala/calcite/util/JavaCCParserUtils.java@39
PS14, Line 39: or null if coordinates are invalid.
nit: it throws exceptions but doesn't return null


http://gerrit.cloudera.org:8080/#/c/24577/14/testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
File testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test:

http://gerrit.cloudera.org:8080/#/c/24577/14/testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test@651
PS14, Line 651: AS OF clause is only supported for Iceberg tables.
Do we need CALCITE_PLANNER_CATCH here as well?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 14
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 23 Jul 2026 14:05:11 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 13:

Build Failed 

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 13
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 21 Jul 2026 00:39:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 15:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/24577/15/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java:

http://gerrit.cloudera.org:8080/#/c/24577/15/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java@152
PS15, Line 152:             String timeTravelTableKey = lowerCaseTableName + "_tt_" + tts.hashCode();
> Ok, I don't mind using a stable hash. Is there one in specific you would re
I traced this through the current flow. I think we can keep the fix small.

A Funnel is not needed here. Guava can hash the string directly:

```java
Hashing.murmur3_128()
    .hashString(tts.toSql(), StandardCharsets.UTF_8)
    .toString()
```

`TimeTravelSpec.toSql()` already includes the kind and the AS OF expression. I would use that instead of `Expr.toString()`, since `Expr.toString()` contains debug and analyzed state. Murmur3 128 is already used in Impala, and I would keep the full hash rather than truncate it.

One shared helper for the `_tt_...` suffix should be enough. Use it in `ImpalaSnapshotSqlNode`, and build the schema key in `populateCalciteSchema()` before calling `tts.analyze()`. The same `TimeTravelSpec` object is carried between those points, so no extra wrapper or Funnel is needed.

I checked the test-only alternative as well. This string is also the schema deduplication key and becomes the qualified table name used by self-join identity checks. Ignoring it in test output would hide the instability without fixing that identity.

I would be comfortable approving this small version. Making different expressions that resolve to the same snapshot share an identity can remain a follow-up.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 15
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 17 Aug 2026 23:53:40 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 21: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 21
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 21 Aug 2026 02:29:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 19: Code-Review+1

Carrying over my +1 from PS18 - PS19 is the rebase, the only difference being sql-parser.cup moving to common/cup/ with IMPALA-9153.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 19
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Aug 2026 12:13:12 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 20:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 20
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Aug 2026 13:22:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 17:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 17
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Aug 2026 16:07:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 22:

Apologies for committing this too early. I shouldn't have carried over the +2, but this one has been sitting around for over a month and it did get 2 +1s.

I'll be happy to address any further comments on this commit.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 22
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Aug 2026 15:25:12 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 15:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/24577/15/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java:

http://gerrit.cloudera.org:8080/#/c/24577/15/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java@152
PS15, Line 152:             String timeTravelTableKey = lowerCaseTableName + "_tt_" + tts.hashCode();
> TimeTravelSpec doesn't override equals()/hashCode() (nor does StmtNode), so
I think we can keep this simpler. Generate the synthetic name once in `ImpalaSnapshotSqlNode`, using a stable hash of the original spec, and pass that same name through to schema population. That avoids needing the Analyzer or a pre-validation rewrite.

Mapping different specs to the same resolved snapshot can be a follow-up, but I'd fix the unstable name here because it affects digests.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 15
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 17 Aug 2026 19:02:23 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 15:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/24577/15/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java:

http://gerrit.cloudera.org:8080/#/c/24577/15/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java@152
PS15, Line 152:             String timeTravelTableKey = lowerCaseTableName + "_tt_" + tts.hashCode();
> I traced this through the current flow. I think we can keep the fix small.
Ok, easy enough :)

Makes sense.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 15
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Aug 2026 13:48:12 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 16:

Quick note on the red check, in case it saves you the digging: it looks unrelated to the patch. gerrit-code-review-checks #23654 failed only through its clang-tidy-ub2004 child (#14565), and that one died in the build step before clang-tidy ran - pip could not fetch bitarray-2.3.0.tar.gz from files.pythonhosted.org ("too many 502 error responses"), which failed the shell_python3_install target. The same PyPI trouble hit clang-tidy #14568 on another change an hour later and its next patch set passed, so a re-run should clear it.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 16
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Aug 2026 20:07:46 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 18:

Build Failed 

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 18
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Aug 2026 16:01:26 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 21:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 21
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Aug 2026 20:44:56 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 15:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/24577/15/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java:

http://gerrit.cloudera.org:8080/#/c/24577/15/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java@152
PS15, Line 152:             String timeTravelTableKey = lowerCaseTableName + "_tt_" + tts.hashCode();
> TimeTravelSpec doesn't override equals()/hashCode() (nor does StmtNode), so
Yeah, I didn't love doing this.

My problem came down to the fact that I needed to generate an Identifier name at SqlNode creation time in Parser.jj for this table.  I thought i had put in a comment somewhere and if I didn't, I should.

The ImpalaSnapshotSqlNode.getTimeTravelSpecTableRef() uses the same table name string. This is what allows the internals of Calcite to validate the name of the table produced here.  And this SqlNode is created at Parser.jj time. 

The Analyzer object isn't available at Parser.jj time. Ensuring this gets analyzed may be possible, but it would have been jumping through hoops in an already complicated commit. So the asOfVersion and asOfMicros isn't available until after analysis. 

One thing I will change is to generate the "..._tt_..." in a static method that is called by both places to ensure the strings match.

You do make some good points, of course. This is gonna be a headache at some point for the Digest reasons. And the optimization is a good point too.

There are ways we can do this, of course. Off the top of my head, we can run a pre-validation step that changes the SqlNodes.  Or maybe somehow have the Digest obtain the name elsewhere (not sure if that's possible)? We can potentially find some way to avoid the duplication as well. We'd probably want to do that after the snapshotId is determined, since SYSTEM_TIME and SYSTEM_VERSION can use different expressions and still point to the same table (as well as multiple different SYSTEM_TIMEs)

My initial thought was to punt this and file a Jira and handle this later. We can live without the optimization for a first pass, but the Digest thing makes this a little sketchy to do. 

I'm open to thoughts on this as to what you think would be a good solution here if you have one.  Otherwise, I think I'd still like to punt this into a Jira to be handled later on.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 15
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 17 Aug 2026 15:20:38 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 11:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 11
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sun, 19 Jul 2026 15:44:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 14:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/24577/14//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/24577/14//COMMIT_MSG@47
PS14, Line 47: <TimeTravelSpec hash code>
> If the tt table is visible to users via plans/query profiles, then probably
This should not be visible to users.  If it is, then it's a bug.

We should have enough going information to have the real table name. It's only when Calcite has the thread of control where we need the ability to match the names up in this special way.


http://gerrit.cloudera.org:8080/#/c/24577/14/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:

http://gerrit.cloudera.org:8080/#/c/24577/14/fe/src/main/cup/sql-parser.cup@385
PS14, Line 385: Object
> Can you add comment why it is an Object?
Done


http://gerrit.cloudera.org:8080/#/c/24577/14/fe/src/main/java/org/apache/impala/analysis/Parser.java
File fe/src/main/java/org/apache/impala/analysis/Parser.java:

http://gerrit.cloudera.org:8080/#/c/24577/14/fe/src/main/java/org/apache/impala/analysis/Parser.java@65
PS14, Line 65: timestamp
> nit: timestamp or snapshot id?
Done


http://gerrit.cloudera.org:8080/#/c/24577/14/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/RemoveSnapshotRule.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/RemoveSnapshotRule.java:

http://gerrit.cloudera.org:8080/#/c/24577/14/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/RemoveSnapshotRule.java@41
PS14, Line 41: onMatch
> Should we check that Snapshot is created on top of an Iceberg table, and on
Done


http://gerrit.cloudera.org:8080/#/c/24577/14/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteIcebergTable.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteIcebergTable.java:

http://gerrit.cloudera.org:8080/#/c/24577/14/java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteIcebergTable.java@135
PS14, Line 135: true
> Should it only return true when timeTravelName_ != null?
I'm debating on this one...the Iceberg table can always be temporal whether we are using time travel in the query or not.  But putting "!= null" will also always be correct in our case.

I guess I decided to make the change though :)


http://gerrit.cloudera.org:8080/#/c/24577/14/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java:

http://gerrit.cloudera.org:8080/#/c/24577/14/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java@255
PS14, Line 255: localTableNames
> unused
Done


http://gerrit.cloudera.org:8080/#/c/24577/14/java/calcite-planner/src/main/java/org/apache/impala/calcite/util/JavaCCParserUtils.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/util/JavaCCParserUtils.java:

http://gerrit.cloudera.org:8080/#/c/24577/14/java/calcite-planner/src/main/java/org/apache/impala/calcite/util/JavaCCParserUtils.java@39
PS14, Line 39: or null if coordinates are invalid.
> nit: it throws exceptions but doesn't return null
Done


http://gerrit.cloudera.org:8080/#/c/24577/14/testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
File testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test:

http://gerrit.cloudera.org:8080/#/c/24577/14/testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test@651
PS14, Line 651: AS OF clause is only supported for Iceberg tables.
> Do we need CALCITE_PLANNER_CATCH here as well?
Removed this.

I sometimes test this on top of other commits I have in my local branch.  I have a commit that only falls back when the error is UnsupportedFeatureException.  When that happens, I need to capture the Calcite specific message.

In this case with the code currently committed, this will fall back to the original planner and produce the same message.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 14
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Jul 2026 12:21:25 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 11:

(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/24577/11/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java:

http://gerrit.cloudera.org:8080/#/c/24577/11/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java@50
PS11, Line 50: import com.google.common.base.Preconditions;
unused import: com.google.common.base.Preconditions



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 11
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sun, 19 Jul 2026 15:22:42 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 12:

My apologies if you were starting to review this.  One of my last second changes must have broken this.  Will post this again soon.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 12
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Jul 2026 15:22:53 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 14:

re-uploaded patch.  This is good to go again for review


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 14
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 21 Jul 2026 15:25:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 15:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/24577/15/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java:

http://gerrit.cloudera.org:8080/#/c/24577/15/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java@152
PS15, Line 152:             String timeTravelTableKey = lowerCaseTableName + "_tt_" + tts.hashCode();
> I think we can keep this simpler. Generate the synthetic name once in `Impa
Ok, I don't mind using a stable hash. Is there one in specific you would recommend? Never used one before. 

But before I do this, I want to code something you would approve of.

So I did a search and came up with the HashFunction in Guava, which I know we link against. Is this what you're thinking? I don't want to include another library. And I need some hash code creator because the string needs to be stable, compact, and without special characters.

So this doesn't look super straightforward, since I really only have objects that also have to use this special Hash fucnction. I suppose I can somehow use the string from the "Expr asOfExpr.toString()" passed into the constructor along with the Kind?  I'd somehow have to use that Funnel object too that is in the HashFunction documentation.  

So it's smelling a little funky and overkill here to me.

There is an alternative.  The Digest only really matters from a testing perspective. I don't think it matters in a production environment.  It's not like someone is storing the plan anywhere to be used later on. So if the tests were smart enough to ignore the hash part in the name, it would work too. I suppose the downside is that it could match any hash.  But I'm not sure that is enough of a testing downside to matter either. The question is more how hard it is to code the testing environment to understand this.

So I'm not sure. Lemme know what you think.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 15
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 17 Aug 2026 22:57:28 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 15:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/24577/15/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java:

http://gerrit.cloudera.org:8080/#/c/24577/15/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java@152
PS15, Line 152:             String timeTravelTableKey = lowerCaseTableName + "_tt_" + tts.hashCode();
> Ok, easy enough :)
PS16 looks right to me. One thing I wondered about and checked: computing the key here after tts.analyze() is safe even though analyzeTimeBased() replaces asOfExpr_ with CastExpr.createImplicit(TIMESTAMP, ...), because CastExpr.toSqlImpl() prints only the child unless ToSqlOptions.showImplictCasts() is set, and TimeTravelSpec.toSql() uses DEFAULT. So the string is the same before and after analysis, and both call sites end up with the same name.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 15
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Aug 2026 20:07:44 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 16:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/24577/16/java/calcite-planner/src/main/java/org/apache/impala/calcite/util/JavaCCParserUtils.java
File java/calcite-planner/src/main/java/org/apache/impala/calcite/util/JavaCCParserUtils.java:

http://gerrit.cloudera.org:8080/#/c/24577/16/java/calcite-planner/src/main/java/org/apache/impala/calcite/util/JavaCCParserUtils.java@67
PS16, Line 67:         currentAbsoluteIndex += lineLength + 1;
> Small thing about the arithmetic here: JavaCC treats \r\n as a single line 
Didn't really like the find/replace.  I think I like finding the exact location a little better.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 16
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Aug 2026 15:50:13 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Aleksandr Efimov, Zoltan Borok-Nagy, Peter Rozsa, Joe McDonnell, Michael Smith, Noemi Pap-Takacs, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................

IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

This commit supports the Iceberg time travel feature for queries.
Iceberg uses either the "FOR SYSTEM_TIME AS OF" or "FOR
SYSTEM_VERSION AS OF" sql phrase to retrieve data (with its proper
schema) at a given time.

Calcite does provide some code to support snapshots with its
SqlSnapshotNode (validation time) and LogicalSnapshot (optimization
time) objects. However, the Calcite implementation did not quite
fit the architecture used by Impala.

The SqlSnapshot object has a timestamp type "Period" and directly
creates a LogicalSnapshot RelNode on top of a LogicalTableScan. The
reason this didn't work for Impala is because:
  - Iceberg works off a long value of snapshotId. While a timestamp
    can be provided at query time, sometimes it is the snapshotId
    itself that is provided through the SYSTEM_VERSION feature.
  - Impala creates a pseudo-catalog table at analysis time called
    IcebergTimeTravelTable. This table holds the schema at the given
    time instance for the table. The problem for Calcite is that this
    schema needs to be known at validation time to handle validation
    steps like expanding a select star operation. When Calcite calls
    back into Impala, it needs a unique table name to fetch this.

Some creative engineering was needed to handled these issues. The
architecture here is most likely safe from Calcite upgrades.

Highlights of these changes include:

- In the Parser.jj file, the Impala TimeTravelSpec object is created.
A JavaCCParserUtils utility class helps grab the special token after
the "AS OF" keyword. The Impala CUP parser has the code which parses
the time travel SQL. So some code was added to the CUP parser which
allows the object to be created, located in the ".cup" file and the
Parser java file.

- The SqlSnapshot Calcite class was extended (ImpalaSnapshotSqlNode
to hold onto the Impala specific TimeTravelSpec. The Period variable
is ignored. The tableRef name contains the table name concatenated
with "_tt_<TimeTravelSpec hash code>" to generate a unique table name
that will be found in the CalciteCatalogReader in the validation
stage.

- The CalciteMetadataHandler.TableVisitor is still called after the
parsing and before the validation. It walks through the SqlNode tree.
It still tracks all the tables as before. When it encounters the
ImpalaSnapshotSqlNode, it keeps a map of a table with a list of
TimeTravelSpecs which need a catalog table object. For normal tables
without a TimeTravelSpec, the list will contain a "null" for the
TimeTravelSpec. Duplicates are ok here as will be discussed in the
next bullet point.

- While there may be duplicates, the TableName objects duplicates
are removed via the TableVisitor.getTableNames() by the caller of
the visitor.  These unique table names eventually get into the
StatementMetadataLoader class.

- The CalciteMetadataHandler.populateCalciteSchema is a callback
from the Impala framework called by the StatementMetadataLoader
before the Calcite validation step. In this phase, the tables get
loaded from catalogd. At this point, the Calcite schema gets loaded
with all the catalogd objects. For time travel objects, as mentioned
earlier, an IcebergTimeTravelTable object gets created. There may
be some TimeTravel tables that are duplicates, but it is not worth
the coding headache to make them unique since they only exist at
analysis time. The unique time travel name uses the same TimeTravelSpec
hash code in the ImpalaSnapshotSqlNode object which is how Calcite
can fetch the correct schema.

- At Logical node creation time, a LogicalSnapshot node is created.
This is not needed by Impala since the LogicalTableScan object will
contain the CalciteIcebergTable which contains the
IcebergTimeTravelTable, so it is removed from the logical RelNode
tree via the RemoveSnapshotRule.

- The existing ScanNodeHelper interface has a new getTimeTravelSpec
method which allows the IcebergScanPlanner to fetch the
TimeTravelSpec.

Testing: This was run against the time_travel e2e test in
test_iceberg.py and manually checked to ensure that there was never
any fallback. In a later commit, there will be tests to ensure that
fallback does not happen.

Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
---
M common/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/Parser.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelper.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelperImpl.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M java/calcite-planner/src/main/codegen/templates/Parser.jj
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/RemoveSnapshotRule.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteIcebergTable.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/service/CalciteAnalysisDriver.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteParsedStatement.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/util/JavaCCParserUtils.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/validate/ImpalaSnapshotSqlNode.java
A java/calcite-planner/src/test/java/org/apache/impala/calcite/util/JavaCCParserUtilTest.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-scan-metrics-with-deletes.test
M tests/query_test/test_iceberg.py
23 files changed, 637 insertions(+), 73 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/24577/19
-- 
To view, visit http://gerrit.cloudera.org:8080/24577
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 19
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Aleksandr Efimov, Zoltan Borok-Nagy, Peter Rozsa, Joe McDonnell, Michael Smith, Noemi Pap-Takacs, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................

IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

This commit supports the Iceberg time travel feature for queries.
Iceberg uses either the "FOR SYSTEM_TIME AS OF" or "FOR
SYSTEM_VERSION AS OF" sql phrase to retrieve data (with its proper
schema) at a given time.

Calcite does provide some code to support snapshots with its
SqlSnapshotNode (validation time) and LogicalSnapshot (optimization
time) objects. However, the Calcite implementation did not quite
fit the architecture used by Impala.

The SqlSnapshot object has a timestamp type "Period" and directly
creates a LogicalSnapshot RelNode on top of a LogicalTableScan. The
reason this didn't work for Impala is because:
  - Iceberg works off a long value of snapshotId. While a timestamp
    can be provided at query time, sometimes it is the snapshotId
    itself that is provided through the SYSTEM_VERSION feature.
  - Impala creates a pseudo-catalog table at analysis time called
    IcebergTimeTravelTable. This table holds the schema at the given
    time instance for the table. The problem for Calcite is that this
    schema needs to be known at validation time to handle validation
    steps like expanding a select star operation. When Calcite calls
    back into Impala, it needs a unique table name to fetch this.

Some creative engineering was needed to handled these issues. The
architecture here is most likely safe from Calcite upgrades.

Highlights of these changes include:

- In the Parser.jj file, the Impala TimeTravelSpec object is created.
A JavaCCParserUtils utility class helps grab the special token after
the "AS OF" keyword. The Impala CUP parser has the code which parses
the time travel SQL. So some code was added to the CUP parser which
allows the object to be created, located in the ".cup" file and the
Parser java file.

- The SqlSnapshot Calcite class was extended (ImpalaSnapshotSqlNode
to hold onto the Impala specific TimeTravelSpec. The Period variable
is ignored. The tableRef name contains the table name concatenated
with "_tt_<TimeTravelSpec hash code>" to generate a unique table name
that will be found in the CalciteCatalogReader in the validation
stage.

- The CalciteMetadataHandler.TableVisitor is still called after the
parsing and before the validation. It walks through the SqlNode tree.
It still tracks all the tables as before. When it encounters the
ImpalaSnapshotSqlNode, it keeps a map of a table with a list of
TimeTravelSpecs which need a catalog table object. For normal tables
without a TimeTravelSpec, the list will contain a "null" for the
TimeTravelSpec. Duplicates are ok here as will be discussed in the
next bullet point.

- While there may be duplicates, the TableName objects duplicates
are removed via the TableVisitor.getTableNames() by the caller of
the visitor.  These unique table names eventually get into the
StatementMetadataLoader class.

- The CalciteMetadataHandler.populateCalciteSchema is a callback
from the Impala framework called by the StatementMetadataLoader
before the Calcite validation step. In this phase, the tables get
loaded from catalogd. At this point, the Calcite schema gets loaded
with all the catalogd objects. For time travel objects, as mentioned
earlier, an IcebergTimeTravelTable object gets created. There may
be some TimeTravel tables that are duplicates, but it is not worth
the coding headache to make them unique since they only exist at
analysis time. The unique time travel name uses the same TimeTravelSpec
hash code in the ImpalaSnapshotSqlNode object which is how Calcite
can fetch the correct schema.

- At Logical node creation time, a LogicalSnapshot node is created.
This is not needed by Impala since the LogicalTableScan object will
contain the CalciteIcebergTable which contains the
IcebergTimeTravelTable, so it is removed from the logical RelNode
tree via the RemoveSnapshotRule.

- The existing ScanNodeHelper interface has a new getTimeTravelSpec
method which allows the IcebergScanPlanner to fetch the
TimeTravelSpec.

Testing: This was run against the time_travel e2e test in
test_iceberg.py and manually checked to ensure that there was never
any fallback. In a later commit, there will be tests to ensure that
fallback does not happen.

Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
---
M common/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/Parser.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelper.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelperImpl.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M java/calcite-planner/src/main/codegen/templates/Parser.jj
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/RemoveSnapshotRule.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteIcebergTable.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/service/CalciteAnalysisDriver.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteParsedStatement.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/util/JavaCCParserUtils.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/validate/ImpalaSnapshotSqlNode.java
A java/calcite-planner/src/test/java/org/apache/impala/calcite/util/JavaCCParserUtilTest.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-scan-metrics-with-deletes.test
22 files changed, 632 insertions(+), 72 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/24577/20
-- 
To view, visit http://gerrit.cloudera.org:8080/24577
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 20
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................


Patch Set 18: Code-Review+1

+1. Searching for each line sidesteps the line-ending question entirely - better than the normalizing I suggested. One thing I did check: because each search starts right after the previous line, a line whose text repeats an earlier one still lands on its own position.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 18
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Aug 2026 19:55:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Aleksandr Efimov, Zoltan Borok-Nagy, Peter Rozsa, Joe McDonnell, Michael Smith, Noemi Pap-Takacs, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-15178: Calcite Planner: support Iceberg time travel feature.
......................................................................

IMPALA-15178: Calcite Planner: support Iceberg time travel feature.

This commit supports the Iceberg time travel feature for queries.
Iceberg uses either the "FOR SYSTEM_TIME AS OF" or "FOR
SYSTEM_VERSION AS OF" sql phrase to retrieve data (with its proper
schema) at a given time.

Calcite does provide some code to support snapshots with its
SqlSnapshotNode (validation time) and LogicalSnapshot (optimization
time) objects. However, the Calcite implementation did not quite
fit the architecture used by Impala.

The SqlSnapshot object has a timestamp type "Period" and directly
creates a LogicalSnapshot RelNode on top of a LogicalTableScan. The
reason this didn't work for Impala is because:
  - Iceberg works off a long value of snapshotId. While a timestamp
    can be provided at query time, sometimes it is the snapshotId
    itself that is provided through the SYSTEM_VERSION feature.
  - Impala creates a pseudo-catalog table at analysis time called
    IcebergTimeTravelTable. This table holds the schema at the given
    time instance for the table. The problem for Calcite is that this
    schema needs to be known at validation time to handle validation
    steps like expanding a select star operation. When Calcite calls
    back into Impala, it needs a unique table name to fetch this.

Some creative engineering was needed to handled these issues. The
architecture here is most likely safe from Calcite upgrades.

Highlights of these changes include:

- In the Parser.jj file, the Impala TimeTravelSpec object is created.
A JavaCCParserUtils utility class helps grab the special token after
the "AS OF" keyword. The Impala CUP parser has the code which parses
the time travel SQL. So some code was added to the CUP parser which
allows the object to be created, located in the ".cup" file and the
Parser java file.

- The SqlSnapshot Calcite class was extended (ImpalaSnapshotSqlNode
to hold onto the Impala specific TimeTravelSpec. The Period variable
is ignored. The tableRef name contains the table name concatenated
with "_tt_<TimeTravelSpec hash code>" to generate a unique table name
that will be found in the CalciteCatalogReader in the validation
stage.

- The CalciteMetadataHandler.TableVisitor is still called after the
parsing and before the validation. It walks through the SqlNode tree.
It still tracks all the tables as before. When it encounters the
ImpalaSnapshotSqlNode, it keeps a map of a table with a list of
TimeTravelSpecs which need a catalog table object. For normal tables
without a TimeTravelSpec, the list will contain a "null" for the
TimeTravelSpec. Duplicates are ok here as will be discussed in the
next bullet point.

- While there may be duplicates, the TableName objects duplicates
are removed via the TableVisitor.getTableNames() by the caller of
the visitor.  These unique table names eventually get into the
StatementMetadataLoader class.

- The CalciteMetadataHandler.populateCalciteSchema is a callback
from the Impala framework called by the StatementMetadataLoader
before the Calcite validation step. In this phase, the tables get
loaded from catalogd. At this point, the Calcite schema gets loaded
with all the catalogd objects. For time travel objects, as mentioned
earlier, an IcebergTimeTravelTable object gets created. There may
be some TimeTravel tables that are duplicates, but it is not worth
the coding headache to make them unique since they only exist at
analysis time. The unique time travel name uses the same TimeTravelSpec
hash code in the ImpalaSnapshotSqlNode object which is how Calcite
can fetch the correct schema.

- At Logical node creation time, a LogicalSnapshot node is created.
This is not needed by Impala since the LogicalTableScan object will
contain the CalciteIcebergTable which contains the
IcebergTimeTravelTable, so it is removed from the logical RelNode
tree via the RemoveSnapshotRule.

- The existing ScanNodeHelper interface has a new getTimeTravelSpec
method which allows the IcebergScanPlanner to fetch the
TimeTravelSpec.

Testing: This was run against the time_travel e2e test in
test_iceberg.py and manually checked to ensure that there was never
any fallback. In a later commit, there will be tests to ensure that
fallback does not happen.

Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
---
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/Parser.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelper.java
M fe/src/main/java/org/apache/impala/planner/ScanNodeHelperImpl.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M java/calcite-planner/src/main/codegen/templates/Parser.jj
M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/RemoveSnapshotRule.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteIcebergTable.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/service/CalciteAnalysisDriver.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java
M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteParsedStatement.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/util/JavaCCParserUtils.java
A java/calcite-planner/src/main/java/org/apache/impala/calcite/validate/ImpalaSnapshotSqlNode.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-scan-metrics-with-deletes.test
M tests/query_test/test_iceberg.py
22 files changed, 537 insertions(+), 73 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/24577/16
-- 
To view, visit http://gerrit.cloudera.org:8080/24577
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6ab3466d6c453f8e030763749dd64903b8c41264
Gerrit-Change-Number: 24577
Gerrit-PatchSet: 16
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>