danny0405 commented on code in PR #19205:
URL: https://github.com/apache/hudi/pull/19205#discussion_r3773725075
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieWriteClient.java:
##########
@@ -1543,13 +1544,63 @@ protected boolean loadActiveTimelineOnTableInit() {
return true;
}
+ /**
+ * Pure validation: this method reads both configs and throws, and never
modifies either.
+ *
+ * <p>Nothing reconciles the write config against the table beforehand. That
is deliberate: the mode
+ * is read further down the write path by handles and writer factories, some
of which hold no table
+ * config at all, so the write config has to be correct on its own rather
than corrected on the way
+ * in. This gate is what makes that true, by refusing writes whose
meta-field settings do not already
+ * agree with the table.
+ */
public void validateAgainstTableProperties(HoodieTableConfig tableConfig,
HoodieWriteConfig writeConfig) {
// 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 disagree with
the table about which
+ // meta columns hold values. 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.
+ //
+ // hoodie.meta.fields.mode is a table property, changeable only via
hudi-cli or an upgrade -- never
+ // as a side effect of a write.
+ MetaFieldsMode tableMetaFieldsMode = tableConfig.getMetaFieldsMode();
+ // Used only to phrase the error: the deprecated boolean counts as a
statement of intent too, so a
+ // writer passing populate.meta.fields=false is told it "requests NONE"
rather than that it failed
+ // to state anything.
+ boolean writerStatedMetaFields =
+ (writeConfig.contains(HoodieTableConfig.META_FIELDS_MODE)
+ &&
!StringUtils.isNullOrEmpty(writeConfig.getString(HoodieTableConfig.META_FIELDS_MODE)))
+ || writeConfig.contains(HoodieTableConfig.POPULATE_META_FIELDS);
+
+ // The writer's resolved mode must equal the table's. Both directions are
wrong, for different
+ // reasons: widening would leave earlier commits missing a column later
ones have, and readers
+ // cannot tell the two apart; narrowing would leave rows the table still
advertises as populated,
+ // which incremental queries then silently skip. Only hudi-cli or an
upgrade may change the mode.
+ //
+ // This is checked on the resolved values rather than only on what the
writer stated, so it also
+ // catches the writer that stated nothing at all. Such a writer resolves
to the ALL default, which
+ // agrees with an ALL table -- the overwhelmingly common case, and the
reason nearly every existing
+ // caller is unaffected -- but disagrees with every other mode. Against
those, saying nothing is
+ // not a request to inherit; it is a writer that has not been told, and it
would go on to stamp the
+ // wrong set of meta columns.
+ MetaFieldsMode writeMetaFieldsMode = writeConfig.getMetaFieldsMode();
+ if (writeMetaFieldsMode != tableMetaFieldsMode) {
Review Comment:
nit: writerStatedMetaFields can be computed under the `if`.
--
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]
