You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "abhiramaab (via GitHub)" <gi...@apache.org> on 2026/08/25 18:13:07 UTC
[PR] Fix ALTER TABLE Hudi property persistence [hudi]
abhiramaab opened a new pull request, #19733:
URL: https://github.com/apache/hudi/pull/19733
### Describe the issue this Pull Request addresses
Closes #19722
ALTER TABLE SET/UNSET TBLPROPERTIES for Hudi tables only updated the Spark catalog and did not persist the corresponding Hudi table properties in hoodie.properties.
### Summary and Changelog
Persist Hudi table properties changed through ALTER TABLE SET/UNSET TBLPROPERTIES into hoodie.properties.
Changes:
- Persist Hudi-related properties to hoodie.properties on ALTER TABLE SET TBLPROPERTIES.
- Remove Hudi-related properties from hoodie.properties on ALTER TABLE UNSET TBLPROPERTIES.
- Keep non-Hudi/Spark properties catalog-only.
- Reuse Hudi's existing option mapping and table-config validation.
### Impact
Low user-facing impact. ALTER TABLE SET/UNSET TBLPROPERTIES now correctly persists and removes Hudi table configurations in hoodie.properties. Non-Hudi Spark table properties continue to behave as catalog-only properties.
No public API changes and no expected performance impact.
### Risk Level
low
The change is limited to ALTER TABLE property handling and uses the existing Hudi table-config validation and persistence APIs. TestAlterTable was executed with 33/33 tests passing, and the Maven build completed successfully.
### Documentation Update
none
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: ALTER TABLE Hudi property persistence [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #19733:
URL: https://github.com/apache/hudi/pull/19733#discussion_r3859309219
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/command/AlterTableCommand.scala:
##########
@@ -208,10 +210,57 @@ case class AlterTableCommand(table: CatalogTable, changes: Seq[TableChange], cha
val newTable = table.copy(
properties = table.properties ++ properties,
comment = properties.get(TableCatalog.PROP_COMMENT).orElse(table.comment))
+ updateHoodieTableConfigs(sparkSession, properties)
catalog.alterTable(newTable)
logInfo("table properties change finished")
}
+ /**
+ * Persists Hudi table properties in hoodie.properties as well as the Spark catalog.
+ *
+ * The analyzer only creates this command for Hudi V2 tables, so the table is known to be a Hudi
+ * table. Keep non-Hudi properties catalog-only, matching Spark's normal ALTER TABLE behavior.
+ * Hudi's SQL aliases and datasource options are converted to the canonical keys stored in
+ * hoodie.properties before validation and persistence.
+ */
+ private def updateHoodieTableConfigs(sparkSession: SparkSession, properties: Map[String, String]): Unit = {
+ val tableConfigs = HoodieOptionConfig.mapSqlOptionsToTableConfigs(
+ HoodieOptionConfig.extractHoodieOptions(properties))
+
+ if (tableConfigs.nonEmpty) {
+ val metaClient = getMetaClient(sparkSession)
+
+ HoodieWriterUtils.validateTableConfig(
Review Comment:
**[P1] Validate invariants against the merged table config**
`validateTableConfig` mainly compares each incoming key with an existing value; it does not re-run the cross-property validation used at table creation. For example, a normal single-partition table can already have `hoodie.datasource.write.hive_style_partitioning=true` and no explicit slash-separated property. Setting `hoodie.datasource.write.slash.separated.date.partitioning=true` passes here because the new key has no existing value and the partition count is one, then persists both flags even though `HoodieCatalogTable.extraTableConfig` explicitly rejects that pair as mutually exclusive. Please validate the merged existing+incoming configuration, including cross-property and value constraints, before updating `hoodie.properties`.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: ALTER TABLE Hudi property persistence [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19733:
URL: https://github.com/apache/hudi/pull/19733#issuecomment-5434105550
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "1c1a8f991c98fe4509dba421779270dd50b4fa88",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16539",
"triggerID" : "1c1a8f991c98fe4509dba421779270dd50b4fa88",
"triggerType" : "PUSH"
}, {
"hash" : "64cec58d4b1f5ea19b719df4feb66fd25b04fb2d",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16598",
"triggerID" : "64cec58d4b1f5ea19b719df4feb66fd25b04fb2d",
"triggerType" : "PUSH"
}, {
"hash" : "2e0d5f74750eb7dc45f18cfe6384cd4f3cf7fb03",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16607",
"triggerID" : "2e0d5f74750eb7dc45f18cfe6384cd4f3cf7fb03",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 64cec58d4b1f5ea19b719df4feb66fd25b04fb2d Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16598)
* 2e0d5f74750eb7dc45f18cfe6384cd4f3cf7fb03 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16607)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: ALTER TABLE Hudi property persistence [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19733:
URL: https://github.com/apache/hudi/pull/19733#issuecomment-5434458007
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "1c1a8f991c98fe4509dba421779270dd50b4fa88",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16539",
"triggerID" : "1c1a8f991c98fe4509dba421779270dd50b4fa88",
"triggerType" : "PUSH"
}, {
"hash" : "64cec58d4b1f5ea19b719df4feb66fd25b04fb2d",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16598",
"triggerID" : "64cec58d4b1f5ea19b719df4feb66fd25b04fb2d",
"triggerType" : "PUSH"
}, {
"hash" : "2e0d5f74750eb7dc45f18cfe6384cd4f3cf7fb03",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16607",
"triggerID" : "2e0d5f74750eb7dc45f18cfe6384cd4f3cf7fb03",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2e0d5f74750eb7dc45f18cfe6384cd4f3cf7fb03 Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16607)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: ALTER TABLE Hudi property persistence [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19733:
URL: https://github.com/apache/hudi/pull/19733#issuecomment-5427421961
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "1c1a8f991c98fe4509dba421779270dd50b4fa88",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16539",
"triggerID" : "1c1a8f991c98fe4509dba421779270dd50b4fa88",
"triggerType" : "PUSH"
}, {
"hash" : "64cec58d4b1f5ea19b719df4feb66fd25b04fb2d",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "64cec58d4b1f5ea19b719df4feb66fd25b04fb2d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 1c1a8f991c98fe4509dba421779270dd50b4fa88 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16539)
* 64cec58d4b1f5ea19b719df4feb66fd25b04fb2d UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: ALTER TABLE Hudi property persistence [hudi]
Posted by "abhiramaab (via GitHub)" <gi...@apache.org>.
abhiramaab closed pull request #19733: fix: ALTER TABLE Hudi property persistence
URL: https://github.com/apache/hudi/pull/19733
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: ALTER TABLE Hudi property persistence [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19733:
URL: https://github.com/apache/hudi/pull/19733#issuecomment-5438219720
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "1c1a8f991c98fe4509dba421779270dd50b4fa88",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16539",
"triggerID" : "1c1a8f991c98fe4509dba421779270dd50b4fa88",
"triggerType" : "PUSH"
}, {
"hash" : "64cec58d4b1f5ea19b719df4feb66fd25b04fb2d",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16598",
"triggerID" : "64cec58d4b1f5ea19b719df4feb66fd25b04fb2d",
"triggerType" : "PUSH"
}, {
"hash" : "2e0d5f74750eb7dc45f18cfe6384cd4f3cf7fb03",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16607",
"triggerID" : "2e0d5f74750eb7dc45f18cfe6384cd4f3cf7fb03",
"triggerType" : "PUSH"
}, {
"hash" : "28c33a02fb4281bc0bd0c2c7cee4526570a1dc86",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16626",
"triggerID" : "28c33a02fb4281bc0bd0c2c7cee4526570a1dc86",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2e0d5f74750eb7dc45f18cfe6384cd4f3cf7fb03 Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16607)
* 28c33a02fb4281bc0bd0c2c7cee4526570a1dc86 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16626)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: ALTER TABLE Hudi property persistence [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #19733:
URL: https://github.com/apache/hudi/pull/19733#discussion_r3870039411
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/command/AlterTableCommand.scala:
##########
@@ -208,10 +211,98 @@ case class AlterTableCommand(table: CatalogTable, changes: Seq[TableChange], cha
val newTable = table.copy(
properties = table.properties ++ properties,
comment = properties.get(TableCatalog.PROP_COMMENT).orElse(table.comment))
+ updateHoodieTableConfigs(sparkSession, properties)
catalog.alterTable(newTable)
logInfo("table properties change finished")
}
+ /**
+ * Persists Hudi table properties in hoodie.properties as well as the Spark catalog.
+ *
+ * The analyzer only creates this command for Hudi V2 tables, so the table is known to be a Hudi
+ * table. Keep non-Hudi properties catalog-only, matching Spark's normal ALTER TABLE behavior.
+ * Hudi's SQL aliases and datasource options are converted to the canonical keys stored in
+ * hoodie.properties before validation and persistence.
+ */
+ private val protectedTableConfigs = Set(
+ HoodieTableConfig.NAME.key,
+ HoodieTableConfig.TYPE.key,
+ HoodieTableConfig.VERSION.key,
+ HoodieTableConfig.INITIAL_VERSION.key,
+ HoodieTableConfig.PRECOMBINE_FIELD.key,
+ HoodieTableConfig.LOG_FILE_FORMAT.key,
+ HoodieTableConfig.RECORDKEY_FIELDS.key,
+ HoodieTableConfig.PARTITION_FIELDS.key,
+ HoodieTableConfig.ORDERING_FIELDS.key,
+ HoodieTableConfig.KEY_GENERATOR_CLASS_NAME.key,
+ HoodieTableConfig.KEY_GENERATOR_TYPE.key,
+ HoodieTableConfig.RECORD_MERGE_MODE.key,
+ HoodieTableConfig.RECORD_MERGE_STRATEGY_ID.key,
+ HoodieTableConfig.PAYLOAD_CLASS_NAME.key,
+ HoodieTableConfig.LEGACY_PAYLOAD_CLASS_NAME.key,
+ HoodieTableConfig.TABLE_CHECKSUM.key,
+ HoodieTableConfig.CREATE_SCHEMA.key,
+ HoodieTableConfig.POPULATE_META_FIELDS.key,
+ HoodieTableConfig.META_FIELDS_MODE.key,
+ HoodieTableConfig.BOOTSTRAP_BASE_PATH.key,
+ HoodieTableConfig.DATABASE_NAME.key,
+ HoodieTableConfig.TIMELINE_LAYOUT_VERSION.key,
+ HoodieTableConfig.TABLE_STORAGE_LAYOUT.key,
+ HoodieTableConfig.TABLE_FORMAT.key
Review Comment:
**[P1] Keep metadata-partition state system-managed**
`HoodieTableConfig.TABLE_METADATA_PARTITIONS` and `TABLE_METADATA_PARTITIONS_INFLIGHT` are also absent from this set. Since arbitrary `hoodie.*` keys reach `HoodieTableConfig.update/delete`, SQL can mark an unbuilt metadata partition as completed or remove a real completed/inflight marker. Readers and index support trust `getMetadataPartitions` to decide which metadata indexes are available, while the metadata writer uses both sets to decide what to update, so altering these values can route reads to missing or stale indexes and corrupt subsequent metadata maintenance. These internal state keys must not be user-mutable through TBLPROPERTIES.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: ALTER TABLE Hudi property persistence [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #19733:
URL: https://github.com/apache/hudi/pull/19733#discussion_r3870038116
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/command/AlterTableCommand.scala:
##########
@@ -208,10 +211,98 @@ case class AlterTableCommand(table: CatalogTable, changes: Seq[TableChange], cha
val newTable = table.copy(
properties = table.properties ++ properties,
comment = properties.get(TableCatalog.PROP_COMMENT).orElse(table.comment))
+ updateHoodieTableConfigs(sparkSession, properties)
catalog.alterTable(newTable)
logInfo("table properties change finished")
}
+ /**
+ * Persists Hudi table properties in hoodie.properties as well as the Spark catalog.
+ *
+ * The analyzer only creates this command for Hudi V2 tables, so the table is known to be a Hudi
+ * table. Keep non-Hudi properties catalog-only, matching Spark's normal ALTER TABLE behavior.
+ * Hudi's SQL aliases and datasource options are converted to the canonical keys stored in
+ * hoodie.properties before validation and persistence.
+ */
+ private val protectedTableConfigs = Set(
+ HoodieTableConfig.NAME.key,
+ HoodieTableConfig.TYPE.key,
+ HoodieTableConfig.VERSION.key,
+ HoodieTableConfig.INITIAL_VERSION.key,
+ HoodieTableConfig.PRECOMBINE_FIELD.key,
+ HoodieTableConfig.LOG_FILE_FORMAT.key,
+ HoodieTableConfig.RECORDKEY_FIELDS.key,
+ HoodieTableConfig.PARTITION_FIELDS.key,
+ HoodieTableConfig.ORDERING_FIELDS.key,
+ HoodieTableConfig.KEY_GENERATOR_CLASS_NAME.key,
+ HoodieTableConfig.KEY_GENERATOR_TYPE.key,
+ HoodieTableConfig.RECORD_MERGE_MODE.key,
+ HoodieTableConfig.RECORD_MERGE_STRATEGY_ID.key,
+ HoodieTableConfig.PAYLOAD_CLASS_NAME.key,
+ HoodieTableConfig.LEGACY_PAYLOAD_CLASS_NAME.key,
+ HoodieTableConfig.TABLE_CHECKSUM.key,
+ HoodieTableConfig.CREATE_SCHEMA.key,
+ HoodieTableConfig.POPULATE_META_FIELDS.key,
+ HoodieTableConfig.META_FIELDS_MODE.key,
+ HoodieTableConfig.BOOTSTRAP_BASE_PATH.key,
+ HoodieTableConfig.DATABASE_NAME.key,
+ HoodieTableConfig.TIMELINE_LAYOUT_VERSION.key,
Review Comment:
**[P1] Protect the physical timeline paths**
This immutable-config set includes the timeline layout version but omits `HoodieTableConfig.TIMELINE_PATH` and `TIMELINE_HISTORY_PATH`. Both keys pass `extractHoodieOptions`, so setting `hoodie.timeline.path` to another directory rewrites `hoodie.properties`. Every newly constructed `HoodieTableMetaClient` derives its timeline location from that property, so it will point away from the existing instants and the table can appear to have no history; UNSET similarly breaks a table that legitimately uses a custom path by reverting it to the default. These structural paths need the same protection, or the implementation should use an explicit allowlist of mutable properties.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: ALTER TABLE Hudi property persistence [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #19733:
URL: https://github.com/apache/hudi/pull/19733#discussion_r3859310193
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/command/AlterTableCommand.scala:
##########
@@ -194,6 +195,7 @@ case class AlterTableCommand(table: CatalogTable, changes: Seq[TableChange], cha
val tableComment = if (propKeys.contains(TableCatalog.PROP_COMMENT)) None else table.comment
val newProperties = table.properties.filter { case (k, _) => !propKeys.contains(k) }
val newTable = table.copy(properties = newProperties, comment = tableComment)
+ deleteHoodieTableConfigs(sparkSession, propKeys)
Review Comment:
**[P1] Normalize aliases on both sides before deleting**
The catalog removal above uses the requested spelling verbatim, but this call canonicalizes SQL aliases before deleting from `hoodie.properties`. If the catalog contains the supported legacy alias `preCombineField` and the user unsets `orderingFields`, `newProperties` leaves `preCombineField` in the catalog while this path maps `orderingFields` to `hoodie.table.ordering.fields` and deletes the persisted config. That recreates the catalog/file divergence this PR is meant to fix. Please resolve aliases to one canonical identity when deciding what to remove from both stores.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: ALTER TABLE Hudi property persistence [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19733:
URL: https://github.com/apache/hudi/pull/19733#issuecomment-5434070801
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "1c1a8f991c98fe4509dba421779270dd50b4fa88",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16539",
"triggerID" : "1c1a8f991c98fe4509dba421779270dd50b4fa88",
"triggerType" : "PUSH"
}, {
"hash" : "64cec58d4b1f5ea19b719df4feb66fd25b04fb2d",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16598",
"triggerID" : "64cec58d4b1f5ea19b719df4feb66fd25b04fb2d",
"triggerType" : "PUSH"
}, {
"hash" : "2e0d5f74750eb7dc45f18cfe6384cd4f3cf7fb03",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "2e0d5f74750eb7dc45f18cfe6384cd4f3cf7fb03",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 64cec58d4b1f5ea19b719df4feb66fd25b04fb2d Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16598)
* 2e0d5f74750eb7dc45f18cfe6384cd4f3cf7fb03 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: ALTER TABLE Hudi property persistence [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19733:
URL: https://github.com/apache/hudi/pull/19733#issuecomment-5427473441
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "1c1a8f991c98fe4509dba421779270dd50b4fa88",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16539",
"triggerID" : "1c1a8f991c98fe4509dba421779270dd50b4fa88",
"triggerType" : "PUSH"
}, {
"hash" : "64cec58d4b1f5ea19b719df4feb66fd25b04fb2d",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16598",
"triggerID" : "64cec58d4b1f5ea19b719df4feb66fd25b04fb2d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 1c1a8f991c98fe4509dba421779270dd50b4fa88 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16539)
* 64cec58d4b1f5ea19b719df4feb66fd25b04fb2d Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16598)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: ALTER TABLE Hudi property persistence [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19733:
URL: https://github.com/apache/hudi/pull/19733#issuecomment-5438158042
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "1c1a8f991c98fe4509dba421779270dd50b4fa88",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16539",
"triggerID" : "1c1a8f991c98fe4509dba421779270dd50b4fa88",
"triggerType" : "PUSH"
}, {
"hash" : "64cec58d4b1f5ea19b719df4feb66fd25b04fb2d",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16598",
"triggerID" : "64cec58d4b1f5ea19b719df4feb66fd25b04fb2d",
"triggerType" : "PUSH"
}, {
"hash" : "2e0d5f74750eb7dc45f18cfe6384cd4f3cf7fb03",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16607",
"triggerID" : "2e0d5f74750eb7dc45f18cfe6384cd4f3cf7fb03",
"triggerType" : "PUSH"
}, {
"hash" : "28c33a02fb4281bc0bd0c2c7cee4526570a1dc86",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "28c33a02fb4281bc0bd0c2c7cee4526570a1dc86",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2e0d5f74750eb7dc45f18cfe6384cd4f3cf7fb03 Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16607)
* 28c33a02fb4281bc0bd0c2c7cee4526570a1dc86 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: ALTER TABLE Hudi property persistence [hudi]
Posted by "hudi-agent (via GitHub)" <gi...@apache.org>.
hudi-agent commented on code in PR #19733:
URL: https://github.com/apache/hudi/pull/19733#discussion_r3868563978
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/command/AlterTableCommand.scala:
##########
@@ -208,10 +211,98 @@ case class AlterTableCommand(table: CatalogTable, changes: Seq[TableChange], cha
val newTable = table.copy(
properties = table.properties ++ properties,
comment = properties.get(TableCatalog.PROP_COMMENT).orElse(table.comment))
+ updateHoodieTableConfigs(sparkSession, properties)
catalog.alterTable(newTable)
logInfo("table properties change finished")
}
+ /**
Review Comment:
🤖 nit: this Scaladoc block describes the update/delete methods' design intent (alias mapping, catalog-only for non-Hudi props, etc.), but it's placed directly above `protectedTableConfigs` so readers will associate it with the val — could you move it to sit above `updateHoodieTableConfigs` where it actually applies?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/command/AlterTableCommand.scala:
##########
@@ -208,10 +211,98 @@ case class AlterTableCommand(table: CatalogTable, changes: Seq[TableChange], cha
val newTable = table.copy(
properties = table.properties ++ properties,
comment = properties.get(TableCatalog.PROP_COMMENT).orElse(table.comment))
+ updateHoodieTableConfigs(sparkSession, properties)
catalog.alterTable(newTable)
logInfo("table properties change finished")
}
+ /**
+ * Persists Hudi table properties in hoodie.properties as well as the Spark catalog.
+ *
+ * The analyzer only creates this command for Hudi V2 tables, so the table is known to be a Hudi
+ * table. Keep non-Hudi properties catalog-only, matching Spark's normal ALTER TABLE behavior.
+ * Hudi's SQL aliases and datasource options are converted to the canonical keys stored in
+ * hoodie.properties before validation and persistence.
+ */
+ private val protectedTableConfigs = Set(
Review Comment:
🤖 nit: the field is called `protectedTableConfigs` but the thrown error messages say "immutable table config" / "immutable table property" — could you rename it to `immutableTableConfigs` so the two align? Also, since this set doesn't depend on instance state, it might be cleaner in the companion object to avoid reallocating it per instance.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: ALTER TABLE Hudi property persistence [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19733:
URL: https://github.com/apache/hudi/pull/19733#issuecomment-5429313721
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "1c1a8f991c98fe4509dba421779270dd50b4fa88",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16539",
"triggerID" : "1c1a8f991c98fe4509dba421779270dd50b4fa88",
"triggerType" : "PUSH"
}, {
"hash" : "64cec58d4b1f5ea19b719df4feb66fd25b04fb2d",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16598",
"triggerID" : "64cec58d4b1f5ea19b719df4feb66fd25b04fb2d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 64cec58d4b1f5ea19b719df4feb66fd25b04fb2d Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16598)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: ALTER TABLE Hudi property persistence [hudi]
Posted by "hudi-agent (via GitHub)" <gi...@apache.org>.
hudi-agent commented on code in PR #19733:
URL: https://github.com/apache/hudi/pull/19733#discussion_r3856931758
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/command/AlterTableCommand.scala:
##########
@@ -208,10 +210,57 @@ case class AlterTableCommand(table: CatalogTable, changes: Seq[TableChange], cha
val newTable = table.copy(
properties = table.properties ++ properties,
comment = properties.get(TableCatalog.PROP_COMMENT).orElse(table.comment))
+ updateHoodieTableConfigs(sparkSession, properties)
catalog.alterTable(newTable)
logInfo("table properties change finished")
}
+ /**
+ * Persists Hudi table properties in hoodie.properties as well as the Spark catalog.
+ *
+ * The analyzer only creates this command for Hudi V2 tables, so the table is known to be a Hudi
+ * table. Keep non-Hudi properties catalog-only, matching Spark's normal ALTER TABLE behavior.
+ * Hudi's SQL aliases and datasource options are converted to the canonical keys stored in
+ * hoodie.properties before validation and persistence.
+ */
+ private def updateHoodieTableConfigs(sparkSession: SparkSession, properties: Map[String, String]): Unit = {
+ val tableConfigs = HoodieOptionConfig.mapSqlOptionsToTableConfigs(
+ HoodieOptionConfig.extractHoodieOptions(properties))
+
+ if (tableConfigs.nonEmpty) {
+ val metaClient = getMetaClient(sparkSession)
+
+ HoodieWriterUtils.validateTableConfig(
+ sparkSession,
+ tableConfigs,
+ metaClient.getTableConfig)
+
+ HoodieTableConfig.update(
+ metaClient.getStorage,
+ metaClient.getMetaPath,
+ TypedProperties.fromMap(tableConfigs.asJava))
+ }
+ }
+ private def deleteHoodieTableConfigs(sparkSession: SparkSession, propertyKeys: Seq[String]): Unit = {
+ val tableConfigs = HoodieOptionConfig.mapSqlOptionsToTableConfigs(
+ HoodieOptionConfig.extractHoodieOptions(propertyKeys.map(_ -> "").toMap))
Review Comment:
🤖 nit: the `map(_ -> "").toMap` trick — using dummy empty values just to get key translation — is a bit surprising on first read. A short inline comment like `// dummy values; only the translated keys are used` would save the next reader a minute of head-scratching.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/command/AlterTableCommand.scala:
##########
@@ -208,10 +210,57 @@ case class AlterTableCommand(table: CatalogTable, changes: Seq[TableChange], cha
val newTable = table.copy(
properties = table.properties ++ properties,
comment = properties.get(TableCatalog.PROP_COMMENT).orElse(table.comment))
+ updateHoodieTableConfigs(sparkSession, properties)
catalog.alterTable(newTable)
logInfo("table properties change finished")
}
+ /**
+ * Persists Hudi table properties in hoodie.properties as well as the Spark catalog.
+ *
+ * The analyzer only creates this command for Hudi V2 tables, so the table is known to be a Hudi
+ * table. Keep non-Hudi properties catalog-only, matching Spark's normal ALTER TABLE behavior.
+ * Hudi's SQL aliases and datasource options are converted to the canonical keys stored in
+ * hoodie.properties before validation and persistence.
+ */
+ private def updateHoodieTableConfigs(sparkSession: SparkSession, properties: Map[String, String]): Unit = {
+ val tableConfigs = HoodieOptionConfig.mapSqlOptionsToTableConfigs(
+ HoodieOptionConfig.extractHoodieOptions(properties))
+
+ if (tableConfigs.nonEmpty) {
+ val metaClient = getMetaClient(sparkSession)
+
+ HoodieWriterUtils.validateTableConfig(
+ sparkSession,
+ tableConfigs,
+ metaClient.getTableConfig)
+
+ HoodieTableConfig.update(
+ metaClient.getStorage,
+ metaClient.getMetaPath,
+ TypedProperties.fromMap(tableConfigs.asJava))
+ }
+ }
+ private def deleteHoodieTableConfigs(sparkSession: SparkSession, propertyKeys: Seq[String]): Unit = {
+ val tableConfigs = HoodieOptionConfig.mapSqlOptionsToTableConfigs(
+ HoodieOptionConfig.extractHoodieOptions(propertyKeys.map(_ -> "").toMap))
+
+ if (tableConfigs.nonEmpty) {
+ val metaClient = getMetaClient(sparkSession)
+ HoodieTableConfig.delete(
+ metaClient.getStorage,
+ metaClient.getMetaPath,
+ tableConfigs.keySet.asJava)
+ }
Review Comment:
🤖 Agree this is worth guarding. `extractHoodieOptions` passes through any `hoodie.*` key, so UNSET could delete immutable configs like `hoodie.table.recordkey.fields`, `hoodie.table.name`, or `hoodie.table.partition.fields` from hoodie.properties and leave the table unreadable. Restricting UNSET to a known-mutable set (or routing it through the same validation) rather than deleting arbitrary hoodie keys seems safer.
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/command/AlterTableCommand.scala:
##########
@@ -208,10 +210,57 @@ case class AlterTableCommand(table: CatalogTable, changes: Seq[TableChange], cha
val newTable = table.copy(
properties = table.properties ++ properties,
comment = properties.get(TableCatalog.PROP_COMMENT).orElse(table.comment))
+ updateHoodieTableConfigs(sparkSession, properties)
catalog.alterTable(newTable)
logInfo("table properties change finished")
}
+ /**
+ * Persists Hudi table properties in hoodie.properties as well as the Spark catalog.
+ *
+ * The analyzer only creates this command for Hudi V2 tables, so the table is known to be a Hudi
+ * table. Keep non-Hudi properties catalog-only, matching Spark's normal ALTER TABLE behavior.
+ * Hudi's SQL aliases and datasource options are converted to the canonical keys stored in
+ * hoodie.properties before validation and persistence.
+ */
+ private def updateHoodieTableConfigs(sparkSession: SparkSession, properties: Map[String, String]): Unit = {
+ val tableConfigs = HoodieOptionConfig.mapSqlOptionsToTableConfigs(
+ HoodieOptionConfig.extractHoodieOptions(properties))
Review Comment:
🤖 `extractHoodieOptions` keeps every key starting with `hoodie.`, and `mapSqlOptionsToTableConfigs` passes keys it doesn't recognize through unchanged, so this looks like it will persist *any* `hoodie.*` property into `hoodie.properties` — not just table configs. e.g. `SET TBLPROPERTIES('hoodie.upsert.shuffle.parallelism'='100')` or a `hoodie.datasource.write.*` key would get written there. Since `validateTableConfig` only flags value *conflicts* with existing keys (a brand-new key has `existingValue == null`), nothing filters these out, and they'd become persistent table-level defaults. Could you restrict persistence to actual `HoodieTableConfig` keys?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/command/AlterTableCommand.scala:
##########
@@ -208,10 +210,57 @@ case class AlterTableCommand(table: CatalogTable, changes: Seq[TableChange], cha
val newTable = table.copy(
properties = table.properties ++ properties,
comment = properties.get(TableCatalog.PROP_COMMENT).orElse(table.comment))
+ updateHoodieTableConfigs(sparkSession, properties)
catalog.alterTable(newTable)
logInfo("table properties change finished")
}
+ /**
+ * Persists Hudi table properties in hoodie.properties as well as the Spark catalog.
+ *
+ * The analyzer only creates this command for Hudi V2 tables, so the table is known to be a Hudi
+ * table. Keep non-Hudi properties catalog-only, matching Spark's normal ALTER TABLE behavior.
+ * Hudi's SQL aliases and datasource options are converted to the canonical keys stored in
+ * hoodie.properties before validation and persistence.
+ */
+ private def updateHoodieTableConfigs(sparkSession: SparkSession, properties: Map[String, String]): Unit = {
+ val tableConfigs = HoodieOptionConfig.mapSqlOptionsToTableConfigs(
+ HoodieOptionConfig.extractHoodieOptions(properties))
+
+ if (tableConfigs.nonEmpty) {
+ val metaClient = getMetaClient(sparkSession)
+
+ HoodieWriterUtils.validateTableConfig(
+ sparkSession,
+ tableConfigs,
+ metaClient.getTableConfig)
+
Review Comment:
🤖 Confirmed from the code — `validateTableConfig` appends to `diffConfigs` and throws whenever an incoming key already exists in the table config with a different value (HoodieWriterUtils around line 301). Only a small whitelist in `shouldIgnoreConfig` (base file format, a few payload/merge-mode cases, empty database name) is exempt, so any genuine change to an already-persisted config would fail rather than update. Might be worth clarifying whether SET is intended to be create-only, or whether the mutable-config case needs a different path.
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/command/AlterTableCommand.scala:
##########
@@ -208,10 +210,57 @@ case class AlterTableCommand(table: CatalogTable, changes: Seq[TableChange], cha
val newTable = table.copy(
properties = table.properties ++ properties,
comment = properties.get(TableCatalog.PROP_COMMENT).orElse(table.comment))
+ updateHoodieTableConfigs(sparkSession, properties)
Review Comment:
🤖 Here (and in applyPropertyUnset at line 198) hoodie.properties is mutated before `catalog.alterTable`. If `catalog.alterTable` throws, hoodie.properties has already been updated/deleted while the Spark catalog stays unchanged, leaving the two out of sync with no rollback — and on UNSET the delete is irreversible. Is there a reason to persist to hoodie.properties first rather than after the catalog update succeeds, or is some compensating cleanup expected on failure?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: ALTER TABLE Hudi property persistence [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19733:
URL: https://github.com/apache/hudi/pull/19733#issuecomment-5439230097
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "1c1a8f991c98fe4509dba421779270dd50b4fa88",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16539",
"triggerID" : "1c1a8f991c98fe4509dba421779270dd50b4fa88",
"triggerType" : "PUSH"
}, {
"hash" : "64cec58d4b1f5ea19b719df4feb66fd25b04fb2d",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16598",
"triggerID" : "64cec58d4b1f5ea19b719df4feb66fd25b04fb2d",
"triggerType" : "PUSH"
}, {
"hash" : "2e0d5f74750eb7dc45f18cfe6384cd4f3cf7fb03",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16607",
"triggerID" : "2e0d5f74750eb7dc45f18cfe6384cd4f3cf7fb03",
"triggerType" : "PUSH"
}, {
"hash" : "28c33a02fb4281bc0bd0c2c7cee4526570a1dc86",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16626",
"triggerID" : "28c33a02fb4281bc0bd0c2c7cee4526570a1dc86",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 28c33a02fb4281bc0bd0c2c7cee4526570a1dc86 Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16626)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: ALTER TABLE Hudi property persistence [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19733:
URL: https://github.com/apache/hudi/pull/19733#issuecomment-5416214064
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "1c1a8f991c98fe4509dba421779270dd50b4fa88",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16539",
"triggerID" : "1c1a8f991c98fe4509dba421779270dd50b4fa88",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 1c1a8f991c98fe4509dba421779270dd50b4fa88 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16539)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix ALTER TABLE Hudi property persistence [hudi]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #19733:
URL: https://github.com/apache/hudi/pull/19733#discussion_r3855957917
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/command/AlterTableCommand.scala:
##########
@@ -208,10 +210,57 @@ case class AlterTableCommand(table: CatalogTable, changes: Seq[TableChange], cha
val newTable = table.copy(
properties = table.properties ++ properties,
comment = properties.get(TableCatalog.PROP_COMMENT).orElse(table.comment))
+ updateHoodieTableConfigs(sparkSession, properties)
catalog.alterTable(newTable)
logInfo("table properties change finished")
}
+ /**
+ * Persists Hudi table properties in hoodie.properties as well as the Spark catalog.
+ *
+ * The analyzer only creates this command for Hudi V2 tables, so the table is known to be a Hudi
+ * table. Keep non-Hudi properties catalog-only, matching Spark's normal ALTER TABLE behavior.
+ * Hudi's SQL aliases and datasource options are converted to the canonical keys stored in
+ * hoodie.properties before validation and persistence.
+ */
+ private def updateHoodieTableConfigs(sparkSession: SparkSession, properties: Map[String, String]): Unit = {
+ val tableConfigs = HoodieOptionConfig.mapSqlOptionsToTableConfigs(
+ HoodieOptionConfig.extractHoodieOptions(properties))
+
+ if (tableConfigs.nonEmpty) {
+ val metaClient = getMetaClient(sparkSession)
+
+ HoodieWriterUtils.validateTableConfig(
+ sparkSession,
+ tableConfigs,
+ metaClient.getTableConfig)
+
+ HoodieTableConfig.update(
+ metaClient.getStorage,
+ metaClient.getMetaPath,
+ TypedProperties.fromMap(tableConfigs.asJava))
+ }
+ }
+ private def deleteHoodieTableConfigs(sparkSession: SparkSession, propertyKeys: Seq[String]): Unit = {
+ val tableConfigs = HoodieOptionConfig.mapSqlOptionsToTableConfigs(
+ HoodieOptionConfig.extractHoodieOptions(propertyKeys.map(_ -> "").toMap))
+
+ if (tableConfigs.nonEmpty) {
+ val metaClient = getMetaClient(sparkSession)
+ HoodieTableConfig.delete(
+ metaClient.getStorage,
+ metaClient.getMetaPath,
+ tableConfigs.keySet.asJava)
+ }
Review Comment:
`deleteHoodieTableConfigs` deletes keys from `hoodie.properties` without any validation. This allows `ALTER TABLE ... UNSET TBLPROPERTIES` to remove runtime-immutable configs (for example, `hoodie.meta.fields.mode`) and bypass the config immutability/conflict checks enforced elsewhere, potentially leaving the table in an unsupported state.
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/command/AlterTableCommand.scala:
##########
@@ -208,10 +210,57 @@ case class AlterTableCommand(table: CatalogTable, changes: Seq[TableChange], cha
val newTable = table.copy(
properties = table.properties ++ properties,
comment = properties.get(TableCatalog.PROP_COMMENT).orElse(table.comment))
+ updateHoodieTableConfigs(sparkSession, properties)
catalog.alterTable(newTable)
logInfo("table properties change finished")
Review Comment:
This change adds side effects on `hoodie.properties` for SET/UNSET TBLPROPERTIES, but there is no coverage here ensuring `hoodie.properties` is updated/deleted accordingly (and that non-Hudi properties remain catalog-only). Existing `TestAlterTable` does not exercise SET/UNSET TBLPROPERTIES.
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/command/AlterTableCommand.scala:
##########
@@ -208,10 +210,57 @@ case class AlterTableCommand(table: CatalogTable, changes: Seq[TableChange], cha
val newTable = table.copy(
properties = table.properties ++ properties,
comment = properties.get(TableCatalog.PROP_COMMENT).orElse(table.comment))
+ updateHoodieTableConfigs(sparkSession, properties)
catalog.alterTable(newTable)
logInfo("table properties change finished")
}
+ /**
+ * Persists Hudi table properties in hoodie.properties as well as the Spark catalog.
+ *
+ * The analyzer only creates this command for Hudi V2 tables, so the table is known to be a Hudi
+ * table. Keep non-Hudi properties catalog-only, matching Spark's normal ALTER TABLE behavior.
+ * Hudi's SQL aliases and datasource options are converted to the canonical keys stored in
+ * hoodie.properties before validation and persistence.
+ */
+ private def updateHoodieTableConfigs(sparkSession: SparkSession, properties: Map[String, String]): Unit = {
+ val tableConfigs = HoodieOptionConfig.mapSqlOptionsToTableConfigs(
+ HoodieOptionConfig.extractHoodieOptions(properties))
+
+ if (tableConfigs.nonEmpty) {
+ val metaClient = getMetaClient(sparkSession)
+
+ HoodieWriterUtils.validateTableConfig(
+ sparkSession,
+ tableConfigs,
+ metaClient.getTableConfig)
+
Review Comment:
`updateHoodieTableConfigs` calls `HoodieWriterUtils.validateTableConfig(...)`, which rejects any SET of a table-config key when the key already exists in `hoodie.properties` with a different value. That means `ALTER TABLE ... SET TBLPROPERTIES` cannot actually *change* existing persisted Hudi table configs (it will throw instead), which conflicts with the PR description that these properties are now persisted/changed via ALTER TABLE.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix ALTER TABLE Hudi property persistence [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19733:
URL: https://github.com/apache/hudi/pull/19733#issuecomment-5414876787
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "1c1a8f991c98fe4509dba421779270dd50b4fa88",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16539",
"triggerID" : "1c1a8f991c98fe4509dba421779270dd50b4fa88",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 1c1a8f991c98fe4509dba421779270dd50b4fa88 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16539)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix ALTER TABLE Hudi property persistence [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19733:
URL: https://github.com/apache/hudi/pull/19733#issuecomment-5414797714
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "1c1a8f991c98fe4509dba421779270dd50b4fa88",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "1c1a8f991c98fe4509dba421779270dd50b4fa88",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 1c1a8f991c98fe4509dba421779270dd50b4fa88 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: ALTER TABLE Hudi property persistence [hudi]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #19733:
URL: https://github.com/apache/hudi/pull/19733#issuecomment-5415148339
## [Codecov](https://app.codecov.io/gh/apache/hudi/pull/19733?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 77.95%. Comparing base ([`ef07f0f`](https://app.codecov.io/gh/apache/hudi/commit/ef07f0f948c9260eeaf07d5652ab08cb5401f6e1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`1c1a8f9`](https://app.codecov.io/gh/apache/hudi/commit/1c1a8f991c98fe4509dba421779270dd50b4fa88?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 2 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #19733 +/- ##
=========================================
Coverage 77.95% 77.95%
- Complexity 33447 33449 +2
=========================================
Files 2539 2539
Lines 140891 140891
Branches 17008 17008
=========================================
+ Hits 109829 109833 +4
+ Misses 23401 23399 -2
+ Partials 7661 7659 -2
```
| [Components](https://app.codecov.io/gh/apache/hudi/pull/19733/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hudi-common](https://app.codecov.io/gh/apache/hudi/pull/19733/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.45% <ø> (+<0.01%)` | :arrow_up: |
| [hudi-client](https://app.codecov.io/gh/apache/hudi/pull/19733/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.04% <ø> (+<0.01%)` | :arrow_up: |
| [hudi-flink](https://app.codecov.io/gh/apache/hudi/pull/19733/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `85.66% <ø> (ø)` | |
| [hudi-spark-datasource](https://app.codecov.io/gh/apache/hudi/pull/19733/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `72.37% <ø> (ø)` | |
| [hudi-utilities](https://app.codecov.io/gh/apache/hudi/pull/19733/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `74.33% <ø> (ø)` | |
| [hudi-cli](https://app.codecov.io/gh/apache/hudi/pull/19733/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `15.06% <ø> (ø)` | |
| [hudi-hadoop](https://app.codecov.io/gh/apache/hudi/pull/19733/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `69.23% <ø> (ø)` | |
| [hudi-sync](https://app.codecov.io/gh/apache/hudi/pull/19733/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `75.44% <ø> (ø)` | |
| [hudi-io](https://app.codecov.io/gh/apache/hudi/pull/19733/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `79.85% <ø> (ø)` | |
| [hudi-timeline-service](https://app.codecov.io/gh/apache/hudi/pull/19733/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.44% <ø> (ø)` | |
| [hudi-cloud](https://app.codecov.io/gh/apache/hudi/pull/19733/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `64.33% <ø> (ø)` | |
| [hudi-kafka-connect](https://app.codecov.io/gh/apache/hudi/pull/19733/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.20% <ø> (ø)` | |
| [Flag](https://app.codecov.io/gh/apache/hudi/pull/19733/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hadoop-mr-java-client](https://app.codecov.io/gh/apache/hudi/pull/19733/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `43.75% <ø> (+0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Files with missing lines](https://app.codecov.io/gh/apache/hudi/pull/19733?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...che/spark/sql/hudi/command/AlterTableCommand.scala](https://app.codecov.io/gh/apache/hudi/pull/19733?src=pr&el=tree&filepath=hudi-spark-datasource%2Fhudi-spark-common%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fspark%2Fsql%2Fhudi%2Fcommand%2FAlterTableCommand.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1zcGFyay1kYXRhc291cmNlL2h1ZGktc3BhcmstY29tbW9uL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvc3Bhcmsvc3FsL2h1ZGkvY29tbWFuZC9BbHRlclRhYmxlQ29tbWFuZC5zY2FsYQ==) | `78.07% <ø> (ø)` | |
... and [2 files with indirect coverage changes](https://app.codecov.io/gh/apache/hudi/pull/19733/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org