nsivabalan commented on code in PR #19205:
URL: https://github.com/apache/hudi/pull/19205#discussion_r3654507122
##########
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:
Fixed in
[`664ff2e`](https://github.com/apache/hudi/pull/19205/commits/664ff2ec23ae) —
you're right, and the forward-compat consequence you describe is the important
part.
Whenever an explicit mode is supplied, the legacy boolean is now **derived
from it** rather than taken from the caller (`ALL -> true`, every other mode ->
`false`), so `hoodie.properties` can never contradict the mode:
- `HoodieTableMetaClient.TableBuilder` — the chokepoint every table-creation
path goes through, including `HoodieSparkSqlWriter` and `StreamSync`, which
both call `setPopulateMetaFields(...)` and `setMetaFieldsModeFromString(...)`.
- `HoodieWriteConfig.Builder#withMetaFieldsMode` — so a write config handed
to table creation cannot carry a contradicting boolean either.
With no mode supplied, only the legacy boolean is recorded, so pre-1.3.0
on-disk behavior is unchanged.
I went with "always persist the derived value" rather than "reject the
combination" because rejecting would make `populate.meta.fields=true` + `ALL`
an error, which is a perfectly coherent request, and because the derived write
also fixes the case where the boolean is simply absent.
On `TestMetaFieldsMode#populateTrueWithSelectiveModeIsRejected`: you're
right that it was failing, and I should own how that happened. CI flagged it,
and I rewrote it to accept the new behavior — I read it as stale rather than as
the signal it was. It's now `selectiveModeWinsOverLegacyPopulateTrue`, and it
additionally asserts the persisted boolean is `false`, which is the invariant
that was actually broken. Added `noneModePersistsLegacyBooleanAsFalse` and
`allModePersistsLegacyBooleanAsTrue` alongside it, plus builder-level coverage
of all four combinations in `TestHoodieTableMetaClient`.
--
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]
