nsivabalan commented on code in PR #19205:
URL: https://github.com/apache/hudi/pull/19205#discussion_r3654575278
##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java:
##########
@@ -1230,11 +1250,58 @@ public String getTimelinePath() {
/**
* @returns true is meta fields need to be populated. else returns false.
+ *
+ * <p>Derived from {@link #getMetaFieldsMode()} so that call sites still
written against the
+ * deprecated boolean observe the same answer as the enum: only {@link
MetaFieldsMode#ALL}
+ * populates every meta column. Selective modes report {@code false} here,
which keeps
+ * key-dependent machinery (bloom filters, record-level index) correctly
disabled.
*/
public boolean populateMetaFields() {
+ return getMetaFieldsMode().toLegacyPopulateMetaFields();
+ }
+
+ /**
+ * @return the raw, deprecated {@code hoodie.populate.meta.fields} value,
used only as the
+ * fallback when {@link #META_FIELDS_MODE} is absent. Callers should use
+ * {@link #getMetaFieldsMode()} instead.
+ */
+ private boolean legacyPopulateMetaFields() {
return Boolean.parseBoolean(getStringOrDefault(POPULATE_META_FIELDS));
}
+ /**
+ * @return the {@link MetaFieldsMode} resolved from the on-disk properties.
{@link #META_FIELDS_MODE}
+ * is the source of truth; tables written before that property existed fall
back to
+ * {@link MetaFieldsMode#ALL} or {@link MetaFieldsMode#NONE} based on the
deprecated
+ * {@link #POPULATE_META_FIELDS} boolean.
+ */
+ public MetaFieldsMode getMetaFieldsMode() {
+ return MetaFieldsMode.resolve(getStringOrDefault(META_FIELDS_MODE),
legacyPopulateMetaFields());
Review Comment:
Mostly resolved by the P1 fix, and I think the remaining per-read resolution
is worth keeping. Two parts:
**Inference at creation** — this is now what happens.
[`664ff2e`](https://github.com/apache/hudi/pull/19205/commits/664ff2ec23ae)
makes `TableBuilder` write *both* properties consistently whenever a mode is
supplied, deriving the legacy boolean from the mode. `TableBuilder` is the
single chokepoint for every creation path (datasource, streamer, bootstrap), so
a table created by 1.3+ always has the two in agreement on disk, and
`resolve()` returns the mode branch without consulting the fallback.
**Reading the legacy option each time** — the fallback still runs for tables
created *before* this property existed, which is exactly the case it's there
for. I'd rather not cache the resolved value in a field: `HoodieTableConfig` is
mutable after load (`setValue` / `setAll` / `clearValue`, used by upgrade paths
and tests), so a cached enum would go stale on any property update. That's a
real correctness hazard in exchange for avoiding a map lookup plus a `valueOf`
— `getMetaFieldsMode()` isn't on a per-record path; it's consulted at
writer/reader construction.
If what you have in mind is a one-time migration that *rewrites* old tables
to carry the mode explicitly, that's the upgrade/downgrade work you raised in
your other comment — I'll pick that up there rather than duplicating the
mechanism here.
--
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]
