[GitHub] [flink] luoyuxia commented on a diff in pull request #22928: [FLINK-32473] Introduce base interfaces for time travel

["luoyuxia (via GitHub)" <gi...@apache.org>]

luoyuxia commented on code in PR #22928:
URL: https://github.com/apache/flink/pull/22928#discussion_r1250315785


##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/catalog/CatalogTable.java:
##########
@@ -120,4 +139,9 @@ default TableKind getTableKind() {
     default Map<String, String> toProperties() {
         return Collections.emptyMap();
     }
+
+    /** Return the snapshot specified for the table. Return Optional.empty() if not specified. */
+    default Optional<TableSnapshot> getSnapshot() {

Review Comment:
   Please keep it same as the proposed FLIP.



##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/catalog/Catalog.java:
##########
@@ -244,6 +244,23 @@ void alterDatabase(String name, CatalogDatabase newDatabase, boolean ignoreIfNot
      */
     CatalogBaseTable getTable(ObjectPath tablePath) throws TableNotExistException, CatalogException;
 
+    /**
+     * Returns a {@link CatalogTable} or {@link CatalogView} identified by the given {@link

Review Comment:
   suggestion:
   ```
   Returns a {@link CatalogTable} or {@link CatalogView} at a specific time identified by the given {@link  ObjectPath}.
   ```



##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/catalog/Catalog.java:
##########
@@ -244,6 +244,24 @@ void alterDatabase(String name, CatalogDatabase newDatabase, boolean ignoreIfNot
      */
     CatalogBaseTable getTable(ObjectPath tablePath) throws TableNotExistException, CatalogException;
 
+    /**
+     * Returns a {@link CatalogTable} or {@link CatalogView} identified by the given {@link
+     * ObjectPath} and Timestamp. The framework will resolve the metadata objects when necessary.
+     *
+     * @param tablePath Path of the table or view
+     * @param timestamp Timestamp of the table snapshot, which is milliseconds since 1970-01-01
+     *     00:00:00 UTC
+     * @return The requested table or view
+     * @throws TableNotExistException if the target does not exist
+     * @throws CatalogException in case of any runtime exception
+     */
+    default CatalogBaseTable getTable(ObjectPath tablePath, long timestamp)
+            throws TableNotExistException, CatalogException {
+        throw new UnsupportedOperationException(
+                String.format(
+                        "getTable with snapshot is not implemented for %s.", this.getClass()));

Review Comment:
   `getTable with snapshot` => `getTable(ObjectPath, long)` ?



-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org