Add time travel syntax support for iceberg tables (VERSION and TIMESTAMP).#20991
Add time travel syntax support for iceberg tables (VERSION and TIMESTAMP).#20991tdcmeehan merged 1 commit intoprestodb:masterfrom
Conversation
d9d24f6 to
bfa95ce
Compare
tdcmeehan
left a comment
There was a problem hiding this comment.
What happens if I try to use a connector that doesn't support time travel? Is it unsupported operaton exception?
presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergAbstractMetadata.java
Outdated
Show resolved
Hide resolved
presto-main/src/main/java/com/facebook/presto/metadata/Metadata.java
Outdated
Show resolved
Hide resolved
presto-parser/src/main/java/com/facebook/presto/sql/parser/AstBuilder.java
Outdated
Show resolved
Hide resolved
presto-spi/src/main/java/com/facebook/presto/spi/connector/ConnectorMetadata.java
Outdated
Show resolved
Hide resolved
7e0cb68 to
f301787
Compare
It will give an error. |
14d69a2 to
0ff1b7e
Compare
steveburnett
left a comment
There was a problem hiding this comment.
Thanks for the documentation! I made some suggestions to simplify the text, and have a question about TIMESTAMP (value) and CURRENT_TIMESTAMP for you to consider.
Let me know what you think, please.
0ff1b7e to
4378d9a
Compare
steveburnett
left a comment
There was a problem hiding this comment.
Thanks, looks great! I made a single suggestion in the new text, but everything else is good.
4378d9a to
d51c2c3
Compare
d51c2c3 to
af096ad
Compare
presto-main/src/main/java/com/facebook/presto/metadata/MetadataUtil.java
Outdated
Show resolved
Hide resolved
presto-spi/src/main/java/com/facebook/presto/spi/connector/ConnectorMetadata.java
Outdated
Show resolved
Hide resolved
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff e8e0798...ddba425.
|
af096ad to
43b9fdc
Compare
hantangwangd
left a comment
There was a problem hiding this comment.
Some little problems and comment for discussion.
presto-parser/src/main/java/com/facebook/presto/sql/SqlFormatter.java
Outdated
Show resolved
Hide resolved
presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java
Outdated
Show resolved
Hide resolved
presto-parser/src/main/java/com/facebook/presto/sql/tree/AstVisitor.java
Outdated
Show resolved
Hide resolved
43b9fdc to
7f740ff
Compare
hantangwangd
left a comment
There was a problem hiding this comment.
Do you mind add a test case for sql with time travel syntax being parsed to statement and formatted back?
presto-parser/src/main/java/com/facebook/presto/sql/SqlFormatter.java
Outdated
Show resolved
Hide resolved
presto-parser/src/main/java/com/facebook/presto/sql/SqlFormatter.java
Outdated
Show resolved
Hide resolved
7f740ff to
24180f2
Compare
ZacBlanco
left a comment
There was a problem hiding this comment.
Two minor questions. Otherwise PR looks good!
presto-parser/src/main/antlr4/com/facebook/presto/sql/parser/SqlBase.g4
Outdated
Show resolved
Hide resolved
presto-spi/src/main/java/com/facebook/presto/spi/connector/ConnectorMetadata.java
Outdated
Show resolved
Hide resolved
presto-main/src/main/java/com/facebook/presto/metadata/Metadata.java
Outdated
Show resolved
Hide resolved
24180f2 to
65f2f7c
Compare
Added test cases in |
65f2f7c to
e513254
Compare
e513254 to
4f7a313
Compare
4f7a313 to
ddba425
Compare
Confirming that Prestissimo eval is not affected by this change rightaway
@tdcmeehan : Confirming this PR doesn't affect Prestissimo at this point. |
Description
Presto issue : #20495
Note - This PR has syntax and engine side changes. For iceberg connector changes, test and doc, a new PR will be created.
This feature will allow iceberg connector to query historical data using
AS OFsyntax on a table.Time travel version option will read bigint snapshot id value for the table. Time travel timestamp option will read
timestamp-with-time-zone value for the table.
examples :
Note - TIMESTAMP and VERSION are the alias names for SYSTEM_TIME and SYSTEM_VERSION respectively
Design description :
Parser
AS OFsyntax on table referenceSemantic Analyzer
Metadata
Limitations :
Motivation and Context
Right now, we only support time travel via system tables and system procedures.
The current support does not include a convenient way to select a particular snapshot or the ability to select a snapshot as of a particular timestamp. Using system tables and system procedures to do this is not convenient, and it also is not safe per-user, since the system table alters the table for everyone, not just the current user.
See Trino implementation, which has syntax level support for going back to a particular snapshot via
FOR VERSION AS OF, and to go back to a period in time without having to first fetch a snapshot id viaFOR TIMESTAMP AS OF. We should consider a similar feature for convenience of time travel.Impact
Test Plan
-manual testing
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.
Note - Iceberg PR will have release notes and doc section