pg_lake_iceberg: add SameIcebergStoredRepresentation helper

[sfc-gh-dachristensen]

Problem

Deciding whether an ALTER COLUMN ... TYPE leaves an Iceberg table's stored schema unchanged means asking "do these two Postgres types get stored with the same Iceberg representation?" The transforms that shape how a column is physically stored already live in pg_lake — the create path's recursive unsupported-numeric-to-double conversion, and the compatibility-mode storage mapping — but there is no single entry point that reproduces them, so consumers reimplement the comparison (and can get it wrong: a top-level-only check treats numeric(50,2)[] as equal to text[] while the create path stores list<double> vs list<string>).

Solution

Add an exported SameIcebergStoredRepresentation(oldType, newType, context) to pg_lake_iceberg. It reproduces the full stored schema for each type and compares the derived Iceberg field trees for type equality (ignoring field ids and defaults, which are per-derivation):

• derive each type's field tree with a conversion-aware derivation (PostgresTypeToIcebergFieldConverted) that applies the create path's unsupported-numeric rule at every scalar leaf, recursively — so a nested numeric(50,2)[] becomes list<double>, not the string fallback;
• then apply the compatibility storage mapping over the derived tree (ApplyCompatibilityStorageMapping), so e.g. a nested uuid is stored as string under compatibility_mode='snowflake' while a top-level uuid stays native.

The create-path settings that shape storage — unsupported_numeric_as_double and compatibility_mode — are passed in via an IcebergCreatePathContext rather than read from live state, because the GUC is PGC_USERSET and the mode is per-table: the comparison must use the values in effect when the target table was created. With the GUC off, an unsupported numeric has no faithful stored form (CREATE rejects it at any level), so the leaf is reported unrepresentable and the comparison is conservatively false rather than treating numeric as equal to text.

It answers only the representation question; it does not decide whether a given type change is otherwise permitted (casts, USING clauses, engine policy) — that stays with the caller.

Test plan

• Adds a SQL-callable test wrapper pg_lake_same_iceberg_representation(old_type text, new_type text, unsupported_numeric_as_double bool, compatibility_mode text) and a pytest in pg_lake_iceberg/tests/pytests/, covering:
  • top-level same/different pairs: varchar length changes, varchar/char/text, smallint/integer, time/timetz, json/jsonb→text; int/bigint, numeric precision changes, timestamp/timestamptz, real/double;
  • nested/recursive cases: numeric(50,2)[] vs text[] (different — the false match this fixes), unsupported-numeric arrays vs a genuine double precision[] (same), and a composite with an unsupported-numeric field vs a float8 composite (same — exercises the struct recursion);
  • both GUC states: unsupported numeric → double at every level when on; unrepresentable (conservatively false) when off, with bounded numerics unaffected;
  • the depth-dependent snowflake compatibility uuid mapping: nested uuid ≈ text (both string), top-level uuid ≠ text.
• make check-indent clean (pgindent PG18 + black); builds with -Werror.

[sfc-gh-okalaci]

SameIcebergRepresentation doesn't reproduce the changelog storage shape, and one direction silently over-allows.

who actually calls this

• old = the live PG attribute type of the tracked column (att->atttypid / atttypmod)
• new = the ALTER target type

two facts matter:

1. the tracked table is a plain PG heap table, not an iceberg table, so old is the genuine surface type (numeric(50,2)[], uuid[], ...), not pre-rewritten to float8[].
2. the schema this must protect is the changelog table, which snowflake_cdc creates as a real pg_lake iceberg table with compatibility_mode='snowflake'. so the changelog goes through the full create path (recursive numeric->double AND compat), while this helper reproduces neither.

the two gaps land on opposite sides

return TRUE means "let the ALTER through, replicate nothing", so a wrong TRUE is a silent, permanent mirror divergence and a wrong FALSE is merely over-conservative.

(a) nested unsupported numeric, wrong TRUE, the dangerous one. NormalizeTypeForIcebergSchema only converts the top-level numeric; a nested one falls through to the string fallback. but the changelog create path converts nested numeric to double recursively (MaybeConvertUnsupportedNumericColumnsToDouble -> ConvertTypeTree/NumericLeafToDouble, the create_table.c comment literally says "including nested ones"). so the helper sees list<string> where the changelog stores list<double>. reachable:

-- tracked heap column; changelog stores list<double>
CREATE TABLE t (a numeric(50,2)[]);
-- ... add t to a snowflake_cdc publication ...

ALTER TABLE t ALTER COLUMN a TYPE text[];   -- succeeds WITHOUT a USING clause

same_iceberg_representation('numeric(50,2)[]','text[]') = t (both list<string> to the helper), so AlterColumnTypeIsMirrorNoop lets it through emitting nothing, but the changelog schema genuinely changes list<double> -> list<string> and the mirror is permanently wrong. i checked locally: numeric(50,2)[] -> text[] (and -> varchar[]) is assignment-castable with no USING, so the USING/COLLATE guard does not close this. the reverse text[]->numeric[] and composite->composite both need USING, so the hole is specifically array-of-nested-numeric -> string-family array.

(b) nested uuid under compat, over-restrictive today, but the helper is compat-blind by construction. the changelog stores nested uuid as string (compat), while this helper computes native iceberg uuid (PostgresTypeToIcebergField with no compat). so same_iceberg_representation('uuid[]','text[]') = f and the caller blocks a change that is actually a changelog no-op. for today's snowflake mode that is just too conservative, not a correctness bug, agreed. but i wouldn't file it under "harmless over-restriction". the real issue is structural: SameIcebergRepresentation never takes compatibility_mode as an input, so it is frozen to a single storage shaping. compatibility_mode is going to evolve, and the first mode that makes two different surface types share one storage type (exactly how nested uuid and text already both land on string) turns this same blindness into a wrong TRUE, i.e. it flips from the safe side to the silent-divergence side. so the ask is: make compat a parameter now, otherwise this contract quietly rots as modes change.

net: SameIcebergFieldType is fine, the inputs just aren't changelog-shaped. the header's "never produces a false match" line is itself the false statement, case (a) is exactly a false match.

fix

derive both sides through the same transforms the changelog create uses, not top-level-only + compat-less:

• recursive numeric via MaybeConvertType / ConvertTypeTree (already in rel_utils.c) instead of NormalizeTypeForIcebergSchema
• apply the snowflake compat storage mapping so nested uuid -> string

do that identically on old and new and both (a) and (b) collapse.

even simpler for #704: the changelog iceberg table already exists when the ALTER runs, so compare the new type's derived storage field against the existing stored field from field_id_mappings and drop the re-derived old entirely. that also survives unsupported_numeric_as_double GUC flips and historical divergence, which a pure re-derivation can't.

if recursive/compat is out of scope for this PR: at minimum fix the header (surface-only, can false-match), and tell the #704 side to block nested types (or gate them on a catalog read) so the over-allow can't ship.

[sfc-gh-okalaci]

can we add nested coverage here too? arrays/composites with unsupported numeric, and nested uuid under compatibility_mode='snowflake'. right now its only top-level scalars, so the one case that actually matters is untested: numeric(50,2)[] vs text[] returns t while the changelog stores list<double> vs list<string>. the nested uuid over-block is worth pinning too so a later change doesn't flip it into an over-allow.

minimum cases:

• numeric(50,2)[] vs double precision[] -> same as create path
• numeric(50,2)[] vs text[] -> different
• composite with unsupported numeric field vs rewritten float8 composite
• unbounded numeric[] vs float8[]
• nested uuid vs text under snowflake mode

[sfc-gh-dachristensen]

Added in 958a516: the pinned-GUC test now also checks numeric(50,2)[] and numeric[] against a genuine double precision[] (so it pins that the converted leaf really is double, not just equal to another numeric), and a new test_composite_unsupported_numeric_matches_float8_composite exercises the struct recursion — a composite with an unsupported-numeric field matches a float8 composite but differs from a text one. Combined with the existing nested numeric[] vs text[] and nested-uuid-under-snowflake cases, that covers all five you listed.

[sfc-gh-okalaci]

with the GUC off, create rejects unsupported numeric rather than storing string. PostgresBaseTypeIdToIcebergTypeName still falls back to string, so the helper can say numeric ~ text even though that type never lands in an iceberg table (same underlying "natural mapping != what create does" as the nested cases).

fix the comment to match create behavior (reject when GUC off). optional: when GUC is off, treat unsupported numeric as non-representable / return false against everything except itself-as-string if you want a pure mapping answer.

[sfc-gh-okalaci]

nit: maybe give this its own .c? iceberg_field.c is already the convert/ensure path, and this predicate will grow once nested normalize / compat are handled. own file like same_iceberg_representation.c is enough, no need to move it to engine.

[sfc-gh-dachristensen]

Reworked this to address the nested case. Summary of the change and the reasoning:

Root cause. The old helper normalized only the top-level type before deriving the Iceberg field, so a nested unsupported numeric fell through to the string fallback. That made numeric(50,2)[] compare equal to text[], while the create path actually stores list<double> vs list<string>. That is the false match you flagged, and it could let an ALTER through while silently diverging the stored schema.

Fix. Derivation is now conversion-aware. PostgresTypeToIcebergField gained an internal form that threads a leaf-conversion callback plus an IcebergTypePosition (depth and enclosing container) through the existing array/composite/map recursion, applying the conversion at each scalar leaf. This reproduces the create path recursively. It stays free of catalog side effects because it only builds in-memory Field structs, so it avoids the GetOrCreatePGMapType / composite-creation cost that reusing MaybeConvertType would incur. That cost was the reason the original stayed top-level only.

unsupported_numeric_as_double. The stock conversion captures the flag in a context instead of reading the live GUC. The GUC is PGC_USERSET, so its value at ALTER time can differ from the value in effect when the table was created. With the flag on, an unsupported numeric maps to double at every level. With it off, CREATE errors on such a numeric at any level, so it has no stored form and the leaf is reported unrepresentable, which makes the comparison conservatively false rather than treating numeric as equal to text.

The compatibility concern. You were right that this matters, and compatibility_mode already lives in pg_lake (compatibility_mode.c). So SameIcebergStoredRepresentation now reproduces the full stored schema: the numeric leaf conversion during derivation, then ApplyCompatibilityStorageMapping over the derived tree. That reuses the existing mapping instead of duplicating the uuid rule, and it is why the callback carries position. A nested uuid is stored as string under the snowflake mode while a top-level uuid stays native, so the comparison has to be depth-aware. Tests cover top-level vs nested for both GUC states and the snowflake uuid mapping.

The callback is also the extension point for a caller whose storage rules are not modeled in pg_lake: it can supply its own leaf conversion with the same position information rather than pushing that policy into this helper.

Also moved the whole thing into iceberg_representation.{c,h} as you suggested.

[snowflake-security-bot]

Snowflake Security Review

Security grade: A — Passed ✅

No security findings after adjudication. This PR passes the Snowflake Security Review.

📊 4 of 6 files (2 tests skipped) · 1,282 lines reviewed · 2 candidates → 1 kept · retrieval: on

[sfc-gh-dachristensen]

We don't want caller details here; it's irrelevant to this project.

[sfc-gh-dachristensen]

It's 2026

[sfc-gh-okalaci]

the derivation rework looks right, and building the field tree in-memory (no catalog side effects) is the correct call. one thing i want to push on before we bake it in: re-deriving both sides answers "would a fresh create of old and new match under today's settings", not "does new match what's actually stored". for a mirror those can drift apart.

concrete way it bites: unsupported_numeric_as_double is PGC_USERSET. if it was on when the changelog was created and off at ALTER time (or vice versa), re-deriving old under the current GUC disagrees with what's physically on disk. same if the derivation logic changes across a pg_lake release. in both cases we're comparing against a re-computation instead of ground truth.

what if we derive only the new type and compare it against the persisted field for that column from field_id_mappings? old side becomes ground truth, immune to GUC/version drift, and the decision sits on the #704 side where the changelog catalog already is.

smaller, separate: can we collapse to one entry point? the generic SameIcebergRepresentation (numeric only, no compat) next to SameIcebergStoredRepresentation is easy to grab by mistake and quietly brings back the nested-uuid gap. and the public leaf callback + IcebergTypePosition read like an extension seam for a caller whose rules aren't pg_lake's, which feels backwards for core (and position is unused today). fine to keep if you have a near-term use.

[sfc-gh-dachristensen]

Pushed the cleanup in da57503:

• Collapsed to the single SameIcebergStoredRepresentation entry point and dropped the generic SameIcebergRepresentation (numeric-only, compat-blind) so there's nothing sitting next to it that's easy to grab by mistake and quietly reintroduce the nested-uuid gap.
• Removed IcebergTypePosition/IcebergParentKind. The unsupported-numeric rule is uniform across levels (position really was unused), and the one depth-dependent transform — the compat uuid mapping — is already a separate depth-aware Field-tree pass (ApplyCompatibilityStorageMapping), so nothing needs the position.
• Made IcebergCreatePathLeafConversion file-static and reframed the leaf callback as an internal mechanism this module uses to reproduce the create path, not a public hook for callers whose rules aren't pg_lake's.

On the ground-truth point: agreed, and I don't think it changes this PR. Re-deriving old answers "would a fresh create of old and new match under today's settings", which drifts from what's physically stored the moment unsupported_numeric_as_double flips (it's PGC_USERSET) or the derivation logic moves across a release — exactly your two cases. The fix belongs on #704, where the changelog table already exists and field_id_mappings holds the persisted field: derive only new's stored field via this helper and compare it against that persisted field, so old is ground truth rather than a recomputation. This PR stays the derivation primitive for that; the two-sided SameIcebergStoredRepresentation remains as the convenience/test form, but #704 won't lean on its re-derived old side. I'll wire the ground-truth comparison on the #704 side.

[snowflake-security-bot]

Snowflake Security Review

Security grade: A — Passed ✅

No security findings after adjudication. This PR passes the Snowflake Security Review.

📊 4 of 6 files (2 tests skipped) · 1,186 lines reviewed · retrieval: on

[sfc-gh-dachristensen]

it's 2026.

[sfc-gh-dachristensen]

It's 2026.

[snowflake-security-bot]

Snowflake Security Review

Security grade: A — Passed ✅

No security findings after adjudication. This PR passes the Snowflake Security Review.

📊 4 of 6 files (2 tests skipped) · 1,228 lines reviewed · retrieval: on

[sfc-gh-okalaci]

pushed an alternative for the API shape: #495 (same base as this PR, so the diffs compare directly). the derivation rework here is right — the disagreement is only about what gets exported.

1. one code path, exercised by every caller. register_field_ids.c:247-254 is already PostgresTypeToIcebergField + DeepCopyField + ApplyCompatibilityStorageMapping — exactly what DeriveIcebergStoredField reproduces. so extract that and have registration call it, rather than standing up a second copy beside it.

this matters more here than DRY usually does, because the whole PR is about edge cases. a duplicated model is only ever as correct as the tests that pin it — and for nested oversized numerics, nested uuid under compat, interval-as-struct, nobody writes those tests. that's precisely how the top-level-only bug survived in the first place. extract it and the edge cases stop depending on anticipation: every CREATE TABLE in the suite drives the same function, so a divergence shows up as a broken create-path test rather than as a wrong true in a guard nobody is looking at.

same argument for the numeric rule. ConvertTypeTree is, by its own comment, "the single place that knows how pg_lake types nest ... so independent passes cannot drift out of coverage". a leaf-callback re-implementation is a second nesting model. when someone adds a leaf rule there — and that's the designed extension point — the create path picks it up and the copy doesn't, silently, in exactly the nested cases this PR exists to get right.

2. drop the two-sided entry point — it fails open. a transform we don't model cancels out on both sides and yields a spurious true: ALTER allowed, nothing replicated, permanent silent divergence. against a persisted field the same gap makes them differ, so the caller blocks. and the transform set isn't closed: snowflake_cdc stores enums and user-defined ranges as text before the table is created (type_store_as_text.h), so a pg_lake-side oracle can never be the complete answer for the caller that motivated it. only the persisted field reflects all four transforms.

3. one-sided derivation deletes the rest. no IcebergCreatePathContext (compat comes from the relation, the GUC is read where pg_lake reads it), no leaf callback, no NULL Field, no Internal split — iceberg_field.c untouched, its 11 call sites keep a single postcondition. the NULL was never a style wart; it was the tell that the transform went into the wrong phase. note the other two storage transforms are already post-derivation Field-tree passes, and the codecs already walk surface/storage trees in parallel — threading a callback into the derivation is the odd one out.

net: ~23% less production code, and your tests carried over unmodified (GUC pinned with SET instead of a 4th arg) plus a layer pinning the exact stored type — numeric(50,2)[] → list<double> — because pairwise same/different can agree for the wrong reason when a derivation is wrong identically on both sides, which is the original bug. CI green, 63/63.

to be clear this is me trying to work out a shape i'd be comfortable maintaining, not a rejection of the work here — does the reasoning hold up from your side? happy to be talked out of any of it, particularly if you see a caller that genuinely wants the two-sided form.

[pgguru]

I think this makes sense as a simplification; to the extent we can use the exact same code paths, this would be a better approach.

[sfc-gh-dachristensen]

Closing in lieu of #495