morningman commented on code in PR #15418:
URL: https://github.com/apache/doris/pull/15418#discussion_r1058934476
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/TableRef.java:
##########
@@ -499,6 +515,27 @@ protected void analyzeHints() throws AnalysisException {
}
}
+ public void analyzeTableSnapshot(Analyzer analyzer) throws
AnalysisException {
Review Comment:
```suggestion
protected void analyzeTableSnapshot(Analyzer analyzer) throws
AnalysisException {
```
##########
regression-test/suites/external_table_emr_p2/iceberg/test_external_catalog_icebergv2.groovy:
##########
@@ -24,9 +24,9 @@ suite("test_external_catalog_icebergv2", "p2") {
sql """drop catalog if exists ${catalog_name};"""
sql """
- create catalog if not exists ${catalog_name} properties (
+ create catalog if not exists test_external_catalog_iceberg
properties (
'type'='hms',
- 'hive.metastore.uris' =
'thrift://${extHiveHmsHost}:${extHiveHmsPort}'
+ 'hive.metastore.uris' = 'thrift://172.21.16.47:7004'
Review Comment:
Change this
##########
fe/fe-core/src/main/java/org/apache/doris/planner/external/IcebergScanProvider.java:
##########
@@ -168,6 +176,18 @@ public List<InputSplit> getSplits(List<Expr> exprs) throws
UserException {
org.apache.iceberg.Table table = getIcebergTable();
TableScan scan = table.newScan();
+ TableSnapshot tableSnapshot = desc.getRef().getTableSnapshot();
+ if (tableSnapshot != null) {
+ TableSnapshot.VersionType type = tableSnapshot.getType();
+ if (type == TableSnapshot.VersionType.VERSION) {
+ scan = scan.useSnapshot(tableSnapshot.getVersion());
+ } else {
+ LocalDateTime asOfTime =
LocalDateTime.parse(tableSnapshot.getTime(), DATE_TIME_FORMATTER);
Review Comment:
You can use `TimeUtil.timeStringToLong()`
##########
fe/fe-core/src/main/java/org/apache/doris/planner/external/IcebergScanProvider.java:
##########
@@ -168,6 +176,18 @@ public List<InputSplit> getSplits(List<Expr> exprs) throws
UserException {
org.apache.iceberg.Table table = getIcebergTable();
TableScan scan = table.newScan();
+ TableSnapshot tableSnapshot = desc.getRef().getTableSnapshot();
+ if (tableSnapshot != null) {
+ TableSnapshot.VersionType type = tableSnapshot.getType();
+ if (type == TableSnapshot.VersionType.VERSION) {
+ scan = scan.useSnapshot(tableSnapshot.getVersion());
Review Comment:
What if user give a non-exist snapshot version?
##########
fe/fe-core/src/main/java/org/apache/doris/planner/external/IcebergScanProvider.java:
##########
@@ -168,6 +176,18 @@ public List<InputSplit> getSplits(List<Expr> exprs) throws
UserException {
org.apache.iceberg.Table table = getIcebergTable();
TableScan scan = table.newScan();
+ TableSnapshot tableSnapshot = desc.getRef().getTableSnapshot();
+ if (tableSnapshot != null) {
+ TableSnapshot.VersionType type = tableSnapshot.getType();
+ if (type == TableSnapshot.VersionType.VERSION) {
+ scan = scan.useSnapshot(tableSnapshot.getVersion());
Review Comment:
Catch the `NotFoundException` and throw `UserException`
##########
fe/fe-core/src/main/cup/sql_parser.cup:
##########
@@ -5095,12 +5098,33 @@ base_table_ref_list ::=
;
base_table_ref ::=
- table_name:name opt_partition_names:partitionNames
opt_tablet_list:tabletIds opt_table_alias:alias opt_table_sample:tableSample
opt_common_hints:commonHints
+ table_name:name opt_partition_names:partitionNames
opt_tablet_list:tabletIds opt_table_alias:alias opt_table_sample:tableSample
opt_common_hints:commonHints opt_snapshot_version:snapshotVersion
{:
- RESULT = new TableRef(name, alias, partitionNames, tabletIds,
tableSample, commonHints);
+ RESULT = new TableRef(name, alias, partitionNames, tabletIds,
tableSample, commonHints, snapshotVersion);
:}
;
+opt_snapshot_version ::=
+ /* empty */
+ {:
+ RESULT = null;
+ :}
+ | snapshot_version:snapshotVersion
+ {:
+ RESULT = snapshotVersion;
+ :}
+ ;
+
+snapshot_version ::=
+ KW_FOR KW_VERSION KW_AS KW_OF STRING_LITERAL:version
Review Comment:
`time` should be string and `version` should be integer.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/TableRef.java:
##########
@@ -499,6 +515,27 @@ protected void analyzeHints() throws AnalysisException {
}
}
+ public void analyzeTableSnapshot(Analyzer analyzer) throws
AnalysisException {
+ if (tableSnapshot == null) {
+ return;
+ }
+ TableIf.TableType tableType = this.getTable().getType();
+ if (tableType != TableIf.TableType.HMS_EXTERNAL_TABLE) {
+
ErrorReport.reportAnalysisException(ErrorCode.ERR_NONSUPPORT_TIME_TRAVEL_TABLE);
+ }
+ HMSExternalTable extTable = (HMSExternalTable) this.getTable();
+ if (extTable.getDlaType() != HMSExternalTable.DLAType.ICEBERG) {
Review Comment:
The dlaType may not be initialized, you can wrap this logic inside the
`HMSExternalTable`, and call `makeSureInitialized()` before using `dlaType`
##########
fe/fe-core/src/main/java/org/apache/doris/planner/external/IcebergScanProvider.java:
##########
@@ -75,6 +82,7 @@
*/
public class IcebergScanProvider extends HiveScanProvider {
+ private static final DateTimeFormatter DATE_TIME_FORMATTER =
DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss");
Review Comment:
No need to define this. All these can be found in TimeUtil
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
