Skip to content

Commit 86325a6

Browse files
authored
fix(arrow-pg): emit NULL field for null struct values instead of skipping encoding (#299)
`encode_struct` returned `Ok(())` for null struct values without calling `encoder.encode_field()`. This skipped writing a NULL indicator to the pgwire DataRow, corrupting the column count and breaking client connections. The fix calls `encoder.encode_field(&None::<&[i8]>, ...)` for null structs, consistent with how List and Dictionary types already handle nulls in `encode_value`. Adds a regression test that verifies `encode_field` is called exactly once when encoding a null struct row.
1 parent 07b936d commit 86325a6

2 files changed

Lines changed: 54 additions & 1 deletion

File tree

arrow-pg/src/encoder.rs

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,7 @@ pub fn encode_value<T: Encoder>(
535535

536536
#[cfg(test)]
537537
mod tests {
538+
use arrow::buffer::NullBuffer;
538539
use bytes::BytesMut;
539540
use pgwire::{api::results::FieldFormat, types::format::FormatOptions};
540541
use postgres_types::Type;
@@ -589,6 +590,58 @@ mod tests {
589590
assert!(encoder.encoded_value == val);
590591
}
591592

593+
#[test]
594+
fn encode_struct_null_emits_field() {
595+
// Regression test: encode_struct must call encoder.encode_field for
596+
// NULL struct values so a NULL indicator is written to the DataRow.
597+
// Previously it returned Ok(()) without encoding, corrupting the
598+
// column count.
599+
600+
#[derive(Default)]
601+
struct CountingEncoder {
602+
call_count: usize,
603+
}
604+
605+
impl Encoder for CountingEncoder {
606+
type Item = ();
607+
608+
fn encode_field<T>(&mut self, _value: &T, _pg_field: &FieldInfo) -> PgWireResult<()>
609+
where
610+
T: ToSql + ToSqlText + Sized,
611+
{
612+
self.call_count += 1;
613+
Ok(())
614+
}
615+
616+
fn take_row(&mut self) -> Self::Item {}
617+
}
618+
619+
let fields = vec![
620+
Arc::new(Field::new("a", DataType::Utf8, true)),
621+
Arc::new(Field::new("b", DataType::Utf8, true)),
622+
];
623+
let a = Arc::new(StringArray::from(vec![Some("hello"), Some("x")])) as Arc<dyn Array>;
624+
let b = Arc::new(StringArray::from(vec![Some("world"), Some("y")])) as Arc<dyn Array>;
625+
626+
// Row 0: non-null struct, Row 1: null struct
627+
let null_buffer = NullBuffer::from(vec![true, false]);
628+
let struct_arr: Arc<dyn Array> = Arc::new(
629+
StructArray::try_new(fields.clone().into(), vec![a, b], Some(null_buffer)).unwrap(),
630+
);
631+
632+
let arrow_field = Field::new("s", DataType::Struct(fields.into()), true);
633+
let pg_field = FieldInfo::new("s".to_string(), None, None, Type::TEXT, FieldFormat::Text);
634+
635+
// Encode the NULL row (index 1).
636+
let mut encoder = CountingEncoder::default();
637+
let result = encode_value(&mut encoder, &struct_arr, 1, &arrow_field, &pg_field);
638+
assert!(result.is_ok());
639+
assert_eq!(
640+
encoder.call_count, 1,
641+
"encode_field must be called exactly once for a NULL struct to emit a NULL indicator"
642+
);
643+
}
644+
592645
#[test]
593646
fn test_get_time32_second_value() {
594647
let array = Time32SecondArray::from_iter_values([3723_i32]);

arrow-pg/src/struct_encoder.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ pub(crate) fn encode_struct<T: Encoder>(
122122
) -> PgWireResult<()> {
123123
let arr = arr.as_any().downcast_ref::<StructArray>().unwrap();
124124
if arr.is_null(idx) {
125-
return Ok(());
125+
return encoder.encode_field(&None::<&[i8]>, parent_pg_field_info);
126126
}
127127

128128
let fields = arrow_fields

0 commit comments

Comments
 (0)