mirror of
https://github.com/lancedb/lancedb.git
synced 2026-06-04 12:50:40 +00:00
fix: validate branch inputs (empty names, negative versions)
This commit is contained in:
@@ -133,6 +133,16 @@ describe.each([arrow15, arrow16, arrow17, arrow18])(
|
||||
expect(await (await db.openTable("some_table")).countRows()).toBe(1);
|
||||
});
|
||||
|
||||
it("rejects invalid branch inputs", async () => {
|
||||
const branches = await table.branches();
|
||||
await expect(branches.create("")).rejects.toThrow("non-empty");
|
||||
await expect(branches.checkout("")).rejects.toThrow("non-empty");
|
||||
await expect(branches.delete("")).rejects.toThrow("non-empty");
|
||||
await expect(branches.create("bad", "main", -1)).rejects.toThrow(
|
||||
"non-negative",
|
||||
);
|
||||
});
|
||||
|
||||
it("should show table stats", async () => {
|
||||
await table.add([{ id: 1 }, { id: 2 }]);
|
||||
await table.add([{ id: 1 }]);
|
||||
|
||||
@@ -1179,7 +1179,16 @@ impl Branches {
|
||||
// "main" and None are two spellings of the root branch; normalize so
|
||||
// from_ref = "main" behaves identically to the default.
|
||||
let from_ref = from_ref.filter(|b| b != "main");
|
||||
let from = Ref::Version(from_ref, from_version.map(|v| v as u64));
|
||||
// Reject a negative version up front; `as u64` would silently wrap it
|
||||
// into a huge version number.
|
||||
let from_version = from_version
|
||||
.map(|v| {
|
||||
u64::try_from(v).map_err(|_| {
|
||||
napi::Error::from_reason("from_version must be a non-negative integer")
|
||||
})
|
||||
})
|
||||
.transpose()?;
|
||||
let from = Ref::Version(from_ref, from_version);
|
||||
let table = self
|
||||
.inner
|
||||
.create_branch(&name, from)
|
||||
|
||||
@@ -942,6 +942,18 @@ def test_branches(tmp_path):
|
||||
assert "exp" not in table.branches.list()
|
||||
|
||||
|
||||
def test_branch_name_validation(tmp_path):
|
||||
db = lancedb.connect(tmp_path)
|
||||
table = db.create_table("t", [{"id": 1}])
|
||||
|
||||
with pytest.raises(ValueError, match="non-empty"):
|
||||
table.branches.create("")
|
||||
with pytest.raises(ValueError, match="non-empty"):
|
||||
table.branches.checkout("")
|
||||
with pytest.raises(ValueError, match="non-empty"):
|
||||
table.branches.delete("")
|
||||
|
||||
|
||||
def test_branches_preserve_namespace(tmp_path):
|
||||
pytest.importorskip(
|
||||
"lance"
|
||||
|
||||
@@ -1930,6 +1930,15 @@ impl NativeTable {
|
||||
}
|
||||
}
|
||||
|
||||
fn validate_branch_name(name: &str, field: &str) -> Result<()> {
|
||||
if name.is_empty() {
|
||||
return Err(Error::InvalidInput {
|
||||
message: format!("{field} must be a non-empty string"),
|
||||
});
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Opens an existing Table using a namespace client.
|
||||
///
|
||||
/// This method uses `DatasetBuilder::from_namespace` to open the table, which
|
||||
@@ -2726,6 +2735,10 @@ impl BaseTable for NativeTable {
|
||||
name: &str,
|
||||
from: lance::dataset::refs::Ref,
|
||||
) -> Result<Arc<dyn BaseTable>> {
|
||||
Self::validate_branch_name(name, "branch name")?;
|
||||
if let lance::dataset::refs::Ref::Version(Some(from_branch), _) = &from {
|
||||
Self::validate_branch_name(from_branch, "from_ref")?;
|
||||
}
|
||||
let mut ds = (*self.dataset.get().await?).clone();
|
||||
let branch_ds = ds.create_branch(name, from, None).await?;
|
||||
let dataset = dataset::DatasetConsistencyWrapper::new_latest(
|
||||
@@ -2736,6 +2749,7 @@ impl BaseTable for NativeTable {
|
||||
}
|
||||
|
||||
async fn checkout_branch(&self, name: &str) -> Result<Arc<dyn BaseTable>> {
|
||||
Self::validate_branch_name(name, "branch name")?;
|
||||
let branch_ds = self.dataset.get().await?.checkout_branch(name).await?;
|
||||
let dataset = dataset::DatasetConsistencyWrapper::new_latest(
|
||||
branch_ds,
|
||||
@@ -2749,6 +2763,7 @@ impl BaseTable for NativeTable {
|
||||
}
|
||||
|
||||
async fn delete_branch(&self, name: &str) -> Result<()> {
|
||||
Self::validate_branch_name(name, "branch name")?;
|
||||
let mut ds = (*self.dataset.get().await?).clone();
|
||||
ds.delete_branch(name).await?;
|
||||
Ok(())
|
||||
@@ -3544,6 +3559,42 @@ mod tests {
|
||||
assert!(!branches.contains_key("exp"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_branch_name_validation() {
|
||||
let tmp_dir = tempdir().unwrap();
|
||||
let uri = tmp_dir.path().to_str().unwrap();
|
||||
let conn = ConnectBuilder::new(uri).execute().await.unwrap();
|
||||
let table = conn
|
||||
.create_table("my_table", some_sample_data())
|
||||
.execute()
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
// every entry point rejects an empty name instead of passing it down
|
||||
assert!(matches!(
|
||||
table.create_branch("", 1u64).await,
|
||||
Err(Error::InvalidInput { .. })
|
||||
));
|
||||
assert!(matches!(
|
||||
table.checkout_branch("").await,
|
||||
Err(Error::InvalidInput { .. })
|
||||
));
|
||||
assert!(matches!(
|
||||
table.delete_branch("").await,
|
||||
Err(Error::InvalidInput { .. })
|
||||
));
|
||||
// an empty source branch is rejected too
|
||||
assert!(matches!(
|
||||
table
|
||||
.create_branch(
|
||||
"ok",
|
||||
lance::dataset::refs::Ref::Version(Some(String::new()), None)
|
||||
)
|
||||
.await,
|
||||
Err(Error::InvalidInput { .. })
|
||||
));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_create_index() {
|
||||
use arrow_array::RecordBatch;
|
||||
|
||||
Reference in New Issue
Block a user