[SPARK-52729][SQL][FOLLOWUP] Introduce RelationInfo as a common base for TableInfo and ViewInfo

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<String, String> 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

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.
+ * <p>
+ * 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<String, String> 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<String, String> 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<B extends BaseBuilder<B>> {
+    protected Column[] columns = new Column[0];
+    protected Map<String, String> 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 <i>after</i> this method, not before.
+     */
+    public B withProperties(Map<String, String> 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();
+  }
+}

sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/TableInfo.java

@@ -16,139 +16,69 @@
  */
 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<String, String> 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<String, String> properties() {
-    return properties;
-  }
-
   public Transform[] partitions() {
     return partitions;
   }
 
   public Constraint[] constraints() { return constraints; }
 
   public static class Builder extends BaseBuilder<Builder> {
-    @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<B extends BaseBuilder<B>> {
-    protected Column[] columns = new Column[0];
-    protected Map<String, String> 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 <i>after</i> this method, not before.
-     */
-    public B withProperties(Map<String, String> 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();
   }
 }

sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/TableViewCatalog.java

@@ -125,7 +125,7 @@ public interface TableViewCatalog extends TableCatalog, ViewCatalog {
    * <p>
    * 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;
     }