hudi-agent commented on code in PR #669:
URL: https://github.com/apache/hudi-rs/pull/669#discussion_r3771653824
##########
crates/core/src/file_group/log_file/content.rs:
##########
@@ -39,13 +46,48 @@ use std::sync::Arc;
pub struct Decoder {
batch_size: usize,
hudi_configs: Arc<HudiConfigs>,
+ /// Predicate to push into a parquet log block, when the caller has decided
+ /// it is safe to evaluate before the merge. See
+ /// [`Decoder::with_row_filter`].
+ row_filter: Option<RowFilterBuilder>,
+ /// Schema an Avro block is resolved up to, as Avro JSON. See
Review Comment:
π€ nit: could you rename `required_schema_json` to `reader_schema_json`?
"Required" reads as mandatory/must-have, which clashes with the `Option`
wrapper; "reader" is also the Avro term of art and matches the
`reader_schema_json` parameter name already used in
`AvroBlockDecoder::try_new_with_reader`.
<sub><i>β οΈ AI-generated; verify before applying. React π/π to flag
quality.</i></sub>
##########
crates/core/src/file_group/log_file/content.rs:
##########
@@ -110,23 +152,48 @@ impl Decoder {
) -> Result<RecordBatches> {
Decoder::validate_log_block_version(&mut reader)?;
- let writer_schema =
header.get(&BlockMetadataKey::Schema).ok_or_else(|| {
+ let writer_schema_json =
header.get(&BlockMetadataKey::Schema).ok_or_else(|| {
CoreError::LogBlockError("Schema not found in block
header".to_string())
})?;
- let writer_schema = Arc::new(AvroSchema::parse_str(writer_schema)?);
let mut record_count_buf = [0u8; 4];
reader.read_exact(&mut record_count_buf)?;
let record_count = u32::from_be_bytes(record_count_buf);
- let record_content_reader =
- AvroDataBlockContentReader::new(reader, writer_schema.as_ref(),
record_count);
- let mut avro_arrow_array_reader =
- AvroArrowArrayReader::try_new(record_content_reader,
writer_schema.as_ref())?;
+ // A partial-update block carries only the columns that were written,
and
+ // the merge needs to know which those are. Resolving it up to the
table
+ // schema would fabricate the rest, so it decodes against its own
schema.
+ let is_partial = header.contains_key(&BlockMetadataKey::IsPartial);
Review Comment:
π€ This keys off the header's presence rather than its value β could a block
written with `IS_PARTIAL=false` be misread as partial here? Java's reader uses
`Boolean.parseBoolean(getOrDefault(IS_PARTIAL, "false"))`, so if the writer
ever emits `false`, a full block would decode writer-only and skip the
reader-schema resolution, reintroducing the intβlong mismatch at merge time on
an evolved table. Might be safer to parse the value to match Java.
<sub><i>β οΈ AI-generated; verify before applying. React π/π to flag
quality.</i></sub>
--
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]
