duckdb

duckdb copied to clipboard

This PR reworks the way indexes are bound.

We now lazily bind all indexes (even the built in ART index type) when first accessed instead of initializing/loading them when loading a database. A key change is that the index class itself is now split into UnboundIndex (replacing the UnknownIndex class) and BoundIndex (functioning as the parent class/interface for all concrete instantiated index types) child classes. This unifies the handling of the ART index and custom extension indexes.

Additionally, this PR also adds a no_extension_load optional parameter to the sqllogic unittesters restart command, which prevents previously require'd extensions from being loaded after a restart within a test, making it easier to write tests for extension indexes that verify that a database with a custom index type can be initialized. I have some tests for this in vss that I will push soon, but they're not included in this PR since (afaik) I can't add new tests (or new files in general) through patches.

Also changes the vss extension config to not link so that we actually test loading the extension index properly.

Closes https://github.com/duckdblabs/duckdb-internal/issues/1892

Ive adapted to your feedback and done some more refactoring moving members between the index/unboundindex/boundindex types. The UnboundIndex now takes ownership of the entire CreateInfo and I've added abstract getters to the index class so that it doesn't have to fields such as name or constraint type separately when unbound.

Could you have a look at the Force Restart CI? It looks like there might be a hang/deadlock there

I can't reproduce it locally, but I did find get another hang in another alter test. The issue is that in CatalogSet::AlterEntry we lock the catalog, but later when binding the index we need to be able to access it so that we can add the base table to the bind context...

It passed my local CI now after merging with main and getting Marks concurrency patches, lets see if it passes here too.

Thanks for the changes! LGTM