[HUDI-4071] Enable schema reconciliation by default

[codope]

What is the purpose of the pull request

Segregating all the config flips in #5643 . This one enables reconcile schema by default.

Brief change log

(for example:)

• Modify AnnotationLocation checkstyle rule in checkstyle.xml

Verify this pull request

(Please pick either of the following options)

This pull request is a trivial rework / code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

• Added integration tests for end-to-end.
• Added HoodieClientWriteTest to verify the change.
• Manually verified the change by running a job locally.

Committer checklist

• Has a corresponding JIRA in PR title & commit

• Commit message is descriptive of the change

• CI is green

• Necessary doc changes done or have another open PR

• For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.

[xushiyan]

some spark sql test failed. is it actually safe to enable this? not sure if this config could be unintended in some cases.

[yihua]

@alexeykudinkin could you also review this PR?

[alexeykudinkin]

Initially was not in favor of this change, but now thinking about it a little more and especially in the light of #6358, i think this is the right thing to do: for ex, after #6358, we'd be allowing to go writes, which might have columns dropped in the new batch. Now, there are 2 scenarios based on whether the reconciliation is enabled or not:

1. If reconciliation is enabled: we will be favoring table's schema and use it as a writer-schema. So in that case we will rewrite the incoming batch into the table's schema before applying it to the table.

2. If reconciliation is disabled: we will be favoring incoming batch's schema and use it as a writer-schema. In this case, for ex, for COW, we will be reading the table in its existing schema, but the new base files will be written in the writer's schema (ie w/ the column dropped)

Both of these approaches are legitimate and could be preferred in different circumstances. What's important here for us is to pick the right default setting that would minimize the surprise effect.

Having reflected on this for some time now i think, that enabling reconciliation by default makes more sense as it protects table's schema from accidental mishaps in the incoming batches. And if somebody prefers the flow #2 the could easily opt-in for it by simply disabling the reconciliation.

WDYT?

[codope]

I agree. That was precisely the intention behind flipping this default.

[alexeykudinkin]

On the other hand, in its current form reconciliation doesn't allow to evolve the schema (unless comprehensive Schema Evolution is enabled) since it'll be essentially just favoring the table's schema always (there's no way for you to add new column for ex, other than switching off reconciliation)

[yihua]

Is the difference mainly around the case of dropping a column?

[yihua]

I’m thinking whether we should decouple the handling of drop column separately instead of turning on “schema reconciliation” by default, e.g., we should still allow new columns to be added instead of dropping them to favor table’s schema by default, while properly handling the column drop (maybe a different config?).

[alexeykudinkin]

@yihua it's more of a discussion what should be the default behavior:

• Should we (by default) favor existing table's schema a SoT and rewrite incoming batch into it (unless Schema Evolution is enabled, in that case we will try to evolve the schema)
• Should we (by default) favor incoming batch's schema as the schema we want table to be rewritten in

I still think that the #1 is a safer option as a default (optimizing for least amount of surprise to the user)

[alexeykudinkin]

@kazdy i think we just need to clearly disambiguate our configuration to make sure users can clearly understand what they can achieve and how (see my previous comment #6196 (comment)): what you're describing could be achieved today enabling Reconciliation and Schema Evolution.

[kazdy]

@alexeykudinkin afaik Schema Evolution config is there because it's an experimental feature and soon it will become GA? Then this config should be enabled by default or deprecated, will this logic hold then? I feel like hudi config is already very broad and therefore a bit hard to grasp and users would appreciate if it was one switch instead of a combination of two

[alexeykudinkin]

@kazdy correct, when Schema Evolution will become GA (cc @xiarixiaoyao) we will be flipping it to be on by default

[xiarixiaoyao]

@alexeykudinkin @kazdy , now schema evolution cannot read by hive and presto, but we aready has pr to support that
#6989
prestodb/presto#18557
#7045

once those pr merged, i think it will be ok.

[hudi-bot]

CI report:

• 1dfb9ff Azure: FAILURE

Bot commands

@hudi-bot supports the following commands:

• @hudi-bot run azure re-run the last Azure build

[codope]

Closing it as we first need to audit the full schema evolution scenarios.