diff --git a/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/MetadataTable.java b/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/MetadataTable.java index 1d1acfde80f9d..0e256efa4754b 100644 --- a/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/MetadataTable.java +++ b/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/MetadataTable.java @@ -39,17 +39,18 @@ * from {@link TableViewCatalog#loadTableOrView(Identifier)} as the single-RPC perf opt-in * for a view. * Downstream consumers distinguish the two by checking - * {@code getTableInfo() instanceof ViewInfo}. + * {@code getRelationInfo() instanceof ViewInfo}. * * @since 4.2.0 */ @Evolving public class MetadataTable implements Table { - private final TableInfo info; + private final RelationInfo info; private final String name; /** - * @param info metadata for the table or view. Pass a {@link ViewInfo} for a view. + * @param info metadata for the table or view: a {@link TableInfo} for a data-source table or a + * {@link ViewInfo} for a view. * @param name human-readable name for this table, returned by {@link #name()} and surfaced * in places that read it (e.g. {@code BatchScan} plan-tree labels and * partition-management error messages). {@code DESCRIBE TABLE EXTENDED} does @@ -60,12 +61,12 @@ public class MetadataTable implements Table { * {@code ident.toString()}, matching the quoted multi-part form used elsewhere * for v2 identifiers. */ - public MetadataTable(TableInfo info, String name) { + public MetadataTable(RelationInfo info, String name) { this.info = Objects.requireNonNull(info, "info should not be null"); this.name = Objects.requireNonNull(name, "name should not be null"); } - public TableInfo getTableInfo() { + public RelationInfo getRelationInfo() { return info; } @@ -81,12 +82,14 @@ public Map properties() { @Override public Transform[] partitioning() { - return info.partitions(); + // Partitioning is a table-only concept; a view wrapped as a MetadataTable has none. + return info instanceof TableInfo tableInfo ? tableInfo.partitions() : new Transform[0]; } @Override public Constraint[] constraints() { - return info.constraints(); + // Constraints are a table-only concept; a view wrapped as a MetadataTable has none. + return info instanceof TableInfo tableInfo ? tableInfo.constraints() : new Constraint[0]; } @Override diff --git a/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/RelationInfo.java b/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/RelationInfo.java new file mode 100644 index 0000000000000..8834657a7942c --- /dev/null +++ b/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/RelationInfo.java @@ -0,0 +1,120 @@ +/* + * 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.HashMap; +import java.util.Map; + +import org.apache.spark.annotation.Evolving; +import org.apache.spark.sql.types.StructType; + +/** + * Common metadata shared by {@link TableInfo} (data-source tables) and {@link ViewInfo} (views): + * the columns and the string properties bag. Tables and views are modeled as sibling subclasses + * rather than one extending the other, so that table-only concepts (partitioning, constraints, + * provider, location) never leak onto the view builder and vice versa. + *

+ * A {@code MetadataTable} wraps a {@code RelationInfo} so a single carrier can transport either + * kind through {@link TableViewCatalog#loadTableOrView}; downstream consumers discriminate via + * {@code getRelationInfo() instanceof ViewInfo}. + * + * @since 4.2.0 + */ +@Evolving +public abstract class RelationInfo { + + private final Column[] columns; + private final Map properties; + + protected RelationInfo(BaseBuilder builder) { + this.columns = builder.columns; + this.properties = builder.properties; + } + + public Column[] columns() { + return columns; + } + + public StructType schema() { + return CatalogV2Util.v2ColumnsToStructType(columns); + } + + public Map properties() { + return properties; + } + + /** + * Shared builder state for {@link TableInfo} and {@link ViewInfo}. Holds only the fields common + * to both -- columns and properties -- plus the convenience setters for reserved property keys + * that apply to both tables and views. Setters return {@code B} so subclass builders chain + * through their own type without a covariant override on each inherited setter. Table-only + * setters live on {@link TableInfo.Builder}. + */ + protected abstract static class BaseBuilder> { + protected Column[] columns = new Column[0]; + protected Map properties = new HashMap<>(); + + protected abstract B self(); + + public B withColumns(Column[] columns) { + this.columns = columns; + return self(); + } + + public B withSchema(StructType schema) { + this.columns = CatalogV2Util.structTypeToV2Columns(schema); + return self(); + } + + /** + * Replaces the current properties map with a defensive copy of the given map. Any reserved + * keys set earlier via convenience setters (e.g. {@link #withComment}) are discarded -- + * call those setters after this method, not before. + */ + public B withProperties(Map properties) { + this.properties = new HashMap<>(properties); + return self(); + } + + // Convenience setters below write reserved keys into the current `properties` map. Pair + // each with a preceding `withProperties(...)` call if you want to start from a user map; + // calling `withProperties` after a convenience setter discards the value the convenience + // setter wrote. + + public B withComment(String comment) { + properties.put(TableCatalog.PROP_COMMENT, comment); + return self(); + } + + public B withCollation(String collation) { + properties.put(TableCatalog.PROP_COLLATION, collation); + return self(); + } + + public B withOwner(String owner) { + properties.put(TableCatalog.PROP_OWNER, owner); + return self(); + } + + public B withTableType(String tableType) { + properties.put(TableCatalog.PROP_TABLE_TYPE, tableType); + return self(); + } + + public abstract RelationInfo build(); + } +} diff --git a/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/TableInfo.java b/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/TableInfo.java index 89709c9f1c2f0..2a92c07ff1a4c 100644 --- a/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/TableInfo.java +++ b/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/TableInfo.java @@ -16,43 +16,31 @@ */ package org.apache.spark.sql.connector.catalog; -import java.util.HashMap; -import java.util.Map; import java.util.Objects; import org.apache.spark.sql.connector.catalog.constraints.Constraint; import org.apache.spark.sql.connector.expressions.Transform; -import org.apache.spark.sql.types.StructType; -public class TableInfo { +/** + * Metadata for a data-source table. Adds the table-only fields -- partitioning and constraints -- + * on top of the columns and properties carried by {@link RelationInfo}. Views are represented by + * the sibling {@link ViewInfo}; the two share {@link RelationInfo} rather than one extending the + * other. + */ +public class TableInfo extends RelationInfo { - private final Column[] columns; - private final Map properties; private final Transform[] partitions; private final Constraint[] constraints; /** * Constructor for TableInfo used by the builder. */ - protected TableInfo(BaseBuilder builder) { - this.columns = builder.columns; - this.properties = builder.properties; + protected TableInfo(Builder builder) { + super(builder); this.partitions = builder.partitions; this.constraints = builder.constraints; } - public Column[] columns() { - return columns; - } - - public StructType schema() { - return CatalogV2Util.v2ColumnsToStructType(columns); - } - - public Map properties() { - return properties; - } - public Transform[] partitions() { return partitions; } @@ -60,95 +48,37 @@ public Transform[] partitions() { public Constraint[] constraints() { return constraints; } public static class Builder extends BaseBuilder { - @Override - protected Builder self() { return this; } - - @Override - public TableInfo build() { - Objects.requireNonNull(columns, "columns should not be null"); - return new TableInfo(this); - } - } - - /** - * Shared builder state for {@link TableInfo} and its subclasses. Setters return {@code B} so - * subclass builders (e.g. {@link ViewInfo.Builder}) chain through their own type without - * a covariant override on each inherited setter. - */ - protected abstract static class BaseBuilder> { - protected Column[] columns = new Column[0]; - protected Map properties = new HashMap<>(); protected Transform[] partitions = new Transform[0]; protected Constraint[] constraints = new Constraint[0]; - protected abstract B self(); - - public B withColumns(Column[] columns) { - this.columns = columns; - return self(); - } - - public B withSchema(StructType schema) { - this.columns = CatalogV2Util.structTypeToV2Columns(schema); - return self(); - } - - /** - * Replaces the current properties map with a defensive copy of the given map. Any reserved - * keys set earlier via convenience setters (e.g. {@link #withProvider}) are discarded -- - * call those setters after this method, not before. - */ - public B withProperties(Map properties) { - this.properties = new HashMap<>(properties); - return self(); - } + @Override + protected Builder self() { return this; } - public B withPartitions(Transform[] partitions) { + public Builder withPartitions(Transform[] partitions) { this.partitions = partitions; - return self(); + return this; } - public B withConstraints(Constraint[] constraints) { + public Builder withConstraints(Constraint[] constraints) { this.constraints = constraints; - return self(); + return this; } - // Convenience setters below write reserved keys into the current `properties` map. Pair - // each with a preceding `withProperties(...)` call if you want to start from a user map; - // calling `withProperties` after a convenience setter discards the value the convenience - // setter wrote. - /** Writes {@link TableCatalog#PROP_PROVIDER} into the current properties map. */ - public B withProvider(String provider) { + public Builder withProvider(String provider) { properties.put(TableCatalog.PROP_PROVIDER, provider); - return self(); + return this; } - public B withLocation(String location) { + public Builder withLocation(String location) { properties.put(TableCatalog.PROP_LOCATION, location); - return self(); - } - - public B withComment(String comment) { - properties.put(TableCatalog.PROP_COMMENT, comment); - return self(); + return this; } - public B withCollation(String collation) { - properties.put(TableCatalog.PROP_COLLATION, collation); - return self(); - } - - public B withOwner(String owner) { - properties.put(TableCatalog.PROP_OWNER, owner); - return self(); - } - - public B withTableType(String tableType) { - properties.put(TableCatalog.PROP_TABLE_TYPE, tableType); - return self(); + @Override + public TableInfo build() { + Objects.requireNonNull(columns, "columns should not be null"); + return new TableInfo(this); } - - public abstract TableInfo build(); } } diff --git a/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/TableViewCatalog.java b/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/TableViewCatalog.java index 45ec41d680d8b..791f769a5d271 100644 --- a/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/TableViewCatalog.java +++ b/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/TableViewCatalog.java @@ -125,7 +125,7 @@ public interface TableViewCatalog extends TableCatalog, ViewCatalog { *

* For a table, returns the table's {@link Table}. For a view, returns a * {@link MetadataTable} wrapping a {@link ViewInfo}; callers discriminate via - * {@code getTableInfo() instanceof ViewInfo}. This lets the resolver answer in a single RPC + * {@code getRelationInfo() instanceof ViewInfo}. This lets the resolver answer in a single RPC * instead of falling back from {@link TableCatalog#loadTable} to {@link ViewCatalog#loadView}. * * @param ident the identifier @@ -174,7 +174,7 @@ default TableSummary[] listTableAndViewSummaries(String[] namespace) @Override default Table loadTable(Identifier ident) throws NoSuchTableException { Table t = loadTableOrView(ident); - if (t instanceof MetadataTable mot && mot.getTableInfo() instanceof ViewInfo) { + if (t instanceof MetadataTable mot && mot.getRelationInfo() instanceof ViewInfo) { throw new NoSuchTableException(ident); } return t; @@ -196,7 +196,7 @@ default ViewInfo loadView(Identifier ident) throws NoSuchViewException { } catch (NoSuchTableException e) { throw new NoSuchViewException(ident); } - if (t instanceof MetadataTable mot && mot.getTableInfo() instanceof ViewInfo vi) { + if (t instanceof MetadataTable mot && mot.getRelationInfo() instanceof ViewInfo vi) { return vi; } throw new NoSuchViewException(ident); @@ -212,7 +212,7 @@ default ViewInfo loadView(Identifier ident) throws NoSuchViewException { default boolean tableExists(Identifier ident) { try { Table t = loadTableOrView(ident); - return !(t instanceof MetadataTable mot && mot.getTableInfo() instanceof ViewInfo); + return !(t instanceof MetadataTable mot && mot.getRelationInfo() instanceof ViewInfo); } catch (NoSuchTableException e) { return false; } @@ -228,7 +228,7 @@ default boolean tableExists(Identifier ident) { default boolean viewExists(Identifier ident) { try { Table t = loadTableOrView(ident); - return t instanceof MetadataTable mot && mot.getTableInfo() instanceof ViewInfo; + return t instanceof MetadataTable mot && mot.getRelationInfo() instanceof ViewInfo; } catch (NoSuchTableException e) { return false; } diff --git a/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/ViewInfo.java b/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/ViewInfo.java index 0f46e915a9be2..2f6eeac1d8ffc 100644 --- a/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/ViewInfo.java +++ b/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/ViewInfo.java @@ -28,19 +28,21 @@ * by {@link ViewCatalog#createView} / {@link ViewCatalog#replaceView}. Carries the * view-specific fields that cannot be represented as string table properties: the query text, * captured creation-time resolution context, captured SQL configs, schema-binding mode, and - * query output column names. Schema and user TBLPROPERTIES are inherited from {@link TableInfo} + * query output column names. Schema and user TBLPROPERTIES are inherited from {@link RelationInfo} * via the typed builder. *

- * {@code ViewInfo} extends {@link TableInfo} so that a {@link TableViewCatalog} can opt into the - * single-RPC perf path by returning a {@link MetadataTable} wrapping a {@code ViewInfo} - * from {@link TableViewCatalog#loadTableOrView} for a view identifier. Pure {@link ViewCatalog} - * implementations never see {@code TableInfo}; the typed setters on {@link Builder} cover - * everything they need to construct a {@code ViewInfo}. + * {@code ViewInfo} and {@link TableInfo} are sibling subclasses of {@link RelationInfo}: a view is + * not a table, so it does not inherit the table-only surface (partitioning, constraints, provider, + * location). A {@link TableViewCatalog} can still opt into the single-RPC perf path by returning a + * {@link MetadataTable} wrapping a {@code ViewInfo} from {@link TableViewCatalog#loadTableOrView} + * for a view identifier -- {@code MetadataTable} carries a {@link RelationInfo}, which a + * {@code ViewInfo} is. The typed setters on {@link Builder} cover everything a pure + * {@link ViewCatalog} implementation needs to construct a {@code ViewInfo}. * * @since 4.2.0 */ @Evolving -public class ViewInfo extends TableInfo { +public class ViewInfo extends RelationInfo { private final String queryText; private final String currentCatalog; diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala index 7a5048ed0a41d..f4a70b7123111 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala @@ -1209,9 +1209,9 @@ class Analyzer( // neither exists. try { Some(mc.loadTableOrView(ident) match { - case t: MetadataTable if t.getTableInfo.isInstanceOf[ViewInfo] => + case t: MetadataTable if t.getRelationInfo.isInstanceOf[ViewInfo] => ResolvedPersistentView( - catalog, ident, t.getTableInfo.asInstanceOf[ViewInfo]) + catalog, ident, t.getRelationInfo.asInstanceOf[ViewInfo]) case table => ResolvedTable.create(catalog.asTableCatalog, ident, table) }) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/RelationResolution.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/RelationResolution.scala index 55a7ad10790ea..57c3b389bf106 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/RelationResolution.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/RelationResolution.scala @@ -294,7 +294,7 @@ class RelationResolution( // `table` is `tableOrView` filtered to tables only -- used for cache lookup since // we don't share-cache views. val table: Option[Table] = tableOrView.filter { - case t: MetadataTable if t.getTableInfo.isInstanceOf[ViewInfo] => false + case t: MetadataTable if t.getRelationInfo.isInstanceOf[ViewInfo] => false case _ => true } diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/V1Table.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/V1Table.scala index 8a47cac8e7962..37d3002b7ed98 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/V1Table.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/V1Table.scala @@ -113,9 +113,9 @@ private[sql] object V1Table { def toCatalogTable( catalog: CatalogPlugin, ident: Identifier, - t: MetadataTable): CatalogTable = t.getTableInfo match { + t: MetadataTable): CatalogTable = t.getRelationInfo match { case viewInfo: ViewInfo => toCatalogTable(catalog, ident, viewInfo) - case tableInfo => toCatalogTable(catalog, ident, tableInfo) + case tableInfo: TableInfo => toCatalogTable(catalog, ident, tableInfo) } private def toCatalogTable( diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/InMemoryTableViewCatalog.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/InMemoryTableViewCatalog.scala index a3506938dea7c..5ff70f50e7bc6 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/InMemoryTableViewCatalog.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/InMemoryTableViewCatalog.scala @@ -37,7 +37,7 @@ import org.apache.spark.sql.util.CaseInsensitiveStringMap class InMemoryTableViewCatalog extends TableViewCatalog with SupportsNamespaces { private val store = - new ConcurrentHashMap[(Seq[String], String), TableInfo]() + new ConcurrentHashMap[(Seq[String], String), RelationInfo]() private val namespaces = new ConcurrentHashMap[Seq[String], util.Map[String, String]]() @@ -219,7 +219,7 @@ class InMemoryTableViewCatalog extends TableViewCatalog with SupportsNamespaces // Test-only accessors -------------------------------------------------------------- /** Returns the stored entry (table or view) for the identifier, or throws if missing. */ - def getStoredInfo(namespace: Array[String], name: String): TableInfo = { + def getStoredInfo(namespace: Array[String], name: String): RelationInfo = { Option(store.get((namespace.toSeq, name))).getOrElse { throw new NoSuchTableException(Identifier.of(namespace, name)) } diff --git a/sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2MetadataViewSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2MetadataViewSuite.scala index 969ebb540ae6a..7808c123e2b5c 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2MetadataViewSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2MetadataViewSuite.scala @@ -20,7 +20,7 @@ package org.apache.spark.sql.connector import org.apache.spark.SparkConf import org.apache.spark.sql.{AnalysisException, Row} import org.apache.spark.sql.catalyst.analysis.{NoSuchTableException, NoSuchViewException, TableAlreadyExistsException, ViewAlreadyExistsException} -import org.apache.spark.sql.connector.catalog.{Identifier, MetadataTable, Table, TableCatalog, TableChange, TableInfo, TableSummary, TableViewCatalog, V1Table, ViewCatalog, ViewInfo} +import org.apache.spark.sql.connector.catalog.{Identifier, MetadataTable, RelationInfo, Table, TableCatalog, TableChange, TableInfo, TableSummary, TableViewCatalog, V1Table, ViewCatalog, ViewInfo} import org.apache.spark.sql.internal.SQLConf import org.apache.spark.sql.test.SharedSparkSession import org.apache.spark.sql.types.StructType @@ -409,7 +409,7 @@ class TestingTableViewCatalog extends TableViewCatalog { // distinguishes views from tables. Mixed-catalog: shared identifier namespace per the // TableViewCatalog contract. private val createdViews = - new java.util.concurrent.ConcurrentHashMap[(Seq[String], String), TableInfo]() + new java.util.concurrent.ConcurrentHashMap[(Seq[String], String), RelationInfo]() // Canned read-only view fixtures, exposed only via the perf path (loadTableOrView). loadView // does not need to expose them because the resolver routes TableViewCatalog reads through @@ -473,8 +473,8 @@ class TestingTableViewCatalog extends TableViewCatalog { new MetadataTable(info, ident.toString) } - /** Test-only accessor: returns the stored TableInfo (table or view) for the identifier. */ - def getStoredInfo(namespace: Array[String], name: String): TableInfo = { + /** Test-only accessor: returns the stored RelationInfo (table or view) for the identifier. */ + def getStoredInfo(namespace: Array[String], name: String): RelationInfo = { Option(createdViews.get((namespace.toSeq, name))).getOrElse { throw new NoSuchTableException(Identifier.of(namespace, name)) }