Skip to content

Commit b159bde

Browse files
committed
test(query): cover the batch path, note the version skew
Every existing test called the single-cell helper, so the production path — encoders built once per column per batch, row assembly, and the column naming a failure carries — was exercised only by hand against a live service, which CI does not do. A break in it would have shipped. Adds a test that drives arrow_result_to_query_response over a real two-row batch with a nested column, asserting the JSON shape through the real path rather than the helper. Verified to fail on a mutated loop. Also records a caveat next to the parity claim: this is the same encoder as the service's, not the same build of it. The service runs a later arrow than the SDK this links against, so the two agree because they are observed to, not because a version pins them. Aligning them needs an SDK release first.
1 parent ec650aa commit b159bde

1 file changed

Lines changed: 66 additions & 0 deletions

File tree

‎src/commands/query.rs‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,11 @@ fn value_to_string(v: &Value) -> String {
122122
/// rather than dropping `b`). It has to agree with the service or the same
123123
/// query would describe a struct differently depending on which side rendered
124124
/// it — the divergence this module exists to remove.
125+
///
126+
/// Caveat worth knowing: this is the same *encoder* as the service's, not the
127+
/// same *build* of it. The service is on a later arrow than the SDK this links
128+
/// against, so the two agree because they are observed to, not because a
129+
/// version pins them together. Aligning them needs an SDK release first.
125130
static ENCODER_OPTIONS: LazyLock<EncoderOptions> =
126131
LazyLock::new(|| EncoderOptions::default().with_explicit_nulls(true));
127132

@@ -751,6 +756,67 @@ mod tests {
751756
);
752757
}
753758

759+
/// The production path — `arrow_result_to_query_response` — rather than the
760+
/// single-cell helper the other tests use.
761+
///
762+
/// It owns work the helper does not: encoders built once per column per
763+
/// batch, row assembly, and the column naming carried by a failure. Those
764+
/// were previously exercised only by hand against a live service, which CI
765+
/// does not do, so a break in them would have shipped.
766+
#[test]
767+
fn the_batch_path_renders_a_whole_result() {
768+
use arrow::array::TimestampMicrosecondArray;
769+
use arrow::array::{ArrayRef, Int64Array, Int64Builder, ListBuilder, RecordBatch};
770+
use arrow::datatypes::{DataType, Field, Schema};
771+
772+
// Two rows, and a nested column so shape is actually asserted.
773+
let mut lb = ListBuilder::new(Int64Builder::new());
774+
lb.values().append_value(7);
775+
lb.values().append_value(8);
776+
lb.append(true);
777+
lb.values().append_value(9);
778+
lb.append(true);
779+
let list: ArrayRef = Arc::new(lb.finish());
780+
781+
let micros = 1_767_268_800_000_000i64;
782+
let ts: ArrayRef = Arc::new(
783+
TimestampMicrosecondArray::from(vec![micros, micros]).with_timezone("Asia/Kolkata"),
784+
);
785+
let ints: ArrayRef = Arc::new(Int64Array::from(vec![1, 2]));
786+
787+
let schema = Arc::new(Schema::new(vec![
788+
Field::new("n", DataType::Int64, false),
789+
Field::new("l", list.data_type().clone(), true),
790+
Field::new("t", ts.data_type().clone(), true),
791+
]));
792+
let batch = RecordBatch::try_new(schema.clone(), vec![ints, list, ts]).expect("batch");
793+
794+
let result = hotdata::ArrowResult {
795+
batches: vec![batch],
796+
schema,
797+
total_row_count: Some(2),
798+
next_link: None,
799+
};
800+
let resp = arrow_result_to_query_response(result, "rslt_test".to_string())
801+
.expect("the batch path renders");
802+
803+
assert_eq!(resp.columns, vec!["n", "l", "t"]);
804+
assert_eq!(resp.row_count, 2);
805+
assert_eq!(resp.total_row_count, Some(2));
806+
assert_eq!(resp.result_id.as_deref(), Some("rslt_test"));
807+
assert_eq!(resp.rows.len(), 2);
808+
809+
// The nested column must be a JSON array, not display text — the
810+
// invariant that failed before, asserted through the real path.
811+
assert_eq!(resp.rows[0][0], serde_json::json!(1));
812+
assert_eq!(resp.rows[0][1], serde_json::json!([7, 8]));
813+
assert_eq!(resp.rows[1][1], serde_json::json!([9]));
814+
815+
// And the named zone resolves rather than nulling.
816+
let t = resp.rows[0][2].as_str().expect("timestamp is a string");
817+
assert!(t.contains("+05:30"), "expected a resolved offset, got {t}");
818+
}
819+
754820
/// A timestamp carrying a *named* IANA zone must render, not come back as
755821
/// null. Rendering it needs a timezone database compiled in; without one
756822
/// Arrow's formatter errors, and this cell used to swallow that error and

0 commit comments

Comments
 (0)