fix: fix bug in index remapping when plan contained multiple rewrite groups #1415
Conversation
|
ACTION NEEDED Lance follows the Conventional Commits specification for release automation. The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification. For details on the error please inspect the "PR Title Check" action. |
python/python/tests/test_optimize.py
Outdated
| # be compacted with themselves since they have many deleted rows. | ||
| # | ||
| # However, they should not be compacted together because one of | ||
| # them is indexed and the other is not. |
There was a problem hiding this comment.
Curious what would that look llike if we have many indices on different columns created from many version
There was a problem hiding this comment.
I believe the planner will not combine two fragments unless they are covered by the same indices. So if you have:
Index A: [0, 1, 2, 3]
Index B: [1, 2, 3]
Index C: [3]
Then you will get: [0], [1, 2], [3]
westonpace
changed the title
python/python/tests/test_optimize.py
Outdated
| assert ds.has_index | ||
|
|
||
| tbl = create_table(min=0, max=1, nvec=200) | ||
| ds = lance.write_dataset(tbl, base_dir, mode="append") |
There was a problem hiding this comment.
Does it compact non-index files into one?
Shall we write new data twice to create two fragments that are not indexed, and verify that they are combined later?
There was a problem hiding this comment.
We can. There are other tests ensuring that two fragments get combined into one (without any indexing being involved). But it's simple enough to make this test more sophisticated. I'll put two fragments on both sides.
There was a problem hiding this comment.
I've updated the test.
When merge when CI passes |
A bug would occur when some of the rewrite groups were covered by the index and other rewrite groups were not.