szehon-ho commented on code in PR #59036:
URL: https://github.com/apache/spark/pull/59036#discussion_r4150556174
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/InMemoryBaseTable.scala:
##########
@@ -85,25 +87,25 @@ abstract class InMemoryBaseTable(
tableColumns = newColumns
}
+ override def displayProperties(): util.Map[String, String] = {
+ Option(tableDisplayProperties).getOrElse(super.displayProperties())
+ }
+
+ def setDisplayProperties(displayProperties: util.Map[String, String]): Unit
= {
+ tableDisplayProperties = new util.HashMap[String,
String](displayProperties)
+ }
+
override def version(): String = tableVersion.toString
def setVersion(version: String): Unit = {
tableVersion = version.toInt
}
/**
- * Copies version and validated version from another table.
- *
- * Some test catalogs (e.g. [[NullColumnIdInMemoryTableCatalog]],
- * [[NullTableIdAndNullColumnIdInMemoryTableCatalog]]) create a new table
object
- * that overrides specific behavior (such as nulling out column IDs). The new
- * object's version counter starts at 0. Without this call, the version
counter
- * resets every time the catalog creates such a replacement table, breaking
the
- * monotonic-version assumption that downstream consumers rely on (e.g.
- * [[InMemoryTable]].copy, validated-version propagation, and the
join-refresh
- * tests in [[DSv2IncrementallyConstructedQueryTests]]).
+ * Copies version, validated version and display-only metadata into a
replacement table.
Review Comment:
Restored the paragraph explaining why the version must be copied, and
changed the summary to cover version, validated version, and display-only
metadata copied from another table.
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/InMemoryTableCatalog.scala:
##########
@@ -276,6 +279,7 @@ class BasicInMemoryTableCatalog extends TableCatalog {
s"Unsupported InMemoryBaseTable subclass: ${other.getClass.getName}")
}
newTable.setVersion(currentVersion)
+ newTable.setDisplayProperties(table.displayProperties())
Review Comment:
Changed this path to call `copyTableStateFrom` and kept the AddConstraint
loop. The preservation test also checks version copying and that
validatedVersion survives a property alteration.
##########
python/pyspark/sql/catalog.py:
##########
@@ -383,6 +383,12 @@ def getTableProperties(self, tableName: str) -> Dict[str,
str]:
.. versionadded:: 4.2.0
+ Notes
Review Comment:
Moved Notes below Returns and before Examples.
##########
sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala:
##########
@@ -1201,6 +1201,32 @@ class CatalogSuite extends SharedSparkSession with
AnalysisTest with BeforeAndAf
spark.catalog.dropTable(t)
}
+ test("catalog API: getTableProperties includes V2 display-only properties") {
+ val catalogName = "display_catalog"
Review Comment:
Switched the test to the existing `testcat` and removed the `withSQLConf`
wrapper.
--
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]
