voonhous commented on PR #19463: URL: https://github.com/apache/hudi/pull/19463#issuecomment-5468695634
Pushed f1eada0d0b74 to this branch rather than leaving the round-4 comments open, since all of it was small. What changed: - `TestHiveTableSchemaEvolution:163` ids `"6,7"` -> `",,6,7"`, the coverage gap above. It is the only test that reaches the new `cleanProjectionColumnIds` call ahead of `SchemaEvolutionContext`, and `setColumnNameList` had no coverage at all before it. - `TestSchemaEvolutionContext`: `",,"` -> `",0"` so the blank-id case can fail, plus a record-typed field so the padded-ids assertion pins the id-to-field pairing. - `TestHoodieRealtimeInputFormatUtils`: the six `cleanProjectionColumnIds` shapes are one `@ParameterizedTest` case each, so a regression in the first no longer hides the other five. - `getRequireColumn` reads both keys with an `""` default, the same missing-default bug you fixed one method below it (`Configuration.set(name, null)` throws). - Two doc corrections: `doEvolutionForRealtimeInputFormat` calls neither `setColumnNameList` nor `setColumnTypeList`, and the "realtime formats' own call becomes a no-op" comment had the direction backwards, since they clean in `addProjectionToJobConf` before delegating here. Only `getRequireColumn` changes production behaviour. Revert any of it if you would rather make the change yourself. Two things in the description are now stale and worth a pass when you next touch it: the `setColumnNameList` row in the consumer table still says `doEvolutionForRealtimeInputFormat`, and the `hudi-hadoop-mr` count in the Verification block is 205 rather than 200 after the parameterization. -- 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]
