gengliangwang commented on code in PR #56481:
URL: https://github.com/apache/spark/pull/56481#discussion_r3431947971
##########
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:
`visitTableName` now reconciles two time-travel sources inline (the `@` spec
and the `AS OF` `temporalClause`): read the `@` spec, reject the both-specified
case, conditionally wrap in `RelationTimeTravel`, then still
`optionalMap(temporalClause)(withTimeTravel)`. That's a fair bit heavier than
the original 4-line method.
Note it can't simply route through `TemporalIdentifier.wrapTimeTravel` here:
this path needs the `IdentifierReferenceContext` (for `IDENTIFIER(...)`, via
`createUnresolvedRelation` -> `withIdentClause`), which `TemporalIdentifier`'s
`Seq[String]` nameParts can't carry. So the clean reduction is a plain
*plan-level* helper that owns the conflict-check + source-selection + wrap,
leaving `visitTableName` as its original flat `optionalMap` chain:
```scala
val relation = createUnresolvedRelation(ttCtx.identifierReference,
Option(ctx.optionsClause))
val withTT = withTableTimeTravel(relation, ttCtx, ctx.temporalClause)
val table = mayApplyAliasPlan(ctx.tableAlias, withTT)
// ... sample / watermark as before
/** Applies the '@' suffix and/or the AS OF clause, rejecting more than one.
*/
private def withTableTimeTravel(
relation: LogicalPlan,
ttCtx: TemporalTableIdentifierReferenceContext,
clause: TemporalClauseContext): LogicalPlan = {
val (atTs, atVer) = temporalSpec(ttCtx, ttCtx.timestamp, ttCtx.version)
if ((atTs.isDefined || atVer.isDefined) && clause != null) {
withOrigin(clause)(throw
QueryParsingErrors.multipleTimeTravelSpec(clause))
}
val withAt =
if (atTs.isDefined || atVer.isDefined) RelationTimeTravel(relation,
atTs, atVer) else relation
withAt.optionalMap(clause)(withTimeTravel)
}
```
This isolates the new behavior in one named, testable helper. The
reader/Connect paths have no `AS OF` clause (so no conflict to check) and keep
the simpler `wrapTimeTravel`.
--
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]
