danny0405 commented on code in PR #19205:
URL: https://github.com/apache/hudi/pull/19205#discussion_r3651812711
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:
##########
@@ -3883,6 +3932,27 @@ private void validate() {
checkArgument(ttlStatsMaxParallelism > 0,
String.format("%s must be positive, but was %d",
HoodieTTLConfig.STATS_MAX_PARALLELISM.key(),
ttlStatsMaxParallelism));
+
+ // hoodie.meta.fields.mode is the source of truth for meta-column
population; the deprecated
+ // populate.meta.fields boolean is consulted only when the mode is
absent. There is therefore
+ // no ambiguous combination to reject here — MetaFieldsMode.resolve
throws on unrecognized
+ // values.
+ MetaFieldsMode metaFieldsMode = writeConfig.getMetaFieldsMode();
Review Comment:
[P1] `populate.meta.fields=true` must not be accepted with `NONE` or a
selective mode unless the legacy flag is rewritten before table creation. The
new builder/table-builder paths persist the two properties independently, so a
table can be written selectively while `hoodie.properties` still says
`hoodie.populate.meta.fields=true`. Pre-1.3 readers ignore the new property and
then treat the table as `ALL`; for `NONE`, an older incremental reader can be
allowed to run while every commit time is null and silently return no rows.
Please either reject conflicting combinations or always persist the derived
legacy value (`ALL -> true`, every other mode -> false`). This also makes
`TestMetaFieldsMode#populateTrueWithSelectiveModeIsRejected` fail: no exception
is thrown.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:
##########
@@ -1772,8 +1773,40 @@ public int getSmallFileGroupCandidatesLimit() {
return getInt(MERGE_SMALL_FILE_GROUP_CANDIDATES_LIMIT);
}
+ /**
+ * @return true when every meta column is populated.
+ *
+ * <p>Derived from {@link #getMetaFieldsMode()} so that call sites still
written against the
+ * deprecated {@code hoodie.populate.meta.fields} boolean observe the same
answer as the enum:
+ * only {@link MetaFieldsMode#ALL} populates every meta column.
+ */
public boolean populateMetaFields() {
- return getBooleanOrDefault(HoodieTableConfig.POPULATE_META_FIELDS);
+ return getMetaFieldsMode().toLegacyPopulateMetaFields();
+ }
+
+ /**
+ * @return the {@link MetaFieldsMode} resolved from the write config.
+ * {@code hoodie.meta.fields.mode} is the source of truth; configs written
before that property
+ * existed fall back to {@link MetaFieldsMode#ALL} or {@link
MetaFieldsMode#NONE} based on the
+ * deprecated {@code hoodie.populate.meta.fields} boolean.
+ */
+ public MetaFieldsMode getMetaFieldsMode() {
+ return
MetaFieldsMode.resolve(getStringOrDefault(HoodieTableConfig.META_FIELDS_MODE),
Review Comment:
[P1] This resolves the mode solely from writer properties, but the common
client validation still compares only `populateMetaFields()`. A direct
`SparkRDDWriteClient` can therefore open a persisted `COMMIT_TIME_ONLY` table
with `populate=false` and no mode, resolve this to `NONE`, and pass
`BaseHoodieWriteClient.validateAgainstTableProperties()` because both legacy
booleans are false. The resulting files have null commit times while the table
remains marked `COMMIT_TIME_ONLY`, so incremental queries accept the table and
silently miss those rows. Please compare the full enum against the persisted
table mode in the base client validation (or inject the on-disk mode before
resolving), not only in the Spark datasource helper.
--
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]
