fix: refuse a CREATE on a name a table already holds - #479
Merged
Merged
Conversation
The startup stand-in for the default table blocked the name. CREATE TABLE and CREATE MATERIALIZED VIEW answered "already exists". The stand-in now yields to the first real table, and startup skips it when a loaded table holds the name. The stand-in also ignored sql.default_table and always registered "default", so BEACON_DEFAULT_TABLE=observations left that table missing.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #479 +/- ##
==========================================
+ Coverage 83.41% 83.44% +0.02%
==========================================
Files 374 375 +1
Lines 63237 63334 +97
==========================================
+ Hits 52749 52848 +99
+ Misses 10488 10486 -2
🚀 New features to boost your workflow:
|
The admin external-table endpoint builds the CREATE statement itself, so the stand-in has to yield there too.
The stand-in is an ordinary table again: a CREATE on its name fails, and you drop it first. Only the startup rule changes, so the stand-in takes the configured sql.default_table instead of the literal "default". The error names the stand-in, because a table nobody made is a confusing thing to collide with.
Both registered over whatever held the name, so a typo repointed a table or swapped a view with no warning, while CREATE TABLE refused the same name. The admin API documented the erroring behaviour it never had. IF NOT EXISTS and OR REPLACE now do the work the SQL reference already describes. REFRESH, ALTER TABLE and the crawler register directly and keep replacing on purpose.
This file contains hidden or 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
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.
Problem
Two defects, both about the name a table holds.
CREATE EXTERNAL TABLEandCREATE VIEWregistered over whatever held the name. A typo repointed a table or swapped a view with no warning.CREATE TABLEandCREATE MATERIALIZED VIEWrefused the same name, so one typo destroyed a table and another failed, by statement. The admin API documented the behavior it did not have: theif_not_existsfield ofPOST /api/admin/external-tablesskips "instead of erroring", and nothing errored.Beacon also registers an empty stand-in table at startup, so a JSON query without a
fromfield reports no missing table. The stand-in ignoredsql.default_tableand always took the literal namedefault. A server withBEACON_DEFAULT_TABLE=observationstherefore held a table calleddefaultand leftobservationsmissing.Change
Every
CREATErefuses a name that a table or a view holds. The modifiers the SQL reference already documents do the work:CREATE [EXTERNAL] TABLECREATE VIEW,CREATE MATERIALIZED VIEWCREATE EXTERNAL TABLE IF NOT EXISTSCREATE OR REPLACE EXTERNAL TABLE,CREATE OR REPLACE VIEWThe paths that replace a provider on purpose register directly and do not change: a materialized-view
REFRESH, anALTER TABLE, and a crawler that re-registers a table it owns.Startup registers the stand-in under the configured name, and only when the name is free. Drop it to take the name:
That table then survives a restart. The error on the stand-in names it and tells you to drop it, because a table nobody made is a confusing thing to collide with.
Breaking
A script that repoints a table with a bare re-
CREATEneedsOR REPLACE, or aDROP TABLEin front.Tests
7 tests in
tests/default_table.rs, 5 intests/external_tables.rs, 3 unit tests, 1 HTTP test.