Skip schema DDL on write-init when tables already exist#26
Conversation
SnowflakeLedgerBackend ran CREATE SCHEMA / CREATE TABLE / ALTER on every read_only=False init. Snowflake checks the CREATE/ALTER privilege even for `IF NOT EXISTS`, so a write-enabled deployment whose schema is provisioned out-of-band was forced to hold schema-ownership-level DDL grants just to write rows — otherwise startup failed (or silently fell back to an empty in-memory backend in wrappers that catch the error). Gate the DDL behind a SELECT-only INFORMATION_SCHEMA existence probe: when MODELS/SNAPSHOTS/TAGS (with MODELS.METADATA) already exist, skip the DDL so plain INSERT/UPDATE/SELECT grants suffice. Fresh deployments still auto-provision, and any introspection failure falls back to the CREATE path (no regression for permissioned deployments). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab5b539ef9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| info_schema = ( | ||
| f"{self._database}.INFORMATION_SCHEMA" if self._database else "INFORMATION_SCHEMA" | ||
| ) | ||
| schema_lit = _esc(self._schema_name.upper()) |
There was a problem hiding this comment.
Preserve quoted schema identifiers in the probe
When callers pass an already-quoted schema such as schema='"DB"."MODEL_LEDGER"', self._schema_name still includes the delimiter quotes, so this comparison looks for '"MODEL_LEDGER"' in INFORMATION_SCHEMA.TABLES. Snowflake stores identifiers in metadata without those quote delimiters, and quoted identifiers preserve exact case (Snowflake docs), so the probe reports that the existing tables are absent and falls back to CREATE SCHEMA IF NOT EXISTS, reintroducing the DDL privilege failure for externally provisioned least-privilege deployments that use quoted schema names. Normalize the schema name for metadata lookup by stripping quotes and only uppercasing unquoted identifiers.
Useful? React with 👍 / 👎.
The bulk write path (_flush_models_pandas / _flush_snapshots_pandas) creates a TEMPORARY staging table, which needs CREATE TABLE on the schema. A least-privilege writer with only INSERT/UPDATE/SELECT can't, so the first write would still fail even after the init-DDL fix. Catch the privilege error (SQLSTATE 42501) and fall back to the existing DDL-free SQL MERGE/INSERT path, which needs only INSERT/UPDATE/SELECT. Privileged deployments keep the fast bulk path unchanged. With this, the entire write path (init, models, snapshots, tags) requires no CREATE/ALTER — only INSERT/UPDATE/SELECT. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Release the unreleased delta since v0.7.4: - #26 — skip schema DDL on write-init when tables exist + SQL fallback for the bulk write path, so a least-privilege role (INSERT/UPDATE/SELECT) can write without schema-ownership DDL grants. - #23 — propagate connector-discovered status to the model row. Also relabels the stale top "Unreleased" CHANGELOG block as v0.7.4 (it shipped in the v0.7.4 release but was never relabeled). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Problem
SnowflakeLedgerBackendcalls_ensure_tables()on everyread_only=Falseinit, which issuesCREATE SCHEMA IF NOT EXISTS,CREATE TABLE IF NOT EXISTS(×3), andALTER TABLE ... ADD COLUMN.Snowflake evaluates the
CREATE/ALTERprivilege even when the object already exists —IF NOT EXISTSonly suppresses the already-exists error, not the authorization check. So a write-enabled deployment whose schema is provisioned out-of-band (IaC, migrations, a DBA) is forced to hold schema-ownership-level DDL grants (CREATE SCHEMAon the database +CREATE TABLEon the schema +ALTERon the table) just to write rows. A least-privilege service identity with onlyINSERT/UPDATE/SELECTcan't start: init raises (or, in wrappers that catch the error, silently degrades to an empty in-memory backend).Fix
Gate the DDL behind a
SELECT-onlyINFORMATION_SCHEMAexistence probe (_schema_objects_exist). WhenMODELS,SNAPSHOTS, andTAGSalready exist (withMODELS.METADATA),_ensure_tables()returns early and issues no DDL — the existingINSERT/UPDATE/SELECTgrants are sufficient.Behavior preserved:
CREATEpath runs).INFORMATION_SCHEMAaccess, transient error) falls back to theCREATEpath — no regression for permissioned deployments.MODELSmissingMETADATA) fall through so the backward-compatALTERstill runs._ensure_tables).Tests
5 new tests in
test_snowflake_ledger.py: skip-DDL-when-provisioned (with a session that fails any DDL), auto-provision-when-absent, fall-back-on-introspection-error, metadata-column-guard, and read-only-issues-nothing. Full suite: 785 passed / 24 skipped.ruff,ruff format,mypyclean.