You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org> on 2021/08/10 11:16:44 UTC
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Zoltan Borok-Nagy has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17765
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
This patch adds support "FOR SYSTEM_TIME AS OF" and
"FOR SYSTEM_VERSION AS OF" clauses for Iceberg tables. The new
clauses are part of the table ref. FOR SYSTEM_TIME AS OF conforms to the
SQL2011 standard:
https://cs.ulb.ac.be/public/_media/teaching/infoh415/tempfeaturessql2011.pdf
"FOR SYSTEM_VERSION AS OF" is a non-standard extension that can be used
to query a table via a snapshot ID. HIVE-25344 also added support for
these clauses to Hive.
Sample queries:
SELECT * FROM t FOR SYSTEM_TIME AS OF now();
SELECT * FROM t FOR SYSTEM_TIME AS OF '2021-08-10 11:02:34';
SELECT * FROM t FOR SYSTEM_TIME AS OF now() - interval 10 days + interval 3 hours;
SELECT * FROM t FOR SYSTEM_VERSION AS OF 7080861547601448759;
SELECT * FROM t FOR SYSTEM_TIME AS OF now()
MINUS
SELECT * FROM t FOR SYSTEM_TIME AS OF now() - interval 1 days;
This patch uses some parts of the in-progress
IMPALA-9773 (https://gerrit.cloudera.org/#/c/13342/) developed by
Todd Lipcon and Grant Henke. This patch also resolves some TODOs of
IMPALA-9773, i.e. after this patch it'll be easier to add
time travel for Kudu tables as well.
Testing:
* added parser tests (ParserTest.java)
* added analyzer tests (AnalyzeStmtsTest.java)
* added e2e tests (test_iceberg.py)
Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
---
M be/src/common/init.cc
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/ColumnDef.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
A fe/src/main/java/org/apache/impala/analysis/TimeTravelSpec.java
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
A fe/src/main/java/org/apache/impala/util/ExprUtil.java
M fe/src/main/java/org/apache/impala/util/IcebergUtil.java
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
19 files changed, 520 insertions(+), 63 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/17765/1
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Attila Jeges (Code Review)" <ge...@cloudera.org>.
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/17765/1/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:
http://gerrit.cloudera.org:8080/#/c/17765/1/tests/query_test/test_iceberg.py@197
PS1, Line 197: # Query old snapshot
> We are using the local timezone of the machine that executes the test. I do
What I was thinking of is to set TIMEZONE query option to a specific timezone for the queries and get the current timestamp after each query with "select now();" (with TIMEZONE set to the same timezone).
This way we wouldn't depend on the local timezone of the machine.
A test could compare the results for the same timestamp in different TIMEZONEs, to prove that time travel uses the coordinator's local timezone.
Anyway, it was just a silly idea. Now that I think about it, it doesn't sound too useful. Feel free to ignore it.
http://gerrit.cloudera.org:8080/#/c/17765/1/tests/query_test/test_iceberg.py@197
PS1, Line 197: # Query old snapshot
> Currently querying the future behaves the same as querying by now(). I'm no
Sure, use your best judgement.
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Aug 2021 15:08:45 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Qifan Chen, Attila Jeges, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17765
to look at the new patch set (#5).
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
This patch adds support "FOR SYSTEM_TIME AS OF" and
"FOR SYSTEM_VERSION AS OF" clauses for Iceberg tables. The new
clauses are part of the table ref. FOR SYSTEM_TIME AS OF conforms to the
SQL2011 standard:
https://cs.ulb.ac.be/public/_media/teaching/infoh415/tempfeaturessql2011.pdf
With FOR SYSTEM_TIME AS OF we can query a table at a specific time
point, e.g. we can retrieve what was the table content 1 day ago.
The timestamp given to "FOR SYSTEM_TIME AS OF" is interpreted in the
local timezone. The local timezone can be set via the query option
TIMEZONE. By default the timezone being used is the coordinator node's
local timezone. The timestamp is translated to UTC because table
snapshots are tagged with a UTC timestamps.
"FOR SYSTEM_VERSION AS OF" is a non-standard extension. It works
similarly to FOR SYSTEM_TIME AS OF, but with this clause we can query
a table via a snapshot ID instead of a timestamp.
HIVE-25344 also added support for these clauses to Hive.
Table snapshot IDs and timestamp information can be queried with the
help of the DESCRIBE HISTORY command.
Sample queries:
SELECT * FROM t FOR SYSTEM_TIME AS OF now();
SELECT * FROM t FOR SYSTEM_TIME AS OF '2021-08-10 11:02:34';
SELECT * FROM t FOR SYSTEM_TIME AS OF now() - interval 10 days + interval 3 hours;
SELECT * FROM t FOR SYSTEM_VERSION AS OF 7080861547601448759;
SELECT * FROM t FOR SYSTEM_TIME AS OF now()
MINUS
SELECT * FROM t FOR SYSTEM_TIME AS OF now() - interval 1 days;
This patch uses some parts of the in-progress
IMPALA-9773 (https://gerrit.cloudera.org/#/c/13342/) developed by
Todd Lipcon and Grant Henke. This patch also resolves some TODOs of
IMPALA-9773, i.e. after this patch it'll be easier to add
time travel for Kudu tables as well.
Testing:
* added parser tests (ParserTest.java)
* added analyzer tests (AnalyzeStmtsTest.java)
* added e2e tests (test_iceberg.py)
Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
---
M be/src/common/init.cc
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/ColumnDef.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
A fe/src/main/java/org/apache/impala/analysis/TimeTravelSpec.java
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
A fe/src/main/java/org/apache/impala/util/ExprUtil.java
M fe/src/main/java/org/apache/impala/util/IcebergUtil.java
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
19 files changed, 575 insertions(+), 63 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/17765/5
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17765/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java:
http://gerrit.cloudera.org:8080/#/c/17765/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@4871
PS4, Line 4871: iceT);
> Yeah, I think it depends on the system time window for a row in the table.
Right, thanks. We don't have these extra startdate/enddate columns here, as time information is only stored in the snapshot files. But at least the behavior is standard-conforming.
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Aug 2021 15:22:14 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17765/1/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:
http://gerrit.cloudera.org:8080/#/c/17765/1/tests/query_test/test_iceberg.py@197
PS1, Line 197: SELECT * FROM {tbl} FOR SYSTEM_VERSION AS OF {v_old}""".format(
> What I was thinking of is to set TIMEZONE query option to a specific timezo
I think it's a good idea. Added a test for it.
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Aug 2021 16:43:34 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Amogh Margoor (Code Review)" <ge...@cloudera.org>.
Amogh Margoor has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 5: Code-Review+1
(3 comments)
nice addition. LGTM!
http://gerrit.cloudera.org:8080/#/c/17765/5/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:
http://gerrit.cloudera.org:8080/#/c/17765/5/tests/query_test/test_iceberg.py@116
PS5, Line 116: def test_time_travel(self, vector, unique_database):
> Snapshots should be available until they are expired:
One more case that can be added to JIRA above is time travel's behaviour with schema evolution. I guess with old snapshot selected old schema is picked up too.
http://gerrit.cloudera.org:8080/#/c/17765/5/tests/query_test/test_iceberg.py@171
PS5, Line 171: # Future queries return the current snapshot.
> No, you can only query concrete snapshots. Also, the snapshot IDs are not i
ahh I see.. didn't know snapshot ids are not monotonically increasing.
http://gerrit.cloudera.org:8080/#/c/17765/5/tests/query_test/test_iceberg.py@205
PS5, Line 205: self.execute_query("SELECT * FROM {0} FOR SYSTEM_TIME AS OF {1}".format(
> Yes, but it's not easy to add a test for it currently. It will be covered b
makes sense.
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Aug 2021 16:03:08 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/main/cup/sql-parser.cup@3069
PS1, Line 3069: expr:expr
This could be integer literal.
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Aug 2021 11:29:59 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Attila Jeges (Code Review)" <ge...@cloudera.org>.
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/17765/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17765/1//COMMIT_MSG@9
PS1, Line 9: This patch adds support "FOR SYSTEM_TIME AS OF" and
Please clarify the the timestamp specified with "FOR SYSTEM_TIME AS OF" is interpreted to be in the local timezone. Local timezone meaning the coordinator node's local timezone.
http://gerrit.cloudera.org:8080/#/c/17765/1/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:
http://gerrit.cloudera.org:8080/#/c/17765/1/tests/query_test/test_iceberg.py@197
PS1, Line 197: # Query old snapshot
> Maybe add another test to query with a timestamp in the future.
You could also test (if not too much work) that switching Impala to another timezone (e.g. using TIMEZONE query option) changes the results of the time travel query.
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 25 Aug 2021 11:58:09 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Qifan Chen, Amogh Margoor, Attila Jeges, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17765
to look at the new patch set (#7).
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
This patch adds support "FOR SYSTEM_TIME AS OF" and
"FOR SYSTEM_VERSION AS OF" clauses for Iceberg tables. The new
clauses are part of the table ref. FOR SYSTEM_TIME AS OF conforms to the
SQL2011 standard:
https://cs.ulb.ac.be/public/_media/teaching/infoh415/tempfeaturessql2011.pdf
With FOR SYSTEM_TIME AS OF we can query a table at a specific time
point, e.g. we can retrieve what was the table content 1 day ago.
The timestamp given to "FOR SYSTEM_TIME AS OF" is interpreted in the
local timezone. The local timezone can be set via the query option
TIMEZONE. By default the timezone being used is the coordinator node's
local timezone. The timestamp is translated to UTC because table
snapshots are tagged with a UTC timestamps.
"FOR SYSTEM_VERSION AS OF" is a non-standard extension. It works
similarly to FOR SYSTEM_TIME AS OF, but with this clause we can query
a table via a snapshot ID instead of a timestamp.
HIVE-25344 also added support for these clauses to Hive.
Table snapshot IDs and timestamp information can be queried with the
help of the DESCRIBE HISTORY command.
Sample queries:
SELECT * FROM t FOR SYSTEM_TIME AS OF now();
SELECT * FROM t FOR SYSTEM_TIME AS OF '2021-08-10 11:02:34';
SELECT * FROM t FOR SYSTEM_TIME AS OF now() - interval 10 days + interval 3 hours;
SELECT * FROM t FOR SYSTEM_VERSION AS OF 7080861547601448759;
SELECT * FROM t FOR SYSTEM_TIME AS OF now()
MINUS
SELECT * FROM t FOR SYSTEM_TIME AS OF now() - interval 1 days;
This patch uses some parts of the in-progress
IMPALA-9773 (https://gerrit.cloudera.org/#/c/13342/) developed by
Todd Lipcon and Grant Henke. This patch also resolves some TODOs of
IMPALA-9773, i.e. after this patch it'll be easier to add
time travel for Kudu tables as well.
Testing:
* added parser tests (ParserTest.java)
* added analyzer tests (AnalyzeStmtsTest.java)
* added e2e tests (test_iceberg.py)
Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
---
M be/src/common/init.cc
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/ColumnDef.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
A fe/src/main/java/org/apache/impala/analysis/TimeTravelSpec.java
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
A fe/src/main/java/org/apache/impala/util/ExprUtil.java
M fe/src/main/java/org/apache/impala/util/IcebergUtil.java
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
19 files changed, 576 insertions(+), 64 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/17765/7
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 7
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 8:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7437/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 8
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Aug 2021 16:54:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 1:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/17765/1/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:
http://gerrit.cloudera.org:8080/#/c/17765/1/tests/query_test/test_iceberg.py@178
PS1, Line 178: t
flake8: E126 continuation line over-indented for hanging indent
http://gerrit.cloudera.org:8080/#/c/17765/1/tests/query_test/test_iceberg.py@183
PS1, Line 183: t
flake8: E126 continuation line over-indented for hanging indent
http://gerrit.cloudera.org:8080/#/c/17765/1/tests/query_test/test_iceberg.py@189
PS1, Line 189: t
flake8: E126 continuation line over-indented for hanging indent
http://gerrit.cloudera.org:8080/#/c/17765/1/tests/query_test/test_iceberg.py@194
PS1, Line 194: t
flake8: E126 continuation line over-indented for hanging indent
http://gerrit.cloudera.org:8080/#/c/17765/1/tests/query_test/test_iceberg.py@201
PS1, Line 201: f
flake8: F821 undefined name 'false'
http://gerrit.cloudera.org:8080/#/c/17765/1/tests/query_test/test_iceberg.py@201
PS1, Line 201:
flake8: E261 at least two spaces before inline comment
http://gerrit.cloudera.org:8080/#/c/17765/1/tests/query_test/test_iceberg.py@207
PS1, Line 207: f
flake8: F821 undefined name 'false'
http://gerrit.cloudera.org:8080/#/c/17765/1/tests/query_test/test_iceberg.py@207
PS1, Line 207:
flake8: E261 at least two spaces before inline comment
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Aug 2021 11:17:34 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 6:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7429/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Aug 2021 16:21:46 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Attila Jeges (Code Review)" <ge...@cloudera.org>.
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/main/java/org/apache/impala/analysis/TableRef.java
File fe/src/main/java/org/apache/impala/analysis/TableRef.java:
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/main/java/org/apache/impala/analysis/TableRef.java@222
PS1, Line 222: timeTravelSpec_ = other.timeTravelSpec_;
> Maybe cloning the TimeTravelSpec object would be safer than just copying th
Alternatively, you could set timeTravelSpec_ to null in reset(), instead of calling timeTravelSpec_.reset()
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 25 Aug 2021 17:55:20 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Attila Jeges, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17765
to look at the new patch set (#2).
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
This patch adds support "FOR SYSTEM_TIME AS OF" and
"FOR SYSTEM_VERSION AS OF" clauses for Iceberg tables. The new
clauses are part of the table ref. FOR SYSTEM_TIME AS OF conforms to the
SQL2011 standard:
https://cs.ulb.ac.be/public/_media/teaching/infoh415/tempfeaturessql2011.pdf
The timestamp given to "FOR SYSTEM_TIME AS OF" is interpreted in the
local timezone. The local timezone can be set via the query option
TIMEZONE. By default the timezone being used is the coordinator node's
local timezone.
"FOR SYSTEM_VERSION AS OF" is a non-standard extension that can be used
to query a table via a snapshot ID. HIVE-25344 also added support for
these clauses to Hive.
Sample queries:
SELECT * FROM t FOR SYSTEM_TIME AS OF now();
SELECT * FROM t FOR SYSTEM_TIME AS OF '2021-08-10 11:02:34';
SELECT * FROM t FOR SYSTEM_TIME AS OF now() - interval 10 days + interval 3 hours;
SELECT * FROM t FOR SYSTEM_VERSION AS OF 7080861547601448759;
SELECT * FROM t FOR SYSTEM_TIME AS OF now()
MINUS
SELECT * FROM t FOR SYSTEM_TIME AS OF now() - interval 1 days;
This patch uses some parts of the in-progress
IMPALA-9773 (https://gerrit.cloudera.org/#/c/13342/) developed by
Todd Lipcon and Grant Henke. This patch also resolves some TODOs of
IMPALA-9773, i.e. after this patch it'll be easier to add
time travel for Kudu tables as well.
Testing:
* added parser tests (ParserTest.java)
* added analyzer tests (AnalyzeStmtsTest.java)
* added e2e tests (test_iceberg.py)
Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
---
M be/src/common/init.cc
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/ColumnDef.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
A fe/src/main/java/org/apache/impala/analysis/TimeTravelSpec.java
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
A fe/src/main/java/org/apache/impala/util/ExprUtil.java
M fe/src/main/java/org/apache/impala/util/IcebergUtil.java
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
19 files changed, 545 insertions(+), 63 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/17765/2
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/9383/ : 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/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Aug 2021 10:22:52 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
This patch adds support "FOR SYSTEM_TIME AS OF" and
"FOR SYSTEM_VERSION AS OF" clauses for Iceberg tables. The new
clauses are part of the table ref. FOR SYSTEM_TIME AS OF conforms to the
SQL2011 standard:
https://cs.ulb.ac.be/public/_media/teaching/infoh415/tempfeaturessql2011.pdf
With FOR SYSTEM_TIME AS OF we can query a table at a specific time
point, e.g. we can retrieve what was the table content 1 day ago.
The timestamp given to "FOR SYSTEM_TIME AS OF" is interpreted in the
local timezone. The local timezone can be set via the query option
TIMEZONE. By default the timezone being used is the coordinator node's
local timezone. The timestamp is translated to UTC because table
snapshots are tagged with a UTC timestamps.
"FOR SYSTEM_VERSION AS OF" is a non-standard extension. It works
similarly to FOR SYSTEM_TIME AS OF, but with this clause we can query
a table via a snapshot ID instead of a timestamp.
HIVE-25344 also added support for these clauses to Hive.
Table snapshot IDs and timestamp information can be queried with the
help of the DESCRIBE HISTORY command.
Sample queries:
SELECT * FROM t FOR SYSTEM_TIME AS OF now();
SELECT * FROM t FOR SYSTEM_TIME AS OF '2021-08-10 11:02:34';
SELECT * FROM t FOR SYSTEM_TIME AS OF now() - interval 10 days + interval 3 hours;
SELECT * FROM t FOR SYSTEM_VERSION AS OF 7080861547601448759;
SELECT * FROM t FOR SYSTEM_TIME AS OF now()
MINUS
SELECT * FROM t FOR SYSTEM_TIME AS OF now() - interval 1 days;
This patch uses some parts of the in-progress
IMPALA-9773 (https://gerrit.cloudera.org/#/c/13342/) developed by
Todd Lipcon and Grant Henke. This patch also resolves some TODOs of
IMPALA-9773, i.e. after this patch it'll be easier to add
time travel for Kudu tables as well.
Testing:
* added parser tests (ParserTest.java)
* added analyzer tests (AnalyzeStmtsTest.java)
* added e2e tests (test_iceberg.py)
Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Reviewed-on: http://gerrit.cloudera.org:8080/17765
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/common/init.cc
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/ColumnDef.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
A fe/src/main/java/org/apache/impala/analysis/TimeTravelSpec.java
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
A fe/src/main/java/org/apache/impala/util/ExprUtil.java
M fe/src/main/java/org/apache/impala/util/IcebergUtil.java
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
19 files changed, 576 insertions(+), 64 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 9
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 6: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/7429/
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Aug 2021 22:37:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 7: Code-Review+2
Fixed trivial test issue.
Carry +2
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 7
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Aug 2021 12:28:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 3:
(12 comments)
Thanks for the comments!
http://gerrit.cloudera.org:8080/#/c/17765/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17765/1//COMMIT_MSG@9
PS1, Line 9: This patch adds support "FOR SYSTEM_TIME AS OF" and
> Please clarify the the timestamp specified with "FOR SYSTEM_TIME AS OF" is
Done
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/main/cup/sql-parser.cup@3069
PS1, Line 3069: numeric_l
> This could be integer literal.
INTERGER_LITERAL has type of BigDecimal, so we can use numeric_literal here.
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/main/java/org/apache/impala/analysis/TableRef.java
File fe/src/main/java/org/apache/impala/analysis/TableRef.java:
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/main/java/org/apache/impala/analysis/TableRef.java@152
PS1, Line 152: // Time travel spec of this table ref. It contains information specified in the
> Please add a comment.
Done
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/main/java/org/apache/impala/analysis/TableRef.java@222
PS1, Line 222: hasExplicitAlias_ = other.hasExplicitAlias_;
> Maybe cloning the TimeTravelSpec object would be safer than just copying th
Done
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/main/java/org/apache/impala/analysis/TableRef.java@222
PS1, Line 222: hasExplicitAlias_ = other.hasExplicitAlias_;
> Alternatively, you could set timeTravelSpec_ to null in reset(), instead of
I think we should clone it because we get the time travel spec in the constructor, i.e. it's not something that we can recreate during analyze().
Setting it to null in reset() would cause bugs when the query is rewritten and re-analyzed without a time travel spec.
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/main/java/org/apache/impala/analysis/TimeTravelSpec.java
File fe/src/main/java/org/apache/impala/analysis/TimeTravelSpec.java:
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/main/java/org/apache/impala/analysis/TimeTravelSpec.java@113
PS1, Line 113: asOfExpr_.analyze(analyzer);
> you could also check that asOfVersion_ > 0
Added a check, but now that FOR SYSTEM_VERSION AS OF only accepts numeric_literals it's not possible to write FOR SYSTEM_TIME AS OF -123; because '-123' can only be parsed as an expr, not as a numeric_literal.
Added tests to ParserTest.java.
Also the above (asOfExpr_ instanceof LiteralExpr) is redundant for the same reason. But I probably keep these checks here in case we change the parser in the future.
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java
File fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java:
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java@132
PS1, Line 132: } catch (IOException ex) {
> Does 'ex' contain dataFile.path() ? If not, please add dataFile.path() to t
Done
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/main/java/org/apache/impala/util/IcebergUtil.java
File fe/src/main/java/org/apache/impala/util/IcebergUtil.java:
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/main/java/org/apache/impala/util/IcebergUtil.java@515
PS1, Line 515: TableScan scan = createScanAsOf(table, timeTravelSpec);
> nit: Since 'baseTable' is not used anywhere else, you could move L514 insid
Good idea, done.
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java:
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@4884
PS1, Line 4884: TblsAnalysisError("select * from $TBL for system_time as of now()", nonIceT,
> The end of the error msg was left out intentionally?
Since then this test has been removed due to SYSTEM_VERSION AS OF expects a numeric_literal now.
http://gerrit.cloudera.org:8080/#/c/17765/1/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:
http://gerrit.cloudera.org:8080/#/c/17765/1/tests/query_test/test_iceberg.py@134
PS1, Line 134: sn
> 'snapshot_id' ?
Done
http://gerrit.cloudera.org:8080/#/c/17765/1/tests/query_test/test_iceberg.py@197
PS1, Line 197: # Query old snapshot
> Maybe add another test to query with a timestamp in the future.
Currently querying the future behaves the same as querying by now(). I'm not sure if we want a test for it to preserve this behavior.
http://gerrit.cloudera.org:8080/#/c/17765/1/tests/query_test/test_iceberg.py@197
PS1, Line 197: # Query old snapshot
> You could also test (if not too much work) that switching Impala to another
We are using the local timezone of the machine that executes the test. I don't know what's the best way to change to a timezene relative to the current one.
In some cases it might be not possible, e.g. if you are in GMT+12 then you cannot switch to GMT+13, but GMT-11 wouldn't work either.
Or am I missing something?
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Aug 2021 12:30:47 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 8: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 8
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Aug 2021 23:09:40 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Amogh Margoor (Code Review)" <ge...@cloudera.org>.
Amogh Margoor has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 5:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/17765/5/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:
http://gerrit.cloudera.org:8080/#/c/17765/5/fe/src/main/cup/sql-parser.cup@3066
PS5, Line 3066: opt_asof ::=
This is cool. Btw Delta also supports '@' syntax which can be convenient and less verbose sometimes:
SELECT * FROM events@20190101000000000 (timestamp)
SELECT * FROM events@v123 (version)
We can consider it in future based on adoption.
http://gerrit.cloudera.org:8080/#/c/17765/5/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:
http://gerrit.cloudera.org:8080/#/c/17765/5/tests/query_test/test_iceberg.py@116
PS5, Line 116: def test_time_travel(self, vector, unique_database):
One generic question about Time Travel I had was w.r.t Compaction:
I guess Iceberg supports compaction of data files using rewriteDataFiles through some engines. Suppose we compact data files between time stamp T1, T2, T3, ... T10 and after compaction we query via this feature for timestamp T3. Will Iceberg library 'scan.asOfTime' fail for that ?
http://gerrit.cloudera.org:8080/#/c/17765/5/tests/query_test/test_iceberg.py@171
PS5, Line 171: # Future queries return the current snapshot.
nice. Is it similar behaviour when we specify version greater than the current version ?
http://gerrit.cloudera.org:8080/#/c/17765/5/tests/query_test/test_iceberg.py@205
PS5, Line 205: self.execute_query("SELECT * FROM {0} FOR SYSTEM_TIME AS OF {1}".format(
Does this cover the case when older snapshots are expired too
?
table.expireSnapshots()
.expireOlderThan(tsToExpire)
.commit();
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Aug 2021 11:06:13 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 8: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/7436/
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 8
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Aug 2021 18:34:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 2:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/17765/2/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:
http://gerrit.cloudera.org:8080/#/c/17765/2/tests/query_test/test_iceberg.py@178
PS2, Line 178: t
flake8: E126 continuation line over-indented for hanging indent
http://gerrit.cloudera.org:8080/#/c/17765/2/tests/query_test/test_iceberg.py@183
PS2, Line 183: t
flake8: E126 continuation line over-indented for hanging indent
http://gerrit.cloudera.org:8080/#/c/17765/2/tests/query_test/test_iceberg.py@189
PS2, Line 189: t
flake8: E126 continuation line over-indented for hanging indent
http://gerrit.cloudera.org:8080/#/c/17765/2/tests/query_test/test_iceberg.py@194
PS2, Line 194: t
flake8: E126 continuation line over-indented for hanging indent
http://gerrit.cloudera.org:8080/#/c/17765/2/tests/query_test/test_iceberg.py@201
PS2, Line 201:
flake8: E261 at least two spaces before inline comment
http://gerrit.cloudera.org:8080/#/c/17765/2/tests/query_test/test_iceberg.py@207
PS2, Line 207:
flake8: E261 at least two spaces before inline comment
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Aug 2021 12:13:14 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/9376/ : 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/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Aug 2021 12:54:50 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Qifan Chen (Code Review)" <ge...@cloudera.org>.
Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 5:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/17765/4/fe/src/main/java/org/apache/impala/analysis/TimeTravelSpec.java
File fe/src/main/java/org/apache/impala/analysis/TimeTravelSpec.java:
http://gerrit.cloudera.org:8080/#/c/17765/4/fe/src/main/java/org/apache/impala/analysis/TimeTravelSpec.java@39
PS4, Line 39: TIME
> Done.
Done
http://gerrit.cloudera.org:8080/#/c/17765/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java:
http://gerrit.cloudera.org:8080/#/c/17765/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@4871
PS4, Line 4871: iceT);
> Yeah I wasn't sure about what should be the behavior of future queries. Ice
Yeah, I think it depends on the system time window for a row in the table. A fresh inserted row will have the end time to be +infinite and be returned with any future time. See page 39 and 40 of the paper.
http://gerrit.cloudera.org:8080/#/c/17765/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@4880
PS4, Line 4880: TblsAnalysisError("select * from $TBL for system_time as of '2021-02-32 15:52:45'",
> version as of -10 raises a parse error. There's a test for it in ParserTest
Done
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Aug 2021 14:50:44 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Attila Jeges (Code Review)" <ge...@cloudera.org>.
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/main/java/org/apache/impala/analysis/TableRef.java
File fe/src/main/java/org/apache/impala/analysis/TableRef.java:
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/main/java/org/apache/impala/analysis/TableRef.java@152
PS1, Line 152: protected TimeTravelSpec timeTravelSpec_;
Please add a comment.
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/main/java/org/apache/impala/analysis/TableRef.java@222
PS1, Line 222: timeTravelSpec_ = other.timeTravelSpec_;
Maybe cloning the TimeTravelSpec object would be safer than just copying the reference.
Perhaps it is not an issue, but if 2 TableRef instances share the same 'timeTravelSpec_' reference and one of them is reset(), that will affect the other instance as well, right?
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/main/java/org/apache/impala/analysis/TimeTravelSpec.java
File fe/src/main/java/org/apache/impala/analysis/TimeTravelSpec.java:
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/main/java/org/apache/impala/analysis/TimeTravelSpec.java@113
PS1, Line 113: asOfVersion_ = asOfExpr_.evalToInteger(analyzer, "SYSTEM_VERSION AS OF");
you could also check that asOfVersion_ > 0
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java
File fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java:
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java@132
PS1, Line 132: } catch (IOException ex) {
Does 'ex' contain dataFile.path() ? If not, please add dataFile.path() to the exception in L133
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 25 Aug 2021 16:08:15 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 5:
(4 comments)
Thanks for the comments!
http://gerrit.cloudera.org:8080/#/c/17765/5/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:
http://gerrit.cloudera.org:8080/#/c/17765/5/fe/src/main/cup/sql-parser.cup@3066
PS5, Line 3066: opt_asof ::=
> This is cool. Btw Delta also supports '@' syntax which can be convenient an
Yeah, that is more concise. I discussed it with Hive/Spark contributors and the conclusion was to use the standard SQL2011 sytnax even if it's a bit more verbose.
But I agree, if the '@' syntax gets popular in multiple engines in the future then we can consider adding support for it.
http://gerrit.cloudera.org:8080/#/c/17765/5/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:
http://gerrit.cloudera.org:8080/#/c/17765/5/tests/query_test/test_iceberg.py@116
PS5, Line 116: def test_time_travel(self, vector, unique_database):
> One generic question about Time Travel I had was w.r.t Compaction:
Snapshots should be available until they are expired:
https://iceberg.apache.org/maintenance/#recommended-maintenance
It's not easy to test it currently as Impala/Hive don't support compaction yet. I created a Jira about testing: IMPALA-10892
http://gerrit.cloudera.org:8080/#/c/17765/5/tests/query_test/test_iceberg.py@171
PS5, Line 171: # Future queries return the current snapshot.
> nice. Is it similar behaviour when we specify version greater than the curr
No, you can only query concrete snapshots. Also, the snapshot IDs are not in an increasing order, i.e. newer snapshot might have a lower snapshot ID.
http://gerrit.cloudera.org:8080/#/c/17765/5/tests/query_test/test_iceberg.py@205
PS5, Line 205: self.execute_query("SELECT * FROM {0} FOR SYSTEM_TIME AS OF {1}".format(
> Does this cover the case when older snapshots are expired too
Yes, but it's not easy to add a test for it currently. It will be covered by IMPALA-10892.
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Aug 2021 11:52:42 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Aug 2021 16:21:45 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Qifan Chen (Code Review)" <ge...@cloudera.org>.
Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 4:
(5 comments)
Looks great!
We should also state that translating the local time to the time zone used for all the rows in the table so that the result is correct WRT to the SQL standard.
http://gerrit.cloudera.org:8080/#/c/17765/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17765/4//COMMIT_MSG@14
PS4, Line 14:
May need to describe the semantics of FOR SYSTEM_TIME AS OF and FOR SYSTEM_VERSION AS OF" here.
http://gerrit.cloudera.org:8080/#/c/17765/4//COMMIT_MSG@18
PS4, Line 18: local timezone.
nit. Translating the local time to the time zone used for all the rows in the table is critical.
http://gerrit.cloudera.org:8080/#/c/17765/4/fe/src/main/java/org/apache/impala/analysis/TimeTravelSpec.java
File fe/src/main/java/org/apache/impala/analysis/TimeTravelSpec.java:
http://gerrit.cloudera.org:8080/#/c/17765/4/fe/src/main/java/org/apache/impala/analysis/TimeTravelSpec.java@39
PS4, Line 39: TIME
nit. May use TIME_AS_OF. Other possibilities exist, such as TIME_FROM or TIME_BETWEEN
http://gerrit.cloudera.org:8080/#/c/17765/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java:
http://gerrit.cloudera.org:8080/#/c/17765/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@4871
PS4, Line 4871: iceT);
nit. May add a test for a time in future, say 3 days from now().
http://gerrit.cloudera.org:8080/#/c/17765/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@4880
PS4, Line 4880:
nit. May add a negative test case on system version of -10.
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Aug 2021 18:45:39 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Attila Jeges, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17765
to look at the new patch set (#3).
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
This patch adds support "FOR SYSTEM_TIME AS OF" and
"FOR SYSTEM_VERSION AS OF" clauses for Iceberg tables. The new
clauses are part of the table ref. FOR SYSTEM_TIME AS OF conforms to the
SQL2011 standard:
https://cs.ulb.ac.be/public/_media/teaching/infoh415/tempfeaturessql2011.pdf
The timestamp given to "FOR SYSTEM_TIME AS OF" is interpreted in the
local timezone. The local timezone can be set via the query option
TIMEZONE. By default the timezone being used is the coordinator node's
local timezone.
"FOR SYSTEM_VERSION AS OF" is a non-standard extension that can be used
to query a table via a snapshot ID. HIVE-25344 also added support for
these clauses to Hive.
Sample queries:
SELECT * FROM t FOR SYSTEM_TIME AS OF now();
SELECT * FROM t FOR SYSTEM_TIME AS OF '2021-08-10 11:02:34';
SELECT * FROM t FOR SYSTEM_TIME AS OF now() - interval 10 days + interval 3 hours;
SELECT * FROM t FOR SYSTEM_VERSION AS OF 7080861547601448759;
SELECT * FROM t FOR SYSTEM_TIME AS OF now()
MINUS
SELECT * FROM t FOR SYSTEM_TIME AS OF now() - interval 1 days;
This patch uses some parts of the in-progress
IMPALA-9773 (https://gerrit.cloudera.org/#/c/13342/) developed by
Todd Lipcon and Grant Henke. This patch also resolves some TODOs of
IMPALA-9773, i.e. after this patch it'll be easier to add
time travel for Kudu tables as well.
Testing:
* added parser tests (ParserTest.java)
* added analyzer tests (AnalyzeStmtsTest.java)
* added e2e tests (test_iceberg.py)
Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
---
M be/src/common/init.cc
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/ColumnDef.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
A fe/src/main/java/org/apache/impala/analysis/TimeTravelSpec.java
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
A fe/src/main/java/org/apache/impala/util/ExprUtil.java
M fe/src/main/java/org/apache/impala/util/IcebergUtil.java
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
19 files changed, 545 insertions(+), 63 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/17765/3
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Attila Jeges (Code Review)" <ge...@cloudera.org>.
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17765/1/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:
http://gerrit.cloudera.org:8080/#/c/17765/1/tests/query_test/test_iceberg.py@197
PS1, Line 197: # Query old snapshot
Maybe add another test to query with a timestamp in the future.
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 25 Aug 2021 11:03:10 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 6:
(1 comment)
Thanks for the review!
http://gerrit.cloudera.org:8080/#/c/17765/5/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:
http://gerrit.cloudera.org:8080/#/c/17765/5/tests/query_test/test_iceberg.py@116
PS5, Line 116: def test_time_travel(self, vector, unique_database):
> One more case that can be added to JIRA above is time travel's behaviour wi
With Iceberg V1 we can only query the old snapshot with the current schema. With V2 it's possible to retrieve the older schema: https://github.com/apache/iceberg/issues/1029
I opened a Jira for this subject: IMPALA-10893
Interestingly MS SQL SERVER also uses the current table schema for time travel queries: http://sqlfiddle.com/#!18/42e613/2/0
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Aug 2021 16:35:35 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Attila Jeges, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17765
to look at the new patch set (#4).
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
This patch adds support "FOR SYSTEM_TIME AS OF" and
"FOR SYSTEM_VERSION AS OF" clauses for Iceberg tables. The new
clauses are part of the table ref. FOR SYSTEM_TIME AS OF conforms to the
SQL2011 standard:
https://cs.ulb.ac.be/public/_media/teaching/infoh415/tempfeaturessql2011.pdf
The timestamp given to "FOR SYSTEM_TIME AS OF" is interpreted in the
local timezone. The local timezone can be set via the query option
TIMEZONE. By default the timezone being used is the coordinator node's
local timezone.
"FOR SYSTEM_VERSION AS OF" is a non-standard extension that can be used
to query a table via a snapshot ID. HIVE-25344 also added support for
these clauses to Hive.
Sample queries:
SELECT * FROM t FOR SYSTEM_TIME AS OF now();
SELECT * FROM t FOR SYSTEM_TIME AS OF '2021-08-10 11:02:34';
SELECT * FROM t FOR SYSTEM_TIME AS OF now() - interval 10 days + interval 3 hours;
SELECT * FROM t FOR SYSTEM_VERSION AS OF 7080861547601448759;
SELECT * FROM t FOR SYSTEM_TIME AS OF now()
MINUS
SELECT * FROM t FOR SYSTEM_TIME AS OF now() - interval 1 days;
This patch uses some parts of the in-progress
IMPALA-9773 (https://gerrit.cloudera.org/#/c/13342/) developed by
Todd Lipcon and Grant Henke. This patch also resolves some TODOs of
IMPALA-9773, i.e. after this patch it'll be easier to add
time travel for Kudu tables as well.
Testing:
* added parser tests (ParserTest.java)
* added analyzer tests (AnalyzeStmtsTest.java)
* added e2e tests (test_iceberg.py)
Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
---
M be/src/common/init.cc
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/ColumnDef.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
A fe/src/main/java/org/apache/impala/analysis/TimeTravelSpec.java
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
A fe/src/main/java/org/apache/impala/util/ExprUtil.java
M fe/src/main/java/org/apache/impala/util/IcebergUtil.java
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
19 files changed, 570 insertions(+), 63 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/17765/4
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Qifan Chen (Code Review)" <ge...@cloudera.org>.
Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 5: Code-Review+2
Based on Attila +2 and Amogh's +1.
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Aug 2021 16:19:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/9269/ : 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/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Aug 2021 11:37:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 4:
(6 comments)
Thanks for the comments!
http://gerrit.cloudera.org:8080/#/c/17765/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17765/4//COMMIT_MSG@14
PS4, Line 14:
> May need to describe the semantics of FOR SYSTEM_TIME AS OF and FOR SYSTEM_
Extended the commit message.
http://gerrit.cloudera.org:8080/#/c/17765/4//COMMIT_MSG@18
PS4, Line 18: local timezone.
> nit. Translating the local time to the time zone used for all the rows in t
Added a sentence that timestamps are translated to UTC because the snapshots use UTC.
http://gerrit.cloudera.org:8080/#/c/17765/4/fe/src/main/java/org/apache/impala/analysis/TimeTravelSpec.java
File fe/src/main/java/org/apache/impala/analysis/TimeTravelSpec.java:
http://gerrit.cloudera.org:8080/#/c/17765/4/fe/src/main/java/org/apache/impala/analysis/TimeTravelSpec.java@39
PS4, Line 39: TIME
> nit. May use TIME_AS_OF. Other possibilities exist, such as TIME_FROM or TI
Done.
Updated VERSION to VERSION_AS_OF as well because we might add similar extensions for version-based time travel.
http://gerrit.cloudera.org:8080/#/c/17765/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java:
http://gerrit.cloudera.org:8080/#/c/17765/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@4871
PS4, Line 4871: iceT);
> nit. May add a test for a time in future, say 3 days from now().
Yeah I wasn't sure about what should be the behavior of future queries. Iceberg returns the current snapshot for them.
MS SQL SERVER also returns the current state for future queries:
http://sqlfiddle.com/#!18/fb878/10/0
So I might just add tests for this behavior.
http://gerrit.cloudera.org:8080/#/c/17765/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@4880
PS4, Line 4880:
> nit. May add a negative test case on system version of -10.
version as of -10 raises a parse error. There's a test for it in ParserTest.java
http://gerrit.cloudera.org:8080/#/c/17765/1/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:
http://gerrit.cloudera.org:8080/#/c/17765/1/tests/query_test/test_iceberg.py@197
PS1, Line 197: SELECT * FROM {tbl} FOR SYSTEM_VERSION AS OF {v_old}""".format(
> Sure, use your best judgement.
Added a test query with a future timestamp.
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Aug 2021 10:00:01 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Attila Jeges (Code Review)" <ge...@cloudera.org>.
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 1:
(3 comments)
The patch looks good. At first glance I found only minor issues.
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/main/java/org/apache/impala/util/IcebergUtil.java
File fe/src/main/java/org/apache/impala/util/IcebergUtil.java:
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/main/java/org/apache/impala/util/IcebergUtil.java@515
PS1, Line 515: TableScan scan = createScanAsOf(
nit: Since 'baseTable' is not used anywhere else, you could move L514 inside createScanAsOf(). This way createScanAsOf() would need only 2 params : table and timeTravelSpec.
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java:
http://gerrit.cloudera.org:8080/#/c/17765/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@4884
PS1, Line 4884: iceT, "FOR SYSTEM_VERSION AS OF <expression> must be an integer type but is");
The end of the error msg was left out intentionally?
http://gerrit.cloudera.org:8080/#/c/17765/1/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:
http://gerrit.cloudera.org:8080/#/c/17765/1/tests/query_test/test_iceberg.py@134
PS1, Line 134: ts
'snapshot_id' ?
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Aug 2021 18:01:16 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Attila Jeges (Code Review)" <ge...@cloudera.org>.
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Aug 2021 15:09:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/9375/ : 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/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Aug 2021 12:35:42 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/9377/ : 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/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Aug 2021 17:05:33 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/9398/ : 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/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 7
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Aug 2021 12:50:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 8:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7436/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 8
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Aug 2021 12:29:12 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10840: Add support for "FOR SYSTEM TIME AS OF" and "FOR SYSTEM VERSION AS OF" for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables
......................................................................
Patch Set 8: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/17765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824
Gerrit-Change-Number: 17765
Gerrit-PatchSet: 8
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Aug 2021 12:29:11 +0000
Gerrit-HasComments: No