test(spark): Add date logical type test to TestAvroConversionUtils#18584
test(spark): Add date logical type test to TestAvroConversionUtils#18584ashokkumar-allu wants to merge 2 commits intoapache:masterfrom
Conversation
Add a test case that verifies createConverterToRow correctly handles Avro's date logical type (int with logicalType=date), ensuring the conversion from epoch-days integer to java.sql.Date preserves the correct date value.
hudi-agent
left a comment
hudi-agent
left a comment
There was a problem hiding this comment.
🤖 This review was generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.
Thanks for the contribution! This PR adds a test for the Avro date logical type conversion in TestAvroConversionUtils. No correctness issues found. A few style/readability suggestions in the inline comments. Please take a look, and this should be ready for a Hudi committer or PMC member to take it from here. A couple of minor naming and clarity suggestions below.
cc @yihua
|
|
||
| val dateInputData = Seq(7, 365, 0) | ||
| val schema = HoodieSchema.parse(dateSchema) | ||
| val convertor = AvroConversionUtils.createConverterToRow(schema, HoodieSchemaConversionUtils.convertHoodieSchemaToStructType(schema)) |
There was a problem hiding this comment.
🤖 nit: convertor looks like a typo — the function being called is createConverterToRow, so converter would be consistent with the rest of the codebase.
- Generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.
| } | ||
| """ | ||
|
|
||
| val dateInputData = Seq(7, 365, 0) |
There was a problem hiding this comment.
🤖 nit: the values 7, 365, and 0 read as arbitrary magic numbers — could you add a brief inline comment (e.g. // one week, one year, epoch) so the intent is clear to future readers?
- Generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.
hudi-agent
left a comment
hudi-agent
left a comment
There was a problem hiding this comment.
🤖 This review was generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.
Thanks for addressing the feedback! Both prior nits were resolved — convertor was renamed to converter, and the inline comment // one week, one year, epoch was added to clarify the test values. No new issues flagged from this automated pass — a Hudi committer or PMC member can take it from here for a final review.
4 participants
Describe the issue this Pull Request addresses
TestAvroConversionUtilswas missing a test for Avro'sdatelogical type (anintfield withlogicalType=date). ThecreateConverterToRowutility handles this case by converting epoch-day integers tojava.sql.Date, but this conversion path had no dedicated test coverage.Summary and Changelog
"Logical type: date"test inTestAvroConversionUtilsthat verifiesAvroConversionUtils.createConverterToRowcorrectly converts an Avro record with an integer date field (epoch days) to ajava.sql.Datewith the expected calendar value.import java.time.LocalDatefor the assertion helper.Impact
None — test-only change, no production code modified.
Risk Level
none
Documentation Update
none
Contributor's checklist