danny0405 commented on code in PR #19205:
URL: https://github.com/apache/hudi/pull/19205#discussion_r3746071172
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieWriteClient.java:
##########
@@ -1544,13 +1545,80 @@ protected boolean loadActiveTimelineOnTableInit() {
return true;
}
+ /**
+ * Adopt the table's {@code hoodie.meta.fields.mode} when this writer did
not state one.
+ *
+ * <p>The mode is a table property: it is settable at table creation,
through hudi-cli, or by an
+ * upgrade, and never by an ordinary write. Callers routinely build a write
config without
+ * restating the table's meta-field settings — table services do, and so
does a restarted
+ * StreamSync — and such a writer must write what the table already
advertises rather than silently
+ * narrowing it. Without this, an unstated writer resolves to {@code NONE}
(via the deprecated
+ * {@code hoodie.populate.meta.fields} fallback) and writes null meta
columns into a table whose
+ * earlier files have them populated.
+ *
+ * <p>Inheritance applies only when the writer states <em>neither</em>
property. If it explicitly
+ * set the mode or the deprecated boolean, that value is left alone so the
comparison below can
+ * reject it on a mismatch: a user who deliberately passed {@code
populate.meta.fields=false}
+ * against an {@code ALL} table should be told the setting conflicts, not
have it silently
+ * overridden.
+ *
+ * @return true when the writer stated neither property, i.e. the mode was
inherited and cannot
+ * disagree with the table.
+ */
+ private static boolean inferMetaFieldsModeFromTable(HoodieTableConfig
tableConfig, HoodieWriteConfig writeConfig) {
+ boolean statedMode =
writeConfig.contains(HoodieTableConfig.META_FIELDS_MODE)
+ &&
!StringUtils.isNullOrEmpty(writeConfig.getString(HoodieTableConfig.META_FIELDS_MODE));
+ boolean statedLegacyBoolean =
writeConfig.contains(HoodieTableConfig.POPULATE_META_FIELDS);
+ if (statedMode || statedLegacyBoolean) {
+ return false;
+ }
+ MetaFieldsMode tableMode = tableConfig.getMetaFieldsMode();
+ writeConfig.setValue(HoodieTableConfig.META_FIELDS_MODE, tableMode.name());
+ // Keep the derived boolean in step, so the ~55 call sites still reading
populateMetaFields()
+ // observe an answer consistent with the mode.
+ writeConfig.setValue(HoodieTableConfig.POPULATE_META_FIELDS,
+ Boolean.toString(tableMode.toLegacyPopulateMetaFields()));
+ return true;
+ }
+
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");
+ // A writer that stated neither meta-field property inherits the table's
mode here, which makes
+ // the comparison below a no-op for it. A writer that stated either keeps
its value and is
+ // compared.
+ inferMetaFieldsModeFromTable(tableConfig, writeConfig);
Review Comment:
1. the inference should happen in the table config construction instead of
here;
2. here we just do some validation instead of modifying the write config
silently;
3. for V9 table, just check `populateMetaFields`;
4. for V10 table, just check `META_FIELDS_MODE`, since `populateMetaFields`
is a table config option, if user still specifies this legacy option for v10,
we should already handle it in 1 for new table, for table migrated from v9, the
option should already be migrated as `META_FIELDS_MODE` during upgrade, and
here we can just do the validation to see if `populateMetaFields` and
`META_FIELDS_MODE` conflicts in semantics;
--
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]