Skip to content

Skip schema DDL on write-init when tables already exist#26

Merged
vigneshnarayanaswamy merged 2 commits into
mainfrom
vigneshn/skip-ddl-when-schema-exists
Jun 22, 2026
Merged

Skip schema DDL on write-init when tables already exist#26
vigneshnarayanaswamy merged 2 commits into
mainfrom
vigneshn/skip-ddl-when-schema-exists

Conversation

@vigneshnarayanaswamy

Copy link
Copy Markdown
Collaborator

Problem

SnowflakeLedgerBackend calls _ensure_tables() on every read_only=False init, which issues CREATE SCHEMA IF NOT EXISTS, CREATE TABLE IF NOT EXISTS (×3), and ALTER TABLE ... ADD COLUMN.

Snowflake evaluates the CREATE/ALTER privilege even when the object already existsIF NOT EXISTS only 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 SCHEMA on the database + CREATE TABLE on the schema + ALTER on the table) just to write rows. A least-privilege service identity with only INSERT/UPDATE/SELECT can'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-only INFORMATION_SCHEMA existence probe (_schema_objects_exist). When MODELS, SNAPSHOTS, and TAGS already exist (with MODELS.METADATA), _ensure_tables() returns early and issues no DDL — the existing INSERT/UPDATE/SELECT grants are sufficient.

Behavior preserved:

  • Fresh deployments still auto-provision (probe finds nothing → full CREATE path runs).
  • Introspection failure (no INFORMATION_SCHEMA access, transient error) falls back to the CREATE path — no regression for permissioned deployments.
  • Partial/old schemas (tables present but MODELS missing METADATA) fall through so the backward-compat ALTER still runs.
  • Read-only init is unchanged (never calls _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, mypy clean.

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>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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>
@vigneshnarayanaswamy vigneshnarayanaswamy merged commit 19a5539 into main Jun 22, 2026
10 checks passed
@vigneshnarayanaswamy vigneshnarayanaswamy deleted the vigneshn/skip-ddl-when-schema-exists branch June 22, 2026 21:43
vigneshnarayanaswamy added a commit that referenced this pull request Jun 23, 2026
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant