cloud-fan commented on code in PR #55487:
URL: https://github.com/apache/spark/pull/55487#discussion_r3189512141
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala:
##########
@@ -323,6 +323,38 @@ class DataSourceV2Strategy(session: SparkSession) extends
Strategy with Predicat
CreateV2ViewExec(catalog.asInstanceOf[ViewCatalog], ident,
userSpecifiedColumns, comment,
collation, properties, sqlText, child, allowExisting, replace,
viewSchemaMode) :: Nil
+ // CREATE VIEW ... WITH METRICS on a non-session v2 catalog. Routes the
metric-view path
+ // through `CreateV2MetricViewExec`, which extends `V2ViewPreparation` to
share the
+ // `IF NOT EXISTS` short-circuit, `OR REPLACE`, and cross-type-collision
decoding with
+ // `CreateV2ViewExec`. Session-catalog dispatch stays in
`CreateMetricViewCommand.run`.
+ case CreateMetricViewCommand(
+ ResolvedIdentifier(catalog, ident), userSpecifiedColumns, comment,
properties,
+ originalText, allowExisting, replace) if
!CatalogV2Util.isSessionCatalog(catalog) =>
+ val viewCatalog = catalog match {
+ case vc: ViewCatalog => vc
+ case _ => throw
QueryCompilationErrors.missingCatalogViewsAbilityError(catalog)
+ }
+ // Parse + analyze the YAML body here (during planning). This mirrors
the v1 path's
+ // late analysis in `CreateMetricViewCommand.run` -- the metric-view
source plan is not
+ // a SQL string, so it can't ride along as a regular `query`
`LogicalPlan` field on the
+ // logical command the way `CreateView` does. Pass the full multi-part
name so v2 metric
+ // views with multi-level-namespace targets analyze correctly
(`asTableIdentifier` would
+ // throw `requiresSinglePartNamespaceError` for namespace arity > 1).
+ val nameParts = (catalog.name() +: ident.namespace().toIndexedSeq) :+
ident.name()
+ val (analyzed, metricView) = MetricViewHelper.analyzeMetricViewText(
+ session, nameParts, originalText)
+ val mergedProps = properties ++ metricView.getProperties
+ val depParts = MetricViewHelper.collectTableDependencies(analyzed)
+ // Always emit a `Some(DependencyList)` for metric views (even when
`depParts` is empty,
+ // e.g. `SQLSource("SELECT 1 AS x")`): per `ViewInfo.viewDependencies()`
Javadoc, `null`
+ // means "no dependency list was supplied" while an empty list means
"supplied but the
+ // object has none". Metric-view CREATE always *computes* deps, so the
right empty
+ // representation is `Some(empty list)`, not `None`.
Review Comment:
Tiny citation nit, not blocking: the empty-list-vs-null distinction is
documented on `DependencyList`'s class-level Javadoc (which spells out all
three states explicitly: null / empty / non-empty), not on
`ViewInfo.viewDependencies()` (whose Javadoc only mentions the null case). The
behavior description in the comment is correct, just attributed to a doc that
doesn't fully cover it. Easy to retarget the citation:
```suggestion
// Always emit a `Some(DependencyList)` for metric views (even when
`depParts` is empty,
// e.g. `SQLSource("SELECT 1 AS x")`): per `DependencyList`'s
contract, `null` means
// "no dependency list was supplied" while an empty list means
"supplied but the
// object has none". Metric-view CREATE always *computes* deps, so the
right empty
// representation is `Some(empty list)`, not `None`.
```
--
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]
