danny0405 commented on code in PR #19205:
URL: https://github.com/apache/hudi/pull/19205#discussion_r3662309279
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieWriteClient.java:
##########
@@ -1548,9 +1549,40 @@ public void
validateAgainstTableProperties(HoodieTableConfig tableConfig, Hoodie
// mismatch of table versions.
CommonClientUtils.validateTableVersion(tableConfig, writeConfig);
- // Once meta fields are disabled, it cant be re-enabled for a given table.
- if (!tableConfig.populateMetaFields() && writeConfig.populateMetaFields())
{
- throw new HoodieException(HoodieTableConfig.POPULATE_META_FIELDS.key() +
" already disabled for the table. Can't be re-enabled back");
+ // Meta-field population is physical, so a writer must not claim columns
the table does not
+ // have. Compare the full enum rather than the legacy booleans: those
collapse every selective
+ // mode to false, so a writer claiming COMMIT_TIME_ONLY against a NONE
table would slip through
+ // and advertise commit times that were never written.
+ //
+ // Two distinct cases, because writers routinely omit meta-field settings
entirely:
+ //
+ // - Widening is always rejected. Enabling a column now would leave
earlier commits without it,
+ // and readers cannot tell the two apart.
+ // - Any disagreement is rejected when the writer *explicitly* sets
hoodie.meta.fields.mode.
+ // That covers narrowing too, e.g. an explicit NONE against a
COMMIT_TIME_ONLY table, which
+ // would write null commit times while the table still advertises
COMMIT_TIME_ONLY and make
+ // incremental queries silently miss those rows.
+ //
+ // A writer that never mentions the mode is left alone: resolving to NONE
against an ALL table
+ // is long-standing behavior for callers that build a write config without
restating the table's
+ // settings, and writing fewer meta columns cannot make a reader believe
in absent data.
+ MetaFieldsMode tableMetaFieldsMode = tableConfig.getMetaFieldsMode();
+ MetaFieldsMode writeMetaFieldsMode = writeConfig.getMetaFieldsMode();
+ boolean writerStatedMode =
writeConfig.contains(HoodieTableConfig.META_FIELDS_MODE)
+ &&
!StringUtils.isNullOrEmpty(writeConfig.getString(HoodieTableConfig.META_FIELDS_MODE));
+ if (writeMetaFieldsMode.isWiderThan(tableMetaFieldsMode)) {
+ throw new HoodieException(String.format(
+ "%s cannot be widened for an existing table: table is %s but the
writer requests %s. Meta "
+ + "columns are physical, so enabling one now would leave earlier
commits without it. "
+ + "Set %s=%s on the writer, or recreate the table to change it.",
+ HoodieTableConfig.META_FIELDS_MODE.key(), tableMetaFieldsMode,
writeMetaFieldsMode,
+ HoodieTableConfig.META_FIELDS_MODE.key(), tableMetaFieldsMode));
+ } else if (writerStatedMode && writeMetaFieldsMode != tableMetaFieldsMode)
{
Review Comment:
if writeMetaFieldsMode != tableMetaFieldsMode is true, then writerStatedMode
must be true right? so we can eliminate writerStatedMode.
--
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]
