cloud-fan commented on a change in pull request #34497:
URL: https://github.com/apache/spark/pull/34497#discussion_r747999585
##########
File path:
sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/SupportsCatalogOptions.java
##########
@@ -52,4 +52,15 @@
default String extractCatalog(CaseInsensitiveStringMap options) {
return CatalogManager.SESSION_CATALOG_NAME();
}
+
+ /**
+ * Return a {@link TimeTravelSpec} instance given DataFrameReader options.
+ *
+ * @param options the user-specified options that can identify a table. It's
an immutable
+ * case-insensitive string-to-string map. The version or timestamp in the
options
+ * is used to construct a TimeTravelSpec.
+ */
+ default TimeTravelSpec extractTimeTravelSpec(CaseInsensitiveStringMap
options) {
Review comment:
Some thoughts:
1. I think we should not ask the implementation to convert timestamp string
to long. It's better to always let Spark do the conversion so that we can
guarantee a clear timestamp semantic. That said, `TimeTravelSpec.timestamp`
should be a string type.
2. The timestamp and version can't be both specified, but can be both
non-specified.
I think what we need here is just a `(Option[String], Option[String])`, but
unfortunately java SE does not have Tuple2 class. My proposal is to have 2
methods
```
default Optional<String> extractTimeTravelTimestamp(...)
default Optional<String> extractTimeTravelVersion(...)
```
The caller side should always call these 2 methods and make sure they are
not both specified. Then `TimeTravelSpec` can just be an internal thing and we
can write it in Scala.
--
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]
