Skip to content

Commit d7c9765

Browse files
committed
[Analytics Backend] UInt64 β†’ Int64 bridge at partition-stream boundary
Resolves the 13 by-clause failures in CalciteTimechartCommandIT (5/18 β†’ 18/18) caused by DataFusion's row_number() physical op emitting UInt64 while Calcite types ROW_NUMBER OVER as BIGINT (Int64 signed). Two coupled changes mirror the existing schema_coerce::coerce_inferred_schema bridge the parquet-scan path uses, applied at the partition-stream boundary where the analytics-engine reduce stage reads from upstream PARTIAL fragments via NativeBridge.registerPartitionStream. 1. api::register_partition_stream β€” coerce the producer's physical schema via schema_coerce before registering the StreamingTable and before writing the IPC bytes back to Java. The Substrait consumer plan's `ensure_schema_compatibility` (datafusion-substrait 53) strict-checks each ReadRel's base_schema against the registered table; the coercion makes Int64-declared plans bind against this table. The Java-side typesMatch tripwire then sees the coerced schema and validates incoming batches against it (next change). 2. api::sender_send β€” coerce incoming RecordBatches to match the sender's (now-coerced) schema via arrow::compute::cast where types differ. Without this, DataFusion's HashJoin / RepartitionExec panics with "primitive array" on as_primitive::<Int64Type>() of a UInt64 column when the producer's actual batch reaches downstream operators. cast() is the existing Arrow kernel β€” no-op when the schemas already match, single-cast on the mismatched columns otherwise. Same width preserves zero-copy semantics modulo the bit-reinterpret arrow handles internally. 3. DatafusionReduceSink.typesMatch β€” relax Java's tripwire to treat same-width Int (any signedness) as compatible, mirroring the existing Timestamp(any precision) tolerance. Same rationale: divergence is harmless because the sender_send coercion produces a downstream- compatible batch. Pass-rate impact: - CalciteTimechartCommandIT: 5/18 β†’ 18/18 βœ… (+13) - CalciteTimechartPerFunctionIT: 1/12 β†’ 1/12 (residuals are opensearch-project#21584's multi-unit-SPAN gap + variable-month TIMESTAMPADD) - Combined timechart: 6/30 β†’ 19/30 (63%) Signed-off-by: Kai Huang <huangkaics@gmail.com>
1 parent ad1cc9b commit d7c9765

2 files changed

Lines changed: 60 additions & 0 deletions

File tree

β€Žsandbox/plugins/analytics-backend-datafusion/rust/src/api.rsβ€Ž

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -793,6 +793,14 @@ pub unsafe fn register_partition_stream(
793793
) -> Result<(i64, Vec<u8>), DataFusionError> {
794794
let session = &mut *(session_ptr as *mut LocalSession);
795795
let schema = derive_schema_from_partial_plan(partial_plan_bytes)?;
796+
// Substrait-narrowing parity with the scan boundary (see schema_coerce). The producer's
797+
// physical output schema may include Arrow-only types Substrait can't bind against β€”
798+
// notably UInt64 from a DataFusion `row_number()` physical op (Calcite types ROW_NUMBER
799+
// OVER as BIGINT / Int64, isthmus serializes accordingly, then `from_substrait_plan` of
800+
// the consumer plan trips `ensure_schema_compatibility` against this registered table).
801+
// Coerce up-front so the consumer plan binds; the channel batches still arrive with the
802+
// producer's native types and get cast to the declared schema by the receiver adapter.
803+
let schema = crate::schema_coerce::coerce_inferred_schema(schema);
796804
let schema_ipc = schema_to_ipc_bytes(schema.as_ref())?;
797805
let sender = session.register_partition(input_id, schema)?;
798806
Ok((Box::into_raw(Box::new(sender)) as i64, schema_ipc))
@@ -934,9 +942,52 @@ pub unsafe fn sender_send(
934942
let struct_array = StructArray::from(array_data);
935943
let batch = RecordBatch::from(struct_array);
936944

945+
// Mirror the schema_coerce step applied at registration (see register_partition_stream):
946+
// when the producer's physical type differs from the consumer-declared type by an
947+
// Arrow-only divergence (UInt64 ↔ Int64, Float16 β†’ Float32, BinaryView ↔ Binary), the
948+
// table is registered with the consumer-declared type and the StreamingTable / downstream
949+
// operators downcast strictly to that type. The batch as received here still carries the
950+
// producer's original type, so cast each mismatched column to the sender's schema before
951+
// pushing into the channel.
952+
let batch = coerce_batch_to_sender_schema(batch, sender.schema())?;
953+
937954
sender.send_blocking(Ok(batch), io_handle)
938955
}
939956

957+
/// Cast batch columns to match the sender's declared schema where they differ. No-op when
958+
/// schemas already match (the common case). Returns an error if a cast is not representable
959+
/// via Arrow's `cast` kernel β€” callers should not register a sender whose schema is not
960+
/// reachable from the batch's via cast (the registration-time `schema_coerce` only narrows
961+
/// types the cast kernel handles).
962+
fn coerce_batch_to_sender_schema(
963+
batch: RecordBatch,
964+
target_schema: &arrow::datatypes::SchemaRef,
965+
) -> Result<RecordBatch, DataFusionError> {
966+
if batch.schema().as_ref() == target_schema.as_ref() {
967+
return Ok(batch);
968+
}
969+
let mut columns: Vec<arrow::array::ArrayRef> = Vec::with_capacity(batch.num_columns());
970+
for (i, target_field) in target_schema.fields().iter().enumerate() {
971+
let src = batch.column(i);
972+
if src.data_type() == target_field.data_type() {
973+
columns.push(src.clone());
974+
} else {
975+
columns.push(arrow::compute::cast(src, target_field.data_type()).map_err(|e| {
976+
DataFusionError::Execution(format!(
977+
"Failed to cast partition stream column '{}' from {:?} to {:?}: {}",
978+
target_field.name(),
979+
src.data_type(),
980+
target_field.data_type(),
981+
e
982+
))
983+
})?);
984+
}
985+
}
986+
RecordBatch::try_new(Arc::clone(target_schema), columns).map_err(|e| {
987+
DataFusionError::Execution(format!("Failed to assemble coerced partition stream batch: {}", e))
988+
})
989+
}
990+
940991
/// Closes a partition stream sender. Dropping the sender closes the mpsc,
941992
/// which the receiver side (DataFusion's streaming table) interprets as
942993
/// end-of-input.

β€Žsandbox/plugins/analytics-backend-datafusion/src/main/java/org/opensearch/be/datafusion/DatafusionReduceSink.javaβ€Ž

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,12 @@ private void feedToSender(DatafusionPartitionSender sender, VectorSchemaRoot bat
234234
* parameters are tolerated because the data-node parquet reader and physical
235235
* planner pick a precision the Java-side declaration does not predict, and the
236236
* chosen precision round-trips through Arrow C Data β€” divergence is harmless.
237+
* Same-width Int signedness divergence is tolerated for the same reason as the
238+
* Rust-side {@code schema_coerce::coerce_inferred_schema(UInt64 β†’ Int64)} bridge:
239+
* DataFusion's {@code row_number()} physical op emits UInt64, while Calcite types
240+
* {@code ROW_NUMBER OVER} as BIGINT (Int64 signed). The Substrait check is satisfied
241+
* on the registration side by the coercer; this tripwire mirrors the relaxation so
242+
* the actual UInt64 batches from the producer pass through.
237243
*/
238244
private static boolean typesMatch(Schema actual, Schema declared) {
239245
List<Field> a = actual.getFields();
@@ -247,6 +253,9 @@ private static boolean typesMatch(Schema actual, Schema declared) {
247253
if (at.getTypeID() == ArrowType.ArrowTypeID.Timestamp && dt.getTypeID() == ArrowType.ArrowTypeID.Timestamp) {
248254
continue;
249255
}
256+
if (at instanceof ArrowType.Int ai && dt instanceof ArrowType.Int di && ai.getBitWidth() == di.getBitWidth()) {
257+
continue;
258+
}
250259
if (!at.equals(dt)) {
251260
return false;
252261
}

0 commit comments

Comments
Β (0)