szehon-ho commented on code in PR #59036:
URL: https://github.com/apache/spark/pull/59036#discussion_r4331099278
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/Table.java:
##########
@@ -95,6 +95,34 @@ default Map<String, String> properties() {
return Collections.emptyMap();
}
+ /**
+ * Returns additional, display-only table properties.
+ *
+ * <p>Spark adds these entries to the non-reserved properties displayed by
V2 table metadata
+ * commands and {@code Catalog.getTableProperties}. Entries in {@link
#properties()} take
+ * precedence, and reserved keys such as {@link TableCatalog#PROP_EXTERNAL}
and
+ * {@link TableCatalog#PROP_LOCATION} are ignored in this map. Table type
and other reserved
+ * metadata are read from {@code properties()}.
+ * After a stored property is removed, Spark displays the entry for that key
if it remains in
Review Comment:
Scoped the show-through sentence to stored non-reserved properties and named
`PROP_COMMENT` alongside the ignored reserved keys.
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/Table.java:
##########
@@ -95,6 +95,34 @@ default Map<String, String> properties() {
return Collections.emptyMap();
}
+ /**
+ * Returns additional, display-only table properties.
+ *
+ * <p>Spark adds these entries to the non-reserved properties displayed by
V2 table metadata
+ * commands and {@code Catalog.getTableProperties}. Entries in {@link
#properties()} take
+ * precedence, and reserved keys such as {@link TableCatalog#PROP_EXTERNAL}
and
+ * {@link TableCatalog#PROP_LOCATION} are ignored in this map. Table type
and other reserved
+ * metadata are read from {@code properties()}.
+ * After a stored property is removed, Spark displays the entry for that key
if it remains in
+ * this map.
+ *
+ * <p>Spark passes {@code ALTER TABLE ... SET/UNSET TBLPROPERTIES} to
+ * {@link TableCatalog#alterTable} without consulting this map. Catalogs
should apply these
+ * changes to {@code properties()} and keep display-only entries unchanged.
+ *
+ * <p>For now, {@code SHOW TABLE EXTENDED} omits these entries for the
built-in session catalog
+ * and its {@link CatalogExtension}s.
+ *
+ * <p>These entries may be transient and are excluded from {@code SHOW
CREATE TABLE}. The
Review Comment:
Added guidance that `TableCatalog.createTableLike` implementations should
not copy display-only entries from the source table, with a link to the full
method signature.
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/Table.java:
##########
@@ -95,6 +95,34 @@ default Map<String, String> properties() {
return Collections.emptyMap();
}
+ /**
+ * Returns additional, display-only table properties.
+ *
+ * <p>Spark adds these entries to the non-reserved properties displayed by
V2 table metadata
+ * commands and {@code Catalog.getTableProperties}. Entries in {@link
#properties()} take
+ * precedence, and reserved keys such as {@link TableCatalog#PROP_EXTERNAL}
and
+ * {@link TableCatalog#PROP_LOCATION} are ignored in this map. Table type
and other reserved
+ * metadata are read from {@code properties()}.
+ * After a stored property is removed, Spark displays the entry for that key
if it remains in
+ * this map.
+ *
+ * <p>Spark passes {@code ALTER TABLE ... SET/UNSET TBLPROPERTIES} to
+ * {@link TableCatalog#alterTable} without consulting this map. Catalogs
should apply these
Review Comment:
Added notes to both the `SHOW TBLPROPERTIES` reference and the `ALTER TABLE
... UNSET TBLPROPERTIES` section. They explain that UNSET leaves display-only
entries intact and can expose a display value after removing a colliding stored
non-reserved property. The SHOW page also notes their exclusion from `SHOW
CREATE TABLE`.
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/TableInfo.java:
##########
@@ -59,19 +62,46 @@ public Map<String, String> properties() {
return properties;
}
+ /**
+ * Returns the additional display-only properties described by {@link
Table#displayProperties()}.
+ *
+ * <p>These properties describe metadata returned by a catalog. Spark leaves
them empty when
+ * calling {@code TableCatalog.createTable},
+ * {@code TableCatalog.createTableLike}, {@code
StagingTableCatalog.stageCreate},
+ * {@code StagingTableCatalog.stageReplace}, and {@code
StagingTableCatalog.stageCreateOrReplace}.
+ *
+ * @since 4.4.0
+ */
+ public Map<String, String> displayProperties() {
+ return displayProperties;
+ }
+
public Transform[] partitions() {
return partitions;
}
public Constraint[] constraints() { return constraints; }
public static class Builder extends RelationBuilder<Builder> {
+ protected Map<String, String> displayProperties = new HashMap<>();
protected Transform[] partitions = new Transform[0];
protected Constraint[] constraints = new Constraint[0];
@Override
protected Builder self() { return this; }
+ /**
+ * Sets additional display-only properties.
+ *
+ * @param displayProperties a non-null map with non-null keys and values
+ * @see Table#displayProperties()
+ * @since 4.4.0
+ */
+ public Builder withDisplayProperties(Map<String, String>
displayProperties) {
Review Comment:
Added to `Table.displayProperties()` that Spark reads these entries only to
display table metadata, so implementations may compute them on demand.
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/Table.java:
##########
@@ -95,6 +95,34 @@ default Map<String, String> properties() {
return Collections.emptyMap();
}
+ /**
+ * Returns additional, display-only table properties.
+ *
+ * <p>Spark adds these entries to the non-reserved properties displayed by
V2 table metadata
Review Comment:
Added the sentence that Spark redacts these entries in the same way as
stored properties.
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/TableInfo.java:
##########
@@ -59,19 +62,46 @@ public Map<String, String> properties() {
return properties;
}
+ /**
+ * Returns the additional display-only properties described by {@link
Table#displayProperties()}.
+ *
+ * <p>These properties describe metadata returned by a catalog. Spark leaves
them empty when
+ * calling {@code TableCatalog.createTable},
+ * {@code TableCatalog.createTableLike}, {@code
StagingTableCatalog.stageCreate},
+ * {@code StagingTableCatalog.stageReplace}, and {@code
StagingTableCatalog.stageCreateOrReplace}.
+ *
+ * @since 4.4.0
+ */
+ public Map<String, String> displayProperties() {
+ return displayProperties;
+ }
+
public Transform[] partitions() {
return partitions;
}
public Constraint[] constraints() { return constraints; }
public static class Builder extends RelationBuilder<Builder> {
+ protected Map<String, String> displayProperties = new HashMap<>();
protected Transform[] partitions = new Transform[0];
protected Constraint[] constraints = new Constraint[0];
@Override
protected Builder self() { return this; }
+ /**
+ * Sets additional display-only properties.
+ *
+ * @param displayProperties a non-null map with non-null keys and values
+ * @see Table#displayProperties()
Review Comment:
Changed the setter's cross-reference to `TableInfo.displayProperties()`,
replaced all five create/stage references with links using full signatures, and
included display-only properties in the class summary.
--
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]
