mirror of
https://github.com/lancedb/lancedb.git
synced 2026-09-04 12:38:38 +00:00
fix(remote): use create-only route for replace false indexes
This commit is contained in:
@@ -91,10 +91,6 @@ impl ServerVersion {
|
||||
pub fn support_fts_document_granularity(&self) -> bool {
|
||||
self.0 >= semver::Version::new(0, 6, 0)
|
||||
}
|
||||
|
||||
pub fn support_create_index_replace_false(&self) -> bool {
|
||||
self.0 >= semver::Version::new(0, 5, 1)
|
||||
}
|
||||
}
|
||||
|
||||
pub const OPT_REMOTE_PREFIX: &str = "remote_database_";
|
||||
|
||||
@@ -491,9 +491,14 @@ impl<S: HttpSend> std::fmt::Debug for RemoteTable<S> {
|
||||
impl<S: HttpSend> RemoteTable<S> {
|
||||
async fn submit_create_index(&self, mut index: IndexBuilder) -> Result<Option<String>> {
|
||||
self.check_mutable().await?;
|
||||
let route = if index.replace {
|
||||
"create_index"
|
||||
} else {
|
||||
"create_index_if_not_exists"
|
||||
};
|
||||
let request = self
|
||||
.client
|
||||
.post(&format!("/v1/table/{}/create_index/", self.identifier));
|
||||
.post(&format!("/v1/table/{}/{}/", self.identifier, route));
|
||||
|
||||
let column = match index.columns.len() {
|
||||
0 => {
|
||||
@@ -508,12 +513,6 @@ impl<S: HttpSend> RemoteTable<S> {
|
||||
});
|
||||
}
|
||||
};
|
||||
if !index.replace && !self.server_version.support_create_index_replace_false() {
|
||||
return Err(Error::NotSupported {
|
||||
message: "create-index replace=false requires remote server version 0.5.1 or later"
|
||||
.into(),
|
||||
});
|
||||
}
|
||||
if matches!(
|
||||
&index.index,
|
||||
Index::FTS(params) if params.get_document_granularity().is_list_element()
|
||||
@@ -6089,33 +6088,29 @@ mod tests {
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_create_index_forwards_replace_false() {
|
||||
let table = Table::new_with_handler_version(
|
||||
"my_table",
|
||||
semver::Version::new(0, 5, 1),
|
||||
move |request| {
|
||||
assert_eq!(request.method(), "POST");
|
||||
match request.url().path() {
|
||||
"/v1/table/my_table/describe/" => {
|
||||
let schema = Schema::new(vec![Field::new("a", DataType::Int32, false)]);
|
||||
http::Response::builder()
|
||||
.status(200)
|
||||
.body(describe_response(&schema))
|
||||
.unwrap()
|
||||
}
|
||||
"/v1/table/my_table/create_index/" => {
|
||||
let body = request.body().unwrap().as_bytes().unwrap();
|
||||
let body: serde_json::Value = serde_json::from_slice(body).unwrap();
|
||||
assert_eq!(body["replace"], json!(false));
|
||||
|
||||
http::Response::builder()
|
||||
.status(200)
|
||||
.body("{}".to_string())
|
||||
.unwrap()
|
||||
}
|
||||
path => panic!("Unexpected path: {}", path),
|
||||
let table = Table::new_with_handler("my_table", move |request| {
|
||||
assert_eq!(request.method(), "POST");
|
||||
match request.url().path() {
|
||||
"/v1/table/my_table/describe/" => {
|
||||
let schema = Schema::new(vec![Field::new("a", DataType::Int32, false)]);
|
||||
http::Response::builder()
|
||||
.status(200)
|
||||
.body(describe_response(&schema))
|
||||
.unwrap()
|
||||
}
|
||||
},
|
||||
);
|
||||
"/v1/table/my_table/create_index_if_not_exists/" => {
|
||||
let body = request.body().unwrap().as_bytes().unwrap();
|
||||
let body: serde_json::Value = serde_json::from_slice(body).unwrap();
|
||||
assert_eq!(body["replace"], json!(false));
|
||||
|
||||
http::Response::builder()
|
||||
.status(200)
|
||||
.body("{}".to_string())
|
||||
.unwrap()
|
||||
}
|
||||
path => panic!("Unexpected path: {}", path),
|
||||
}
|
||||
});
|
||||
|
||||
table
|
||||
.create_index(&["a"], Index::BTree(Default::default()))
|
||||
@@ -6126,23 +6121,43 @@ mod tests {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_create_index_replace_false_rejects_unsupported_server() {
|
||||
let table = Table::new_with_handler("my_table", |_| -> http::Response<String> {
|
||||
panic!("unsupported replace=false should be rejected before issuing remote requests")
|
||||
});
|
||||
async fn test_create_index_replace_false_does_not_use_legacy_route_after_backend_downgrade() {
|
||||
let legacy_create_request_sent = Arc::new(std::sync::atomic::AtomicBool::new(false));
|
||||
let sent = legacy_create_request_sent.clone();
|
||||
let table = Table::new_with_handler_version(
|
||||
"my_table",
|
||||
semver::Version::new(0, 5, 1),
|
||||
move |request| match request.url().path() {
|
||||
"/v1/table/my_table/describe/" => {
|
||||
let schema = Schema::new(vec![Field::new("a", DataType::Int32, false)]);
|
||||
http::Response::builder()
|
||||
.status(200)
|
||||
.body(describe_response(&schema))
|
||||
.unwrap()
|
||||
}
|
||||
"/v1/table/my_table/create_index/" => {
|
||||
sent.store(true, std::sync::atomic::Ordering::SeqCst);
|
||||
http::Response::builder()
|
||||
.status(200)
|
||||
.body("{}".to_string())
|
||||
.unwrap()
|
||||
}
|
||||
"/v1/table/my_table/create_index_if_not_exists/" => http::Response::builder()
|
||||
.status(404)
|
||||
.body("not found".to_string())
|
||||
.unwrap(),
|
||||
path => panic!("Unexpected path: {}", path),
|
||||
},
|
||||
);
|
||||
|
||||
let err = table
|
||||
let result = table
|
||||
.create_index(&["a"], Index::BTree(Default::default()))
|
||||
.replace(false)
|
||||
.execute()
|
||||
.await
|
||||
.unwrap_err();
|
||||
.await;
|
||||
|
||||
assert!(
|
||||
matches!(&err, Error::NotSupported { message }
|
||||
if message.contains("replace=false requires remote server version 0.5.1")),
|
||||
"got {err:?}"
|
||||
);
|
||||
assert!(result.is_err());
|
||||
assert!(!legacy_create_request_sent.load(std::sync::atomic::Ordering::SeqCst));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
Reference in New Issue
Block a user