coord,sql: fix some table buglets #3992
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JLDLaughlin
approved these changes
Aug 20, 2020
| /// | ||
| /// Panics if `id` does not exist, or if `id` is not an object on which | ||
| /// indexes can be built. | ||
| pub fn default_index_for(&self, id: GlobalId) -> Option<GlobalId> { |
There was a problem hiding this comment.
This function is much clearer, thanks!
Is the selection of the default index something we plan on improving later on?
src/coord/src/coord.rs
Outdated
| @@ -2026,7 +2026,7 @@ where | |||
| return; | |||
| } | |||
| // A valid index is any index on `id` that is known to the dataflow | |||
| // layer, as indicated by its presence in `self.indexes`. | |||
| // layer, as indicated by its presence in `self.indexes`.s | |||
| @@ -976,7 +988,20 @@ impl Catalog { | |||
| }] | |||
| } | |||
| Op::DropItem(id) => { | |||
| if !self.get_by_id(&id).item().is_temporary() { | |||
| let entry = self.get_by_id(&id); | |||
| @@ -1806,6 +1806,16 @@ fn handle_insert( | |||
| match source { | |||
| InsertSource::Query(query) => { | |||
| let table = scx.catalog.get_item(&scx.resolve_item(table_name)?); | |||
| if table.id().is_system() { | |||
...without dropping the table too, at least. Unlike sources, where an unmaterialized source is reasonable, tables are implicitly always materialized, since Materialize is the source of truth. Fix #3905.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The first commit is a follow-up from #3982 which I forgot to push before merging. The second commit ensures that a table's default index is not dropped, which came up in #3982 and is tracked in MaterializeInc/database-issues#1213. The third commit is another table bug I discovered that I didn't bother filing an issue for.
I believe the only remaining issue before marking tables non-experimental is MaterializeInc/database-issues#1226.We're good to go after this, I believe!This change is