Skip to content

Commit 58d3404

Browse files
authored
[AURON #2320] Fix PB deserialization bug & improve PB parsing performance (#2339)
# Which issue does this PR close? Closes #2320 # Rationale for this change - Boolean type was not given a default value, causing incorrect data results. - Complex nested PB structures can encounter parsing errors. # What changes are included in this PR? - Fix boolean type was not given a default value, causing incorrect data results. - FIx complex nested PB structures can encounter parsing errors. - improve PB parsing performance # Are there any user-facing changes? - No # How was this patch tested? - No
1 parent 2889b56 commit 58d3404

2 files changed

Lines changed: 885 additions & 156 deletions

File tree

native-engine/datafusion-ext-plans/src/flink/serde/json_deserializer.rs

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1034,4 +1034,58 @@ mod tests {
10341034
// msg2 missing "name" field, should get default empty string from ensure_size
10351035
assert_eq!(name_col.value(1), "");
10361036
}
1037+
1038+
/// Pin the omitted-vs-null distinction for a Boolean field that this PR
1039+
/// introduced by switching the shared `ensure_output_array_builders_size`
1040+
/// boolean default from `append_null()` to `append_value(false)`.
1041+
///
1042+
/// After the change, the PB path correctly emits `false` for a proto3 field
1043+
/// absent from a message, but the shared default also reaches this JSON
1044+
/// path: an *omitted* boolean now yields a non-null `false`, while an
1045+
/// *explicit* JSON `null` still yields a null (the JSON handler appends
1046+
/// null for explicit nulls). This test locks that behavior so a future
1047+
/// change to either side is a conscious decision, not an accident.
1048+
#[test]
1049+
fn test_parse_json_boolean_omitted_vs_explicit_null() {
1050+
let schema = Arc::new(Schema::new(vec![
1051+
Field::new("serialized_kafka_records_partition", DataType::Int32, false),
1052+
Field::new("serialized_kafka_records_offset", DataType::Int64, false),
1053+
Field::new("serialized_kafka_records_timestamp", DataType::Int64, false),
1054+
Field::new("active", DataType::Boolean, true),
1055+
]));
1056+
1057+
let nested_mapping = HashMap::new();
1058+
let mut deserializer = JsonDeserializer::new(schema.clone(), &nested_mapping)
1059+
.expect("Failed to create JsonDeserializer");
1060+
1061+
// row0: explicit true; row1: explicit null; row2: field omitted entirely.
1062+
let msg0 = br#"{"active": true}"#;
1063+
let msg1 = br#"{"active": null}"#;
1064+
let msg2 = br#"{}"#;
1065+
1066+
let messages = create_binary_array(vec![msg0.as_ref(), msg1.as_ref(), msg2.as_ref()]);
1067+
let partitions = create_partition_array(vec![0, 0, 0]);
1068+
let offsets = create_offset_array(vec![100, 101, 102]);
1069+
let timestamps = create_timestamp_array(vec![1000, 1001, 1002]);
1070+
1071+
let batch = deserializer
1072+
.parse_messages_with_kafka_meta(&messages, &partitions, &offsets, &timestamps)
1073+
.expect("Failed to parse messages");
1074+
1075+
assert_eq!(batch.num_rows(), 3);
1076+
let active_col = batch
1077+
.column(3)
1078+
.as_any()
1079+
.downcast_ref::<BooleanArray>()
1080+
.expect("active column");
1081+
1082+
// row0: explicit true.
1083+
assert!(active_col.value(0));
1084+
assert!(!active_col.is_null(0));
1085+
// row1: explicit null → null (JSON handler's own behavior).
1086+
assert!(active_col.is_null(1));
1087+
// row2: omitted → non-null false (shared ensure_size default).
1088+
assert!(!active_col.is_null(2));
1089+
assert!(!active_col.value(2));
1090+
}
10371091
}

0 commit comments

Comments
 (0)