cloud-fan commented on code in PR #55487:
URL: https://github.com/apache/spark/pull/55487#discussion_r3176522065
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala:
##########
@@ -323,6 +324,36 @@ 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 = MetricViewHelper.analyzeMetricViewText(
+ session, nameParts, originalText)
+ val metricView = MetricViewFactory.fromYAML(originalText)
+ val mergedProps = properties ++ metricView.getProperties
Review Comment:
The v2 path merges `metric_view.from.type` / `metric_view.from.name` /
`metric_view.from.sql` / `metric_view.where` into the property bag here
(`properties ++ metricView.getProperties`), but the v1 path in
`CreateMetricViewCommand.createMetricViewInSessionCatalog`
(metricViewCommands.scala:74-78) passes `properties` to `prepareTable` without
the merge. Result: `DESCRIBE TABLE EXTENDED` on a session-catalog metric view
doesn't show these descriptor rows; on a v2 metric view, it does.
The PR description states the merge happens "into the view's properties bag"
without a v1/v2 carve-out — please confirm this is intentional, or merge in v1
too.
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/DependencyList.java:
##########
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.connector.catalog;
+
+import java.util.Objects;
+
+import org.apache.spark.annotation.Evolving;
+
+/**
+ * A list of dependencies for a SQL object such as a view or metric view.
+ * <p>
+ * <ul>
+ * <li>When {@code null}, the dependency information is not provided.</li>
+ * <li>When the array is empty, dependencies are provided but the object has
none.</li>
+ * <li>When the array is non-empty, each entry describes one dependency.</li>
+ * </ul>
+ *
+ * @param dependencies array of dependencies
+ * @since 4.2.0
+ */
+@Evolving
+public record DependencyList(Dependency[] dependencies) {
Review Comment:
Java records auto-generate `equals`/`hashCode` from per-field
`Objects.equals`/`Objects.hashCode`, which on array fields fall through to
`Object.equals` (reference equality). So
`DependencyList.of(Dependency.table("a","b")).equals(DependencyList.of(Dependency.table("a","b")))
== false`. Same issue on `TableDependency.nameParts` (TableDependency.java:42)
and `FunctionDependency.nameParts` (FunctionDependency.java:34).
This undermines the "structural multi-part name" intent — consumers can't
dedup, compare, or use these as Map keys. Please override `equals`/`hashCode`
to use `Arrays.equals`/`Arrays.hashCode` on each record (and
`Arrays.deepEquals`/`Arrays.deepHashCode` on `DependencyList`), or expose
`List<...>` instead of `[]` to inherit value semantics for free.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala:
##########
@@ -323,6 +324,36 @@ 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 = MetricViewHelper.analyzeMetricViewText(
+ session, nameParts, originalText)
+ val metricView = MetricViewFactory.fromYAML(originalText)
Review Comment:
`MetricViewHelper.analyzeMetricViewText` (called on line 345) already parses
the YAML internally via `MetricViewPlanner.parseYAML` →
`MetricViewFactory.fromYAML`. Re-parsing here duplicates the work.
`analyzeMetricViewText` could return `(LogicalPlan, MetricView)` (or expose the
parsed `MetricView` via the `MetricViewPlaceholder` it builds) so the strategy
doesn't pay the parse twice.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala:
##########
@@ -354,6 +385,16 @@ class DataSourceV2Strategy(session: SparkSession) extends
Strategy with Predicat
RenameV2ViewExec(
catalog.asInstanceOf[ViewCatalog], ident, newName.asIdentifier) :: Nil
+ case ShowCreateTable(rpv @ ResolvedPersistentView(catalog, ident, _), _, _)
+ if rpv.info.properties.get(TableCatalog.PROP_TABLE_TYPE) ==
+ TableSummary.METRIC_VIEW_TABLE_TYPE =>
+ // SHOW CREATE TABLE on a metric view is explicitly unsupported:
`ShowCreateV2ViewExec`
+ // would emit a plain `CREATE VIEW <ident> AS <yaml>`, which is not a
round-trippable
+ // metric-view DDL form (the right form is `CREATE VIEW <ident> WITH
METRICS LANGUAGE
+ // YAML AS $$ <yaml> $$`). Reject up front rather than emit lossy DDL.
+ throw QueryCompilationErrors.unsupportedTableOperationError(
Review Comment:
The v1 path uses the dedicated
`UNSUPPORTED_SHOW_CREATE_TABLE.ON_METRIC_VIEW` (tables.scala:1219 →
`showCreateTableNotSupportedOnMetricViewError`); this v2 path uses the generic
`UNSUPPORTED_FEATURE.TABLE_OPERATION`. Same user action, two error classes —
the dedicated one carries a more actionable message ("not supported on metric
view"). `showCreateTableNotSupportedOnMetricViewError` takes a `String`; you
can format the multipart name the same way the case below does
(`(catalog.name() +:
ident.asMultipartIdentifier).map(quoteIfNeeded).mkString(".")`).
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala:
##########
@@ -563,8 +604,9 @@ class DataSourceV2Strategy(session: SparkSession) extends
Strategy with Predicat
}
case DropTable(r: ResolvedIdentifier, ifExists, purge) =>
+ val tableCatalog = r.catalog.asTableCatalog
Review Comment:
Extracting `r.catalog.asTableCatalog` into `tableCatalog` is a no-op
refactor unrelated to metric-view support and is not mentioned in the PR
description. Please revert or split into a separate cleanup PR — keeps the
metric-view PR focused.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/command/metricViewCommands.scala:
##########
@@ -73,15 +88,76 @@ case class CreateMetricViewCommand(
case class AlterMetricViewCommand(child: LogicalPlan, originalText: String)
object MetricViewHelper {
+
+ /**
+ * Walks the analyzed plan to collect direct table/view dependencies. Each
dependency is
+ * returned as a structural multi-part name (`Seq[String]`); arity is
preserved per source
+ * so consumers can reason about catalog / namespace / table boundaries
without parsing a
+ * dot-flattened string.
+ *
+ * Stops recursion at relation leaf nodes and persistent `View` nodes so
only direct
+ * (not transitive) dependencies are recorded.
+ */
+ private[execution] def collectTableDependencies(plan: LogicalPlan):
Seq[Seq[String]] = {
+ val tables = scala.collection.mutable.ArrayBuffer.empty[Seq[String]]
+ def traverse(p: LogicalPlan): Unit = p match {
+ case v: View if !v.isTempView =>
+ tables += v.desc.identifier.nameParts
+ case r: DataSourceV2Relation if r.catalog.isDefined &&
r.identifier.isDefined =>
+ val ident = r.identifier.get
+ // V2 catalogs may have multi-level namespaces; preserve the full
arity rather than
+ // dot-joining the namespace into a single component.
+ tables += (r.catalog.get.name() +: ident.namespace().toIndexedSeq) :+
ident.name()
+ case r: HiveTableRelation =>
+ tables += r.tableMeta.identifier.nameParts
+ case r: LogicalRelation if r.catalogTable.isDefined =>
+ tables += r.catalogTable.get.identifier.nameParts
Review Comment:
`TableIdentifier.nameParts` returns 1, 2, or 3 parts depending on which of
`catalog` / `database` are set on the resolved `CatalogTable.identifier`. The
same v1 source table can yield `[db, tbl]` or `[spark_catalog, db, tbl]` across
runs — the test in `MetricViewV2CatalogSuite` (the V1 source
dependency-extraction test) admits this with `parts.length >= 2 && parts.length
<= 3`. The `TableDependency` Javadoc (TableDependency.java:32-33) acknowledges
it, but downstream consumers comparing dependency names structurally won't be
able to rely on stable arity. Same issue applies to the `View` arm (line 105)
and `HiveTableRelation` arm (line 112).
Consider normalizing v1 sources to always emit 3-part `[spark_catalog, db,
tbl]` (prepending the session-catalog name when missing) so consumers see a
stable shape per source kind. The test should then assert exactly 3 parts.
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/MetricViewV2CatalogSuite.scala:
##########
@@ -0,0 +1,1026 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution
+
+import java.util.concurrent.ConcurrentHashMap
+
+import scala.jdk.CollectionConverters._
+
+import org.apache.spark.sql.{AnalysisException, QueryTest}
+import org.apache.spark.sql.catalyst.analysis.{NoSuchViewException,
ViewAlreadyExistsException}
+import org.apache.spark.sql.connector.catalog.{Identifier,
InMemoryTableCatalog, MetadataTable, Table, TableCatalog, TableDependency,
TableSummary, TableViewCatalog, ViewInfo}
+import org.apache.spark.sql.metricview.serde.{AssetSource, Column, Constants,
DimensionExpression, MeasureExpression, MetricView, MetricViewFactory,
SQLSource}
+import org.apache.spark.sql.test.SharedSparkSession
+import org.apache.spark.sql.types.Metadata
+
+/**
+ * Tests that exercise
[[org.apache.spark.sql.execution.command.CreateMetricViewCommand]] on a
+ * non-session V2 catalog. Metric views are persisted through the same
[[ViewCatalog]] interface
+ * as plain views; the only marker that distinguishes them is `PROP_TABLE_TYPE
= METRIC_VIEW`
+ * plus the typed `viewDependencies` field on [[ViewInfo]]. The recording
catalog used here is a
+ * minimal [[TableViewCatalog]] so the same instance can also host the source
table referenced by
+ * the metric view's YAML.
+ */
+class MetricViewV2CatalogSuite extends QueryTest with SharedSparkSession {
+
+ import testImplicits._
+
+ private val testCatalogName = "testcat"
+ private val testNamespace = "ns"
+ private val sourceTableName = "events"
+ private val fullSourceTableName =
+ s"$testCatalogName.$testNamespace.$sourceTableName"
+ private val metricViewName = "mv"
+ private val fullMetricViewName =
+ s"$testCatalogName.$testNamespace.$metricViewName"
+
+ private val metricViewColumns = Seq(
+ Column("region", DimensionExpression("region"), 0),
+ Column("count_sum", MeasureExpression("sum(count)"), 1))
+
+ private val testTableData = Seq(
+ ("region_1", 1, 5.0),
+ ("region_2", 2, 10.0))
+
+ override protected def beforeAll(): Unit = {
+ super.beforeAll()
+ spark.conf.set(
+ s"spark.sql.catalog.$testCatalogName",
+ classOf[MetricViewRecordingCatalog].getName)
+ // A catalog that does not implement ViewCatalog - used for the negative
gate test.
+ spark.conf.set(
+ s"spark.sql.catalog.${MetricViewV2CatalogSuite.noViewCatalogName}",
+ classOf[InMemoryTableCatalog].getName)
+ }
+
+ override protected def afterAll(): Unit = {
+ spark.conf.unset(s"spark.sql.catalog.$testCatalogName")
+ spark.conf.unset(
+ s"spark.sql.catalog.${MetricViewV2CatalogSuite.noViewCatalogName}")
+ super.afterAll()
+ }
+
+ private def withTestCatalogTables(body: => Unit): Unit = {
+ MetricViewRecordingCatalog.reset()
+ testTableData.toDF("region", "count", "price")
+ .createOrReplaceTempView("metric_view_v2_source")
+ try {
+ sql(
+ s"""CREATE TABLE $fullSourceTableName
+ |USING foo AS SELECT * FROM metric_view_v2_source""".stripMargin)
+ body
+ } finally {
+ // The metric-view ident `mv` may have ended up as either a view (most
tests) or as a
+ // pre-created table (a few negative tests pre-create a table at the
same ident to
+ // exercise cross-type collisions). Sweep both kinds so subsequent tests
in the suite
+ // start from a clean catalog state. Wrap each DROP in a Try because:
+ // - DROP VIEW IF EXISTS on a leftover *table* throws
WRONG_COMMAND_FOR_OBJECT_TYPE
+ // under master's new DropViewExec active-rejection contract.
+ // - DROP TABLE IF EXISTS on a leftover *view* throws the symmetric
error.
+ // - On a totally clean state both are silent no-ops.
+ scala.util.Try(sql(s"DROP VIEW IF EXISTS $fullMetricViewName"))
+ scala.util.Try(sql(s"DROP TABLE IF EXISTS $fullMetricViewName"))
+ scala.util.Try(sql(s"DROP TABLE IF EXISTS $fullSourceTableName"))
+ spark.catalog.dropTempView("metric_view_v2_source")
+ MetricViewRecordingCatalog.reset()
+ }
+ }
+
+ private def createMetricView(
+ name: String,
+ metricView: MetricView,
+ comment: Option[String] = None): String = {
+ val yaml = MetricViewFactory.toYAML(metricView)
+ val commentClause = comment.map(c => s"\nCOMMENT '$c'").getOrElse("")
+ sql(
+ s"""CREATE VIEW $name
+ |WITH METRICS$commentClause
+ |LANGUAGE YAML
+ |AS
+ |$$$$
+ |$yaml
+ |$$$$""".stripMargin)
+ yaml
+ }
+
+ private def capturedViewInfo(): ViewInfo = {
+ val ident = Identifier.of(Array(testNamespace), metricViewName)
+ val info = MetricViewRecordingCatalog.capturedViews.get(ident)
+ assert(info != null,
+ s"Expected ViewInfo for $ident to be captured by the V2 catalog")
+ info
+ }
+
+ // ============================================================
+ // Section 1: CREATE-related tests
+ // ============================================================
+
+
+ test("V2 catalog receives METRIC_VIEW table type and view text via
ViewInfo") {
+ withTestCatalogTables {
+ val metricView = MetricView(
+ "0.1",
+ AssetSource(fullSourceTableName),
+ where = None,
+ select = metricViewColumns)
+ val yaml = createMetricView(fullMetricViewName, metricView)
+
+ val info = capturedViewInfo()
+ // PROP_TABLE_TYPE is overwritten to METRIC_VIEW after `ViewInfo`'s
constructor stamps it
+ // to VIEW; this is the marker `V1Table.toCatalogTable` reads to map the
round-tripped row
+ // back to `CatalogTableType.METRIC_VIEW`.
+ assert(info.properties().get(TableCatalog.PROP_TABLE_TYPE)
+ === TableSummary.METRIC_VIEW_TABLE_TYPE)
+ // The captured queryText is the raw text between `$$ ... $$` --
including the leading
+ // and trailing newline our SQL fixture inserts -- so trim before
comparing to the
+ // pre-substitution YAML body.
+ assert(info.queryText().trim === yaml.trim)
+
+ val deps = info.viewDependencies()
+ assert(deps != null)
+ assert(deps.dependencies().length === 1)
+ val tableDep = deps.dependencies()(0).asInstanceOf[TableDependency]
+ assert(tableDep.nameParts().toSeq ===
+ Seq(testCatalogName, testNamespace, sourceTableName))
+ }
+ }
+
+ test("V2 catalog path populates metric_view.* + view context + sql configs
on ViewInfo") {
+ withTestCatalogTables {
+ val metricView = MetricView(
+ "0.1",
+ AssetSource(fullSourceTableName),
+ where = Some("count > 0"),
+ select = metricViewColumns)
+ createMetricView(fullMetricViewName, metricView)
+
+ val info = capturedViewInfo()
+ val props = info.properties()
+
+ // metric_view.* descriptive properties (mirrors DBR
SingleSourceMetricView).
+ assert(props.get(MetricView.PROP_FROM_TYPE) === "ASSET")
+ assert(props.get(MetricView.PROP_FROM_NAME) === fullSourceTableName)
+ assert(props.get(MetricView.PROP_FROM_SQL) === null)
+ assert(props.get(MetricView.PROP_WHERE) === "count > 0")
+
+ // SQL configs and current catalog/namespace are first-class typed
fields on ViewInfo, no
+ // longer encoded into properties for V2 catalogs.
+ assert(info.sqlConfigs().size > 0,
+ s"Expected at least one captured SQL config; got ${info.sqlConfigs()}")
+ assert(info.currentCatalog() ===
+ spark.sessionState.catalogManager.currentCatalog.name())
+ assert(info.currentNamespace().toSeq ===
+ spark.sessionState.catalogManager.currentNamespace.toSeq)
+ }
+ }
+
+ test("V2 catalog path captures SQL source and comment") {
+ withTestCatalogTables {
+ val metricView = MetricView(
+ "0.1",
+ SQLSource(s"SELECT * FROM $fullSourceTableName"),
+ where = None,
+ select = metricViewColumns)
+ createMetricView(fullMetricViewName, metricView, comment = Some("my mv"))
+
+ val info = capturedViewInfo()
+ val props = info.properties()
+ assert(props.get(TableCatalog.PROP_TABLE_TYPE)
+ === TableSummary.METRIC_VIEW_TABLE_TYPE)
+ assert(props.get(MetricView.PROP_FROM_TYPE) === "SQL")
+ assert(props.get(MetricView.PROP_FROM_NAME) === null)
+ assert(props.get(MetricView.PROP_FROM_SQL) ===
+ s"SELECT * FROM $fullSourceTableName")
+ assert(props.get(TableCatalog.PROP_COMMENT) === "my mv")
+
+ val deps = info.viewDependencies()
+ assert(deps != null && deps.dependencies().length === 1)
+ val tableDep = deps.dependencies()(0).asInstanceOf[TableDependency]
+ assert(tableDep.nameParts().toSeq ===
+ Seq(testCatalogName, testNamespace, sourceTableName))
+ }
+ }
+
+ test("metric view columns carry metric_view.type / metric_view.expr in
column metadata") {
+ withTestCatalogTables {
+ val metricView = MetricView(
+ "0.1",
+ AssetSource(fullSourceTableName),
+ where = None,
+ select = metricViewColumns)
+ createMetricView(fullMetricViewName, metricView)
+
+ val cols = capturedViewInfo().columns()
+ assert(cols.length === metricViewColumns.length)
+
+ val byName = cols.map(c => c.name() -> c).toMap
+ def metadataOf(name: String): Metadata =
+
Metadata.fromJson(Option(byName(name).metadataInJSON()).getOrElse("{}"))
+
+ val regionMeta = metadataOf("region")
+ assert(regionMeta.getString(Constants.COLUMN_TYPE_PROPERTY_KEY) ===
"dimension")
+ assert(regionMeta.getString(Constants.COLUMN_EXPR_PROPERTY_KEY) ===
"region")
+
+ val countMeta = metadataOf("count_sum")
+ assert(countMeta.getString(Constants.COLUMN_TYPE_PROPERTY_KEY) ===
"measure")
+ assert(countMeta.getString(Constants.COLUMN_EXPR_PROPERTY_KEY) ===
"sum(count)")
+ }
+ }
+
+ test("user-specified column names with comments preserve metric_view.*
metadata") {
+ withTestCatalogTables {
+ val metricView = MetricView(
+ "0.1",
+ AssetSource(fullSourceTableName),
+ where = None,
+ select = metricViewColumns)
+ val yaml = MetricViewFactory.toYAML(metricView)
+ // Give both columns new names, and a comment on each. Without the
`retainMetadata`
Review Comment:
Two test comments describe behavior in terms of the in-PR change rather than
the current invariant — they age poorly post-merge:
- Lines 253-254: `// ... Without the `retainMetadata` fix to
`ViewHelper.aliasPlan`, the metric_view.* keys disappear here.` — once merged,
there is no "fix" to refer to. Rephrase as a pin: `// Pins
aliasPlan(retainMetadata=true): metric_view.* keys must survive a column rename
with comments.`
- Lines 749-752 (DESCRIBE EXTENDED test): `// The "Type" row was added
alongside this metric-view PR so DescribeV2ViewExec matches v1 parity...` —
same problem. Rephrase to describe the current invariant being pinned (e.g.
`DescribeV2ViewExec emits a "Type" row matching v1 parity, so users can
distinguish VIEW vs METRIC_VIEW.`).
--
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]