Changing Update record failure semantics to be consistent with inserts - #33
Merged
Conversation
vinothchandar
commented
Jan 6, 2017
Member
- Don't skip, but writes the same old record again now
- Marks the correspoinding HoodieRecord as failure to be handed back to the client
- Don't skip, but writes the same old record again now - Marks the correspoinding HoodieRecord as failure to be handed back to the client
prazanna
approved these changes
Jan 6, 2017
prazanna
left a comment
Contributor
There was a problem hiding this comment.
Looks good. May be add a issue to add unit test for this change in logic.
| * | ||
| * We no longer need to copy the old record over. | ||
| */ | ||
| copyOldRecord = false; |
Contributor
There was a problem hiding this comment.
We should probably have a unit test for this. We can may be shelf that as a seperate jira and move on.
nsivabalan
added a commit
to nsivabalan/hudi
that referenced
this pull request
Aug 7, 2026
Reworks the tests voonhous identified as unable to fail on the bugs they were written for, and adds the coverage that was missing. Clustering (apache#34). The previous cases only reached assertNotNull on _hoodie_file_name, which the buggy pre-clustering value also satisfies -- reverting 358fbfd left them green. They now compare the column against input_file_name() for every row and assert a replacecommit exists, so the test fails if clustering either does not run or writes a stale file name. Reading through Hudi rather than globbing parquet also stops the replaced pre-clustering file from being inspected. The ALL and NONE cases are dropped: both route through writeRow / writeRowNoMetaFields and never enter the branch the fix touched. Metadata table (#32). One selective-mode write with MDT enabled, asserting the write succeeds and the MDT's own table resolves to NONE. Nothing previously proved the data table's mode cannot leak into HoodieMetadataWriteUtils' fresh config. Append (apache#33). Every other functional write is SaveMode.Overwrite, i.e. a fresh table, which skips inheritance entirely -- HoodieSparkSqlWriter folds table props in only when the mode is not Overwrite, and validateTableConfig is bypassed for Overwrite. Adds an append that restates nothing and asserts the table stays COMMIT_TIME_ONLY with commit times on both commits' rows. Streamer restart (apache#37). cshuo's regression had no test: the file had one ingestOnce(). Adds a second run that states neither meta-field property and asserts the mode survives and no row has a null commit time. The parameterized cases are trimmed to the three selective modes -- ALL and NONE add no mode key, so they exercised none of the new plumbing -- and no longer pair the mode with the legacy boolean, which is now a stated conflict. Strict null assertions (apache#40, apache#41). The two Scala assertions accepted both NULL and "", so they no longer pinned which representation is written, disagreeing with the assertNull in the functional test. Replaced by a shared assertNoMetaFieldsPopulated in HoodieSparkWriterTestBase. Naming (apache#36 and a collision). Two classes were named TestMetaFieldsMode; the functional one is now TestMetaFieldsModeE2E. TestHoodieMetaFieldsMode tested HoodieTableConfig resolution rather than the enum, so it is renamed TestHoodieTableConfigMetaFieldsMode, with its javadoc naming the two sibling classes and what each covers. TestHoodieTableConfig gains the storage round-trip assertion the in-memory cases cannot make. TestMetaFieldsModeE2E 27/27, TestHoodieTableConfig + resolution 107/107. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Merged
3 tasks
nsivabalan
added a commit
to nsivabalan/hudi
that referenced
this pull request
Aug 12, 2026
Reworks the tests voonhous identified as unable to fail on the bugs they were written for, and adds the coverage that was missing. Clustering (apache#34). The previous cases only reached assertNotNull on _hoodie_file_name, which the buggy pre-clustering value also satisfies -- reverting 358fbfd left them green. They now compare the column against input_file_name() for every row and assert a replacecommit exists, so the test fails if clustering either does not run or writes a stale file name. Reading through Hudi rather than globbing parquet also stops the replaced pre-clustering file from being inspected. The ALL and NONE cases are dropped: both route through writeRow / writeRowNoMetaFields and never enter the branch the fix touched. Metadata table (#32). One selective-mode write with MDT enabled, asserting the write succeeds and the MDT's own table resolves to NONE. Nothing previously proved the data table's mode cannot leak into HoodieMetadataWriteUtils' fresh config. Append (apache#33). Every other functional write is SaveMode.Overwrite, i.e. a fresh table, which skips inheritance entirely -- HoodieSparkSqlWriter folds table props in only when the mode is not Overwrite, and validateTableConfig is bypassed for Overwrite. Adds an append that restates nothing and asserts the table stays COMMIT_TIME_ONLY with commit times on both commits' rows. Streamer restart (apache#37). cshuo's regression had no test: the file had one ingestOnce(). Adds a second run that states neither meta-field property and asserts the mode survives and no row has a null commit time. The parameterized cases are trimmed to the three selective modes -- ALL and NONE add no mode key, so they exercised none of the new plumbing -- and no longer pair the mode with the legacy boolean, which is now a stated conflict. Strict null assertions (apache#40, apache#41). The two Scala assertions accepted both NULL and "", so they no longer pinned which representation is written, disagreeing with the assertNull in the functional test. Replaced by a shared assertNoMetaFieldsPopulated in HoodieSparkWriterTestBase. Naming (apache#36 and a collision). Two classes were named TestMetaFieldsMode; the functional one is now TestMetaFieldsModeE2E. TestHoodieMetaFieldsMode tested HoodieTableConfig resolution rather than the enum, so it is renamed TestHoodieTableConfigMetaFieldsMode, with its javadoc naming the two sibling classes and what each covers. TestHoodieTableConfig gains the storage round-trip assertion the in-memory cases cannot make. TestMetaFieldsModeE2E 27/27, TestHoodieTableConfig + resolution 107/107.
nsivabalan
added a commit
to nsivabalan/hudi
that referenced
this pull request
Aug 12, 2026
Reworks the tests voonhous identified as unable to fail on the bugs they were written for, and adds the coverage that was missing. Clustering (apache#34). The previous cases only reached assertNotNull on _hoodie_file_name, which the buggy pre-clustering value also satisfies -- reverting 358fbfd left them green. They now compare the column against input_file_name() for every row and assert a replacecommit exists, so the test fails if clustering either does not run or writes a stale file name. Reading through Hudi rather than globbing parquet also stops the replaced pre-clustering file from being inspected. The ALL and NONE cases are dropped: both route through writeRow / writeRowNoMetaFields and never enter the branch the fix touched. Metadata table (#32). One selective-mode write with MDT enabled, asserting the write succeeds and the MDT's own table resolves to NONE. Nothing previously proved the data table's mode cannot leak into HoodieMetadataWriteUtils' fresh config. Append (apache#33). Every other functional write is SaveMode.Overwrite, i.e. a fresh table, which skips inheritance entirely -- HoodieSparkSqlWriter folds table props in only when the mode is not Overwrite, and validateTableConfig is bypassed for Overwrite. Adds an append that restates nothing and asserts the table stays COMMIT_TIME_ONLY with commit times on both commits' rows. Streamer restart (apache#37). cshuo's regression had no test: the file had one ingestOnce(). Adds a second run that states neither meta-field property and asserts the mode survives and no row has a null commit time. The parameterized cases are trimmed to the three selective modes -- ALL and NONE add no mode key, so they exercised none of the new plumbing -- and no longer pair the mode with the legacy boolean, which is now a stated conflict. Strict null assertions (apache#40, apache#41). The two Scala assertions accepted both NULL and "", so they no longer pinned which representation is written, disagreeing with the assertNull in the functional test. Replaced by a shared assertNoMetaFieldsPopulated in HoodieSparkWriterTestBase. Naming (apache#36 and a collision). Two classes were named TestMetaFieldsMode; the functional one is now TestMetaFieldsModeE2E. TestHoodieMetaFieldsMode tested HoodieTableConfig resolution rather than the enum, so it is renamed TestHoodieTableConfigMetaFieldsMode, with its javadoc naming the two sibling classes and what each covers. TestHoodieTableConfig gains the storage round-trip assertion the in-memory cases cannot make. TestMetaFieldsModeE2E 27/27, TestHoodieTableConfig + resolution 107/107.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.