support append commit

[LiWeiJie]

[rust] allow append commit in dataset
[python] fix the missing mode in the dataset commit interface

[wjones127]

It seems like we need better validation when we making these commits. We can't have duplicate fragment ids, because that breaks the unique row ids.

[wjones127]

This will create duplicate paths in the current write append code path. The old fragments are already added here:

lance/rust/src/dataset.rs

Lines 355 to 358 in d3e8153

	let mut fragments: Vec<Fragment> = if matches!(params.mode, WriteMode::Append) {
	dataset
	.as_ref()
	.map_or(vec![], |d| d.manifest.fragments.as_ref().clone())

This is a breaking change in behavior of a public API, so I think I'd rather not make this change here if we can avoid it. But if we do make this change, we need to adjust the other code path as well, and add tests to make sure we aren't creating duplicate fragment entries.

[wjones127]

BTW, how are you generating fragment ids to make them unique? We might want to add validation to this function to make sure we aren't creating duplicate IDs.

[wjones127]

+1 to this change.

[wjones127]

I don't think this should be allowed because the added fragments have the same ids as the old ones.

[wjones127]

Perhaps what we want instead is a new struct to represent uncommitted fragments that doesn't have the ID

struct NewFragment {
    /// Files within the fragment.
    pub files: Vec<DataFile>,

    /// Optional file with deleted row ids.
    pub deletion_file: Option<DeletionFile>,
}

And then some function to append those to the log, adding new ids as appropriate:

impl Dataset {
    async fn append_new_fragments(dataset_uri: &str, new_fragments: &[NewFragment]) -> Result<Self> {
...}
}

That way we can handle the assignment of new fragment ids inside the function.

@eddyxu what do you think of that?

[wjones127]

@LiWeiJie closing this since we've implemented support in _commit for Append and several other operations in #1193

You can see an example of append here:

lance/python/python/tests/test_dataset.py

Lines 399 to 421 in 2ee7f01

	table = pa.Table.from_pydict({"a": range(100), "b": range(100)})
	base_dir = tmp_path / "test"
	lance.write_dataset(table, base_dir)
	fragment = lance.fragment.LanceFragment.create(base_dir, table)
	append = lance.LanceOperation.Append([fragment])
	with pytest.raises(OSError):
	# Must specify read version
	dataset = lance.LanceDataset._commit(base_dir, append)
	dataset = lance.LanceDataset._commit(base_dir, append, read_version=1)
	tbl = dataset.to_table()
	expected = pa.Table.from_pydict(
	{
	"a": list(range(100)) + list(range(100)),
	"b": list(range(100)) + list(range(100)),
	}
	)
	assert tbl == expected