diff --git a/Cargo.lock b/Cargo.lock index 5d07285fc..6b15095d1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9524,6 +9524,7 @@ version = "1.0.151" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c841b55ecdae098c80dcae9cf767f6f8a0c2cdb3416bbef72181df4d0fe73f14" dependencies = [ + "indexmap 2.14.0", "itoa", "memchr", "serde", diff --git a/Cargo.toml b/Cargo.toml index eb691ac48..d7216fbaa 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -72,7 +72,7 @@ oauth2 = { version = "5.0", default-features = false } regex = "1.10" semver = "1.0.25" serde = "1" -serde_json = "1" +serde_json = { version = "1", features = ["preserve_order"] } tempfile = "3.5.0" tokio = { version = "1.23", features = ["rt-multi-thread", "sync"] } tonic = { version = "0.14", features = ["tls-native-roots", "tls-ring"] } diff --git a/rust/lancedb/src/remote/table.rs b/rust/lancedb/src/remote/table.rs index 6bbb49adf..1c8eff845 100644 --- a/rust/lancedb/src/remote/table.rs +++ b/rust/lancedb/src/remote/table.rs @@ -1046,6 +1046,8 @@ impl RemoteTable { ); } Select::Dynamic(pairs) => { + // Keep the caller's alias order in the JSON object sent to the server. + // serde_json's preserve_order feature is required here. let alias_map = serde_json::Map::from_iter(pairs.iter().map(|(name, expr)| { (name.clone(), serde_json::Value::String(expr.clone())) @@ -1387,24 +1389,29 @@ impl RemoteTable { }); let streams = futures::future::try_join_all(futures); - if let Some(timeout) = options.timeout { + let streams = if let Some(timeout) = options.timeout { let timeout_future = tokio::time::sleep(timeout); tokio::pin!(timeout_future); tokio::pin!(streams); tokio::select! { _ = &mut timeout_future => { - Err(Error::Other { + return Err(Error::Other { message: format!("Query timeout after {} ms", timeout.as_millis()), source: None, }) } result = &mut streams => { - Ok(result?) + result? } } } else { - Ok(streams.await?) - } + streams.await? + }; + + Ok(streams + .into_iter() + .map(|stream| restore_projection_order(stream, &query.base().select)) + .collect()) } fn prepare_query_bodies( @@ -1446,6 +1453,52 @@ impl RemoteTable { } } +fn restore_projection_order( + stream: Pin>, + select: &Select, +) -> Pin> { + let pairs = match select { + Select::Dynamic(pairs) => pairs.iter().map(|(name, _)| name).collect::>(), + Select::Expr(pairs) => pairs.iter().map(|(name, _)| name).collect::>(), + _ => return stream, + }; + + let schema = stream.schema(); + let mut remaining = (0..schema.fields().len()).collect::>(); + let mut indices = Vec::with_capacity(remaining.len()); + for name in pairs { + if let Some(position) = remaining + .iter() + .position(|&index| schema.field(index).name() == name) + { + indices.push(remaining.remove(position)); + } + } + indices.extend(remaining); + if indices + .iter() + .enumerate() + .all(|(index, &field)| index == field) + { + return stream; + } + + // Older servers may sort alias maps even when the request preserves their + // order. Reorder the schema and every batch while keeping server-added fields. + let fields = indices + .iter() + .map(|&index| schema.field(index).clone()) + .collect::>(); + let schema = Arc::new(arrow_schema::Schema::new_with_metadata( + fields, + schema.metadata().clone(), + )); + let batches = stream.map(move |batch| { + batch.and_then(|batch| batch.project(&indices).map_err(DataFusionError::from)) + }); + Box::pin(RecordBatchStreamAdapter::new(schema, batches)) +} + #[derive(Deserialize)] struct TableDescription { version: u64, @@ -5809,6 +5862,72 @@ mod tests { .unwrap(); } + #[rstest] + #[case::sql(Select::dynamic(&[("p", "x"), ("id", "id")]))] + #[case::typed(Select::expr_projection(&[ + ("p", crate::expr::col("x")), + ("id", crate::expr::col("id")), + ]))] + #[tokio::test] + async fn test_query_dynamic_projection_order(#[case] select: Select) { + let table = Table::new_with_handler("my_table", |request| { + assert_eq!(request.url().path(), "/v1/table/my_table/query/"); + let body: serde_json::Value = + serde_json::from_slice(request.body().unwrap().as_bytes().unwrap()).unwrap(); + let aliases = body["columns"] + .as_object() + .unwrap() + .keys() + .map(String::as_str) + .collect::>(); + assert_eq!(aliases, ["p", "id"]); + + // Simulate a server that returns aliases in alphabetical order. + let data = RecordBatch::try_new( + Arc::new(Schema::new(vec![ + Field::new("id", DataType::Int32, false), + Field::new("p", DataType::Int32, false), + ])), + vec![ + Arc::new(Int32Array::from(vec![7])), + Arc::new(Int32Array::from(vec![42])), + ], + ) + .unwrap(); + http::Response::builder() + .status(200) + .header(CONTENT_TYPE, ARROW_FILE_CONTENT_TYPE) + .body(write_ipc_file(&data)) + .unwrap() + }); + + let batches = table + .query() + .select(select) + .execute() + .await + .unwrap() + .try_collect::>() + .await + .unwrap(); + assert_eq!(batches.len(), 1); + let schema = batches[0].schema(); + assert_eq!( + schema + .fields() + .iter() + .map(|field| field.name().as_str()) + .collect::>(), + ["p", "id"] + ); + let projected = batches[0] + .column(0) + .as_any() + .downcast_ref::() + .unwrap(); + assert_eq!(projected.value(0), 42); + } + #[tokio::test] async fn test_query_vector_nested_field_path() { let expected_data = RecordBatch::try_new(