sotikoug83 commented on code in PR #56481:
URL: https://github.com/apache/spark/pull/56481#discussion_r3487885475


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -6994,6 +6994,17 @@ object SQLConf {
       .stringConf
       .createWithDefault("versionAsOf")
 
+  val TIME_TRAVEL_AT_SYNTAX_ENABLED =
+    buildConf("spark.sql.timeTravel.atSyntax.enabled")
+      .doc("When true, a table name in a query or in table-reading APIs can 
carry a time " +
+        "travel suffix: 'name@v123' reads version 123 of the table, and " +
+        "'name@20240101000000000' (format yyyyMMddHHmmssSSS, interpreted in 
the session " +
+        "time zone) reads the table as of that timestamp. When false, '@' in 
table names " +
+        "fails at parse time.")
+      .version("5.0.0")
+      .booleanConf
+      .createWithDefault(true)

Review Comment:
   Thank you for raising this, decided default remains true.



##########
sql/connect/server/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala:
##########
@@ -1671,10 +1671,17 @@ class SparkConnectPlanner(
 
     rel.getReadTypeCase match {
       case proto.Read.ReadTypeCase.NAMED_TABLE =>
-        UnresolvedRelation(
-          
parser.parseMultipartIdentifier(rel.getNamedTable.getUnparsedIdentifier),
+        val temporalIdent =
+          
parser.parseTemporalTableIdentifier(rel.getNamedTable.getUnparsedIdentifier)
+        if (temporalIdent.isTemporal && rel.getIsStreaming) {

Review Comment:
   Fixed.



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala:
##########
@@ -2634,13 +2640,83 @@ class AstBuilder extends DataTypeAstBuilder
    * Create an aliased table reference. This is typically used in FROM clauses.
    */
   override def visitTableName(ctx: TableNameContext): LogicalPlan = 
withOrigin(ctx) {
-    val relation = createUnresolvedRelation(ctx.identifierReference, 
Option(ctx.optionsClause))
+    val ttCtx = ctx.temporalTableIdentifierReference
+    val (atTimestamp, atVersion) = temporalSpec(ttCtx, ttCtx.timestamp, 
ttCtx.version)
+    val hasAtSpec = atTimestamp.isDefined || atVersion.isDefined
+    if (hasAtSpec && ctx.temporalClause != null) {
+      withOrigin(ctx.temporalClause) {
+        throw QueryParsingErrors.multipleTimeTravelSpec(ctx.temporalClause)
+      }
+    }
+    val relation = createUnresolvedRelation(ttCtx.identifierReference, 
Option(ctx.optionsClause))
+    val withAtSpec = if (hasAtSpec) {
+      RelationTimeTravel(relation, atTimestamp, atVersion)

Review Comment:
   Fixed.



##########
sql/connect/server/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala:
##########
@@ -1671,10 +1671,17 @@ class SparkConnectPlanner(
 
     rel.getReadTypeCase match {
       case proto.Read.ReadTypeCase.NAMED_TABLE =>
-        UnresolvedRelation(
-          
parser.parseMultipartIdentifier(rel.getNamedTable.getUnparsedIdentifier),
+        val temporalIdent =
+          
parser.parseTemporalTableIdentifier(rel.getNamedTable.getUnparsedIdentifier)
+        if (temporalIdent.isTemporal && rel.getIsStreaming) {
+          throw QueryCompilationErrors.timeTravelUnsupportedError(
+            QuotingUtils.quoteNameParts(temporalIdent.nameParts))

Review Comment:
   Fixed.



-- 
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]

Reply via email to