fix: CREATE TABLE IF NOT EXISTS ignores its guard -- duplicate sqlite_master row + leaked page (#697) - #717
Open
dpsiderius wants to merge 2 commits into
Open
dpsiderius wants to merge 2 commits into
dpsiderius wants to merge 2 commits into
Conversation
…_master row + leaked page (#697) CREATE TABLE/INDEX/VIEW IF NOT EXISTS and DROP TABLE/INDEX IF EXISTS parsed their guard flag but compile_statement never consulted it: a second CREATE TABLE IF NOT EXISTS against an existing table appended a second sqlite_master row and allocated (then abandoned) a root page, which stock sqlite3 reports as "Page N: never used" -- corrupting a database this crate itself created on the very next startup, since "IF NOT EXISTS on catalog bootstrap" is the idiom's whole purpose. Also: a duplicate CREATE TABLE without the guard silently succeeded too, which it must not. compile_statement now checks schemas/views (or the index list, for CREATE/DROP INDEX) before emitting: a satisfied guard compiles to a new compile_noop() (Init -> Halt, no page allocated, no schema-cookie bump); an unsatisfied guard on a CREATE still errors, matching the oracle's wording ("table t already exists" / "index i already exists" / "view v already exists"); DROP ... IF EXISTS on a missing object is now the same no-op instead of propagating NoSuchTable/NoSuchIndex. Refs: 010/Req-8, #678, #695
7 tasks
PR #717's catalog existence checks named the kind of the *statement* (CREATE TABLE/VIEW/INDEX) rather than the kind of the *existing* object, so a cross-kind clash reported the wrong noun in both directions (tables and views share a namespace). It now names the existing object's kind, oracle-matched against 3.53.4 for all four table/view directions. Also fixes the previously-missing index-namespace checks: CREATE TABLE/VIEW against an existing index now reports "there is already an index named X", and CREATE INDEX against an existing table or view reports "there is already a table named X" (the oracle's wording even when the clash is with a view) — both measured to NOT be suppressed by IF NOT EXISTS, unlike the same-kind clash it does guard. spend: matched estimate (small oracle-diff/message fix)
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
1 participant
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.
Stacked on #687 (fix/687-emit-autoindex) — merge that first.
What
compile_statementnow checks the catalog before emitting for all threeCREATE ... IF NOT EXISTSforms and bothDROP ... IF EXISTSforms. Asatisfied guard compiles to a no-op (
Init -> Halt, no page allocation,no schema-cookie bump). Bonus fix found while proving this: an unguarded
duplicate
CREATE TABLEused to silently succeed (rc=0); it now failswith the oracle's wording (
table t already exists).Before/after (measured against the pinned oracle)
Before: running
CREATE TABLE IF NOT EXISTS t (...)a second timecreated a second
sqlite_masterrow for the same table and left aleaked page —
integrity_checkthen reports "Page N: never used" and"wrong # of entries in index" on the file. After: the second run is a
true no-op — rc 0, exactly one
sqlite_masterrow, no leaked page,integrity_checkok, prior data intact.DROP TABLE/INDEX IF EXISTSona missing object confirmed already-correct (no-op) alongside the fix.
Test plan
integrity_check;unguarded duplicate still errors like the oracle; guarded/unguarded
DROPmatrix.make test,make lint,make test-corpus,make check-mvl-limit,make check-mod-files,make assuranceall pass.Refs: 001/Req-*, #685, #687
Closes #697
spend: matched estimate