mikeboone Claude Opus 4.8 commited on
Commit
2cc63cd
·
1 Parent(s): 72b93af

guard: catch CREATE/INSERT column mismatches before and during Snowflake load

Browse files

The Gap e2e failure: FACT_CAMPAIGN_PERFORMANCE's row loader referenced
CHECKOUT_INITIATED, a column CREATE TABLE never declared -> Snowflake
"invalid identifier", ~10 minutes into the run, no indication of which
table/column. Root mechanism wasn't pinned with certainty (DDL and the
loader both derive columns from the same DatasetBundle object, so they
can't diverge in the code as read) — these guards catch the failure
class immediately and name the exact column, regardless of cause.

- validator.py: _check_schema_consistency() — in-memory, at
validate_bundle() time. Flags (a) two column names that collide after
SQL-identifier normalization (duplicate column in CREATE TABLE), and
(b) a generated row carrying a key no declared column covers (INSERT
would reference an identifier CREATE TABLE never defined). Runs before
any DDL is derived or Snowflake is touched.

- dataset_writer.py: _verify_table_columns() — right before each table's
INSERT, DESCRIBE TABLE and diff against the columns we're about to
insert. Catches the mismatch even if its cause is outside this code
(stale worker, schema predating this run, etc.) by checking reality
instead of assuming the DDL that ran matches the object being loaded.

Tests: 4 new validator cases (clean bundle, row-key mismatch, normalized
duplicate names, empty-table no-op) + 2 new dataset_writer cases (missing
column raises before any INSERT; DESCRIBE runs per non-empty table).
Extended test_dataset_writer's FakeCursor with DESCRIBE TABLE support.

Verified locally: new tests pass, full suite unchanged (2 pre-existing
unrelated failures confirmed present on HEAD before this change), and
the blueprint smoke script (real engine, no mocks) passes clean — no
false positives on real generated data.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

demoprep_app/dataset/validator.py CHANGED
@@ -63,6 +63,8 @@ def validate_bundle(blueprint: DemoBlueprint, bundle: DatasetBundle) -> Validati
63
  report = ValidationReport()
64
  tables = bundle.table_map()
65
 
 
 
66
  fact_rows_by_name: dict[str, list[dict[str, Any]]] = {}
67
  for fact in blueprint.facts:
68
  table = tables.get(fact.name)
@@ -89,6 +91,54 @@ def validate_bundle(blueprint: DemoBlueprint, bundle: DatasetBundle) -> Validati
89
  # ---------------------------------------------------------------------------
90
 
91
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
92
  def _check_integrity(blueprint: DemoBlueprint, fact, tables: dict, fact_rows: list[dict[str, Any]], report: ValidationReport) -> None:
93
  for dim in blueprint.dimensions:
94
  if dim.name not in fact.dimension_names:
 
63
  report = ValidationReport()
64
  tables = bundle.table_map()
65
 
66
+ _check_schema_consistency(bundle, report)
67
+
68
  fact_rows_by_name: dict[str, list[dict[str, Any]]] = {}
69
  for fact in blueprint.facts:
70
  table = tables.get(fact.name)
 
91
  # ---------------------------------------------------------------------------
92
 
93
 
94
+ def _check_schema_consistency(bundle: DatasetBundle, report: ValidationReport) -> None:
95
+ """Guard against CREATE TABLE / INSERT column mismatches before either is ever
96
+ sent to Snowflake. Without this, a mismatch surfaces ~10 minutes later as a
97
+ cryptic 'invalid identifier' SQL-compilation error deep inside an executemany —
98
+ this catches it here, in-memory, in milliseconds, and names the exact column.
99
+
100
+ Two failure modes guarded:
101
+ 1. Two distinct column names collide under SQL-identifier normalization
102
+ (e.g. 'Checkout-Initiated' and 'Checkout Initiated' both ->
103
+ CHECKOUT_INITIATED) — CREATE TABLE would get a duplicate column.
104
+ 2. A generated row carries a key that normalization doesn't map onto any
105
+ declared column — the INSERT would reference an identifier CREATE
106
+ TABLE never defined.
107
+
108
+ Both directions derive from the SAME table.columns/table.rows the DDL
109
+ compiler and the row loader each read independently, so if this check
110
+ passes, CREATE and INSERT are provably looking at the same column set.
111
+ """
112
+ from demoprep_app.ddl.from_dataset import safe_identifier
113
+
114
+ for table in bundle.tables:
115
+ seen: dict[str, str] = {}
116
+ for column in table.columns:
117
+ safe = safe_identifier(column.name)
118
+ if safe in seen and seen[safe] != column.name:
119
+ report.integrity_errors.append(
120
+ f"Table {table.name}: columns '{seen[safe]}' and '{column.name}' both "
121
+ f"normalize to SQL identifier '{safe}' — CREATE TABLE would get a "
122
+ "duplicate column."
123
+ )
124
+ continue
125
+ seen[safe] = column.name
126
+
127
+ if not table.rows:
128
+ continue
129
+ declared = set(seen.keys())
130
+ # A generation bug is systemic (every row has it the same way), so the
131
+ # first row is as informative as scanning all of them and far cheaper.
132
+ row_keys = {safe_identifier(k) for k in table.rows[0].keys()}
133
+ extra = row_keys - declared
134
+ if extra:
135
+ report.integrity_errors.append(
136
+ f"Table {table.name}: generated rows contain column(s) {sorted(extra)} "
137
+ "that CREATE TABLE would not declare for this table — INSERT would fail "
138
+ f"with 'invalid identifier'. Declared columns: {sorted(declared)}."
139
+ )
140
+
141
+
142
  def _check_integrity(blueprint: DemoBlueprint, fact, tables: dict, fact_rows: list[dict[str, Any]], report: ValidationReport) -> None:
143
  for dim in blueprint.dimensions:
144
  if dim.name not in fact.dimension_names:
demoprep_app/integrations/snowflake/dataset_writer.py CHANGED
@@ -29,6 +29,7 @@ def populate_dataset_bundle(
29
  quoted_columns = ", ".join(_quote_ident(column) for column in columns)
30
  placeholders = ", ".join(["%s"] * len(columns))
31
  values = [tuple(row.get(column) for column in columns) for row in table.rows]
 
32
  if progress_callback:
33
  progress_callback(f"Loading {table.name}: {len(values):,} rows")
34
  cursor.execute(f"TRUNCATE TABLE {_quote_ident(table.name)}")
@@ -50,6 +51,33 @@ def populate_dataset_bundle(
50
  cursor.close()
51
 
52
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
53
  def _quote_ident(identifier: str) -> str:
54
  # Normalize the SAME way the DDL compiler does before quoting, so the
55
  # INSERT targets the exact columns CREATE TABLE produced. A raw name like
 
29
  quoted_columns = ", ".join(_quote_ident(column) for column in columns)
30
  placeholders = ", ".join(["%s"] * len(columns))
31
  values = [tuple(row.get(column) for column in columns) for row in table.rows]
32
+ _verify_table_columns(cursor, table.name, columns)
33
  if progress_callback:
34
  progress_callback(f"Loading {table.name}: {len(values):,} rows")
35
  cursor.execute(f"TRUNCATE TABLE {_quote_ident(table.name)}")
 
51
  cursor.close()
52
 
53
 
54
+ def _verify_table_columns(cursor: Any, table_name: str, expected_columns: list[str]) -> None:
55
+ """Fail fast, with a clear diagnosis, if the table Snowflake actually has
56
+ doesn't carry every column we're about to INSERT into.
57
+
58
+ Without this, a mismatch (whatever its cause — a stale worker running old
59
+ DDL, a schema that predates this run, a generation bug validate_bundle
60
+ missed) surfaces as a bare 'invalid identifier' SQL-compilation error deep
61
+ inside executemany, with no indication of which table or column. This
62
+ names both, before a single row is sent, by checking reality (what
63
+ Snowflake actually created) against the dataset object's expectations —
64
+ the two are independently derived, so this is the one place they're
65
+ provably reconciled.
66
+ """
67
+ from demoprep_app.ddl.from_dataset import safe_identifier
68
+
69
+ cursor.execute(f"DESCRIBE TABLE {_quote_ident(table_name)}")
70
+ actual = {row[0].upper() for row in cursor.fetchall()}
71
+ expected = {safe_identifier(c) for c in expected_columns}
72
+ missing = expected - actual
73
+ if missing:
74
+ raise RuntimeError(
75
+ f"Table {table_name} is missing column(s) {sorted(missing)} that the generated "
76
+ "dataset expects to insert. CREATE TABLE and the row loader disagree on this "
77
+ f"table's schema. Actual columns in Snowflake: {sorted(actual)}."
78
+ )
79
+
80
+
81
  def _quote_ident(identifier: str) -> str:
82
  # Normalize the SAME way the DDL compiler does before quoting, so the
83
  # INSERT targets the exact columns CREATE TABLE produced. A raw name like
tests/test_dataset_writer.py CHANGED
@@ -6,14 +6,25 @@ from demoprep_app.scenario.contract import ScenarioContract
6
 
7
 
8
  class FakeCursor:
9
- def __init__(self, fail_on_executemany=False):
10
  self.fail_on_executemany = fail_on_executemany
 
 
 
 
11
  self.executed = []
12
  self.executemany_calls = []
13
  self.closed = False
 
14
 
15
  def execute(self, sql):
16
  self.executed.append(sql)
 
 
 
 
 
 
17
 
18
  def executemany(self, sql, values):
19
  if self.fail_on_executemany:
@@ -74,7 +85,7 @@ def _bundle():
74
 
75
 
76
  def test_populate_dataset_bundle_inserts_rows_and_returns_counts():
77
- cursor = FakeCursor()
78
  connection = FakeConnection(cursor)
79
  progress = []
80
 
@@ -99,7 +110,10 @@ def test_populate_dataset_bundle_inserts_rows_and_returns_counts():
99
 
100
 
101
  def test_populate_dataset_bundle_rolls_back_and_closes_cursor_on_error():
102
- cursor = FakeCursor(fail_on_executemany=True)
 
 
 
103
  connection = FakeConnection(cursor)
104
 
105
  with pytest.raises(RuntimeError, match="insert failed"):
@@ -108,3 +122,31 @@ def test_populate_dataset_bundle_rolls_back_and_closes_cursor_on_error():
108
  assert connection.committed is False
109
  assert connection.rolled_back is True
110
  assert cursor.closed is True
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
6
 
7
 
8
  class FakeCursor:
9
+ def __init__(self, fail_on_executemany=False, describe_columns=None):
10
  self.fail_on_executemany = fail_on_executemany
11
+ # table_name -> list of column names DESCRIBE TABLE should report for it.
12
+ # Tests that don't care about the column-verification guard can omit this
13
+ # and get an empty result, which is why they must opt in to match reality.
14
+ self.describe_columns = describe_columns or {}
15
  self.executed = []
16
  self.executemany_calls = []
17
  self.closed = False
18
+ self._last_describe_table = None
19
 
20
  def execute(self, sql):
21
  self.executed.append(sql)
22
+ if sql.upper().startswith("DESCRIBE TABLE"):
23
+ self._last_describe_table = sql.split('"')[1]
24
+
25
+ def fetchall(self):
26
+ cols = self.describe_columns.get(self._last_describe_table, [])
27
+ return [(c,) for c in cols]
28
 
29
  def executemany(self, sql, values):
30
  if self.fail_on_executemany:
 
85
 
86
 
87
  def test_populate_dataset_bundle_inserts_rows_and_returns_counts():
88
+ cursor = FakeCursor(describe_columns={"ACCOUNTS": ["ACCOUNT_ID", "ACCOUNT_NAME"]})
89
  connection = FakeConnection(cursor)
90
  progress = []
91
 
 
110
 
111
 
112
  def test_populate_dataset_bundle_rolls_back_and_closes_cursor_on_error():
113
+ cursor = FakeCursor(
114
+ fail_on_executemany=True,
115
+ describe_columns={"ACCOUNTS": ["ACCOUNT_ID", "ACCOUNT_NAME"]},
116
+ )
117
  connection = FakeConnection(cursor)
118
 
119
  with pytest.raises(RuntimeError, match="insert failed"):
 
122
  assert connection.committed is False
123
  assert connection.rolled_back is True
124
  assert cursor.closed is True
125
+
126
+
127
+ def test_populate_dataset_bundle_raises_clear_error_when_column_missing():
128
+ # Snowflake's table is missing ACCOUNT_NAME — e.g. the CREATE TABLE that ran
129
+ # didn't match the dataset object being loaded. This must fail BEFORE any
130
+ # INSERT is attempted, naming the exact missing column, instead of letting
131
+ # Snowflake raise a bare "invalid identifier" deep inside executemany.
132
+ cursor = FakeCursor(describe_columns={"ACCOUNTS": ["ACCOUNT_ID"]})
133
+ connection = FakeConnection(cursor)
134
+
135
+ with pytest.raises(RuntimeError, match="ACCOUNTS.*ACCOUNT_NAME"):
136
+ populate_dataset_bundle(connection, "DEMO_SCHEMA", _bundle())
137
+
138
+ assert cursor.executemany_calls == []
139
+ assert connection.committed is False
140
+ assert connection.rolled_back is True
141
+ assert cursor.closed is True
142
+
143
+
144
+ def test_populate_dataset_bundle_verifies_columns_before_each_table_insert():
145
+ cursor = FakeCursor(describe_columns={"ACCOUNTS": ["ACCOUNT_ID", "ACCOUNT_NAME"]})
146
+ connection = FakeConnection(cursor)
147
+
148
+ populate_dataset_bundle(connection, "DEMO_SCHEMA", _bundle())
149
+
150
+ assert 'DESCRIBE TABLE "ACCOUNTS"' in cursor.executed
151
+ # EMPTY_DIM has no rows, so it's skipped entirely (no DESCRIBE, no INSERT).
152
+ assert 'DESCRIBE TABLE "EMPTY_DIM"' not in cursor.executed
tests/test_validator_schema_consistency.py ADDED
@@ -0,0 +1,127 @@
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
1
+ """Guard against CREATE TABLE / INSERT column mismatches — catch them in-memory,
2
+ at validate_bundle() time, instead of ~10 minutes later as a cryptic Snowflake
3
+ 'invalid identifier' error deep inside an executemany (see the Gap e2e failure:
4
+ FACT_CAMPAIGN_PERFORMANCE's row loader referenced a CHECKOUT_INITIATED column
5
+ CREATE TABLE never declared).
6
+ """
7
+
8
+ from __future__ import annotations
9
+
10
+ from demoprep_app.dataset.contracts import DatasetBundle, DatasetColumn, DatasetTable
11
+ from demoprep_app.dataset.validator import ValidationReport, _check_schema_consistency
12
+ from demoprep_app.scenario.contract import ScenarioContract
13
+
14
+
15
+ def _scenario() -> ScenarioContract:
16
+ return ScenarioContract(
17
+ company_name="Acme",
18
+ company_url="https://example.com",
19
+ use_case="Marketing Funnel",
20
+ scenario_type="campaign_performance",
21
+ fact_grain="campaign-day",
22
+ )
23
+
24
+
25
+ def test_schema_consistency_passes_for_a_clean_bundle():
26
+ bundle = DatasetBundle(
27
+ scenario=_scenario(),
28
+ tables=[
29
+ DatasetTable(
30
+ name="FACT_CAMPAIGN_PERFORMANCE",
31
+ grain="campaign-day",
32
+ columns=[
33
+ DatasetColumn("FACT_CAMPAIGN_PERFORMANCE_KEY", "fact_key", "NUMBER", nullable=False),
34
+ DatasetColumn("IMPRESSIONS", "impressions", "NUMBER", nullable=False),
35
+ DatasetColumn("CLICKS", "clicks", "NUMBER", nullable=False),
36
+ ],
37
+ rows=[{"FACT_CAMPAIGN_PERFORMANCE_KEY": 1, "IMPRESSIONS": 100, "CLICKS": 5}],
38
+ is_fact=True,
39
+ )
40
+ ],
41
+ )
42
+ report = ValidationReport()
43
+
44
+ _check_schema_consistency(bundle, report)
45
+
46
+ assert report.integrity_errors == []
47
+
48
+
49
+ def test_schema_consistency_catches_a_row_key_with_no_declared_column():
50
+ # Mirrors the Gap failure: the row loader would INSERT a column
51
+ # (CHECKOUT_INITIATED) that CREATE TABLE never declared for this table.
52
+ bundle = DatasetBundle(
53
+ scenario=_scenario(),
54
+ tables=[
55
+ DatasetTable(
56
+ name="FACT_CAMPAIGN_PERFORMANCE",
57
+ grain="campaign-day",
58
+ columns=[
59
+ DatasetColumn("FACT_CAMPAIGN_PERFORMANCE_KEY", "fact_key", "NUMBER", nullable=False),
60
+ DatasetColumn("ADD_TO_CART", "add_to_cart", "NUMBER", nullable=False),
61
+ DatasetColumn("CONVERSIONS", "conversions", "NUMBER", nullable=False),
62
+ ],
63
+ rows=[
64
+ {
65
+ "FACT_CAMPAIGN_PERFORMANCE_KEY": 1,
66
+ "ADD_TO_CART": 10,
67
+ "CONVERSIONS": 2,
68
+ "CHECKOUT_INITIATED": 6, # not a declared column
69
+ }
70
+ ],
71
+ is_fact=True,
72
+ )
73
+ ],
74
+ )
75
+ report = ValidationReport()
76
+
77
+ _check_schema_consistency(bundle, report)
78
+
79
+ assert len(report.integrity_errors) == 1
80
+ assert "FACT_CAMPAIGN_PERFORMANCE" in report.integrity_errors[0]
81
+ assert "CHECKOUT_INITIATED" in report.integrity_errors[0]
82
+
83
+
84
+ def test_schema_consistency_catches_column_names_that_collide_after_normalization():
85
+ # Two distinct Python-level names that normalize to the same SQL identifier
86
+ # would make CREATE TABLE emit a duplicate column.
87
+ bundle = DatasetBundle(
88
+ scenario=_scenario(),
89
+ tables=[
90
+ DatasetTable(
91
+ name="FACT_CAMPAIGN_PERFORMANCE",
92
+ grain="campaign-day",
93
+ columns=[
94
+ DatasetColumn("Checkout Initiated", "checkout_initiated", "NUMBER", nullable=False),
95
+ DatasetColumn("Checkout-Initiated", "checkout_initiated_2", "NUMBER", nullable=False),
96
+ ],
97
+ rows=[{"Checkout Initiated": 1, "Checkout-Initiated": 2}],
98
+ is_fact=True,
99
+ )
100
+ ],
101
+ )
102
+ report = ValidationReport()
103
+
104
+ _check_schema_consistency(bundle, report)
105
+
106
+ assert len(report.integrity_errors) == 1
107
+ assert "CHECKOUT_INITIATED" in report.integrity_errors[0]
108
+ assert "duplicate column" in report.integrity_errors[0]
109
+
110
+
111
+ def test_schema_consistency_ignores_empty_tables():
112
+ bundle = DatasetBundle(
113
+ scenario=_scenario(),
114
+ tables=[
115
+ DatasetTable(
116
+ name="EMPTY_DIM",
117
+ grain="empty",
118
+ columns=[DatasetColumn("EMPTY_DIM_KEY", "dimension_key", "NUMBER", nullable=False)],
119
+ rows=[],
120
+ )
121
+ ],
122
+ )
123
+ report = ValidationReport()
124
+
125
+ _check_schema_consistency(bundle, report)
126
+
127
+ assert report.integrity_errors == []