Skip to content

Commit 6336ff0

Browse files
committed
Extract ClampAndCheckConstraints helper, add constraint interaction tests
Factor the clamping + ExecConstraints sequence into a shared ClampAndCheckConstraints helper used by both INSERT and UPDATE paths. Add tests for CHECK constraint interaction with clamping: - temporal clamp rescues a value that would fail strict CHECK - temporal clamp still fails when clamped value violates CHECK - NaN clamp to NULL succeeds on nullable column (INSERT and UPDATE) Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>
1 parent 9149c48 commit 6336ff0

2 files changed

Lines changed: 122 additions & 37 deletions

File tree

pg_lake_table/src/fdw/pg_lake_table.c

Lines changed: 32 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,9 @@ static void merge_fdw_options(PgLakeRelationInfo * fpinfo,
504504

505505
static void IcebergErrorOrClampSlotInPlace(TupleTableSlot *slot, TupleDesc tupleDesc,
506506
IcebergOutOfRangePolicy policy);
507+
static void ClampAndCheckConstraints(PgLakeModifyState * fmstate,
508+
ResultRelInfo *resultRelInfo,
509+
TupleTableSlot *slot, EState *estate);
507510
static void WriteInsertRecord(PgLakeModifyState * modifyState, TupleTableSlot *slot);
508511
static void PrepareDeletionSlot(PgLakeFileModifyState * fileModifyState,
509512
uint64 fileRowNumber,
@@ -2222,24 +2225,7 @@ postgresExecForeignInsert(EState *estate,
22222225
{
22232226
PgLakeModifyState *fmstate = (PgLakeModifyState *) resultRelInfo->ri_FdwState;
22242227

2225-
/*
2226-
* Clamp out-of-range values (e.g. NaN → NULL) before constraint checks
2227-
* so that ExecConstraints sees the post-clamp slot and can enforce NOT
2228-
* NULL on values that were clamped to NULL.
2229-
*/
2230-
if (fmstate->outOfRangePolicy != ICEBERG_OOR_NONE &&
2231-
fmstate->needsOutOfRangeValidation)
2232-
IcebergErrorOrClampSlotInPlace(slot, fmstate->tupleDesc,
2233-
fmstate->outOfRangePolicy);
2234-
2235-
/*
2236-
* Constraint checks are skipped by PostgreSQL itself, since it assumes
2237-
* them to be unenforceable as data can change underneath.
2238-
*/
2239-
Relation resultRelationDesc = resultRelInfo->ri_RelationDesc;
2240-
2241-
if (resultRelationDesc->rd_att->constr)
2242-
ExecConstraints(resultRelInfo, slot, estate);
2228+
ClampAndCheckConstraints(fmstate, resultRelInfo, slot, estate);
22432229

22442230
WriteInsertRecord(fmstate, slot);
22452231

@@ -2280,22 +2266,7 @@ postgresExecForeignUpdate(EState *estate,
22802266
return NULL;
22812267
}
22822268

2283-
/*
2284-
* Clamp out-of-range values (e.g. NaN → NULL) before constraint checks
2285-
* so that ExecConstraints sees the post-clamp slot and can enforce NOT
2286-
* NULL on values that were clamped to NULL.
2287-
*/
2288-
if (fmstate->outOfRangePolicy != ICEBERG_OOR_NONE &&
2289-
fmstate->needsOutOfRangeValidation)
2290-
IcebergErrorOrClampSlotInPlace(slot, fmstate->tupleDesc,
2291-
fmstate->outOfRangePolicy);
2292-
2293-
/*
2294-
* Constraint checks are skipped by PostgreSQL itself, since it assumes
2295-
* them to be unenforceable as data can change underneath.
2296-
*/
2297-
if (resultRelation->rd_att->constr)
2298-
ExecConstraints(resultRelInfo, slot, estate);
2269+
ClampAndCheckConstraints(fmstate, resultRelInfo, slot, estate);
22992270

23002271
/*
23012272
* Also check the tuple against the partition constraint, since PostgreSQL
@@ -2665,10 +2636,34 @@ IcebergErrorOrClampSlotInPlace(TupleTableSlot *slot, TupleDesc tupleDesc,
26652636

26662637

26672638
/*
2668-
* WriteInsertRecord forwards the tuple to the insert destination.
2639+
* ClampAndCheckConstraints normalizes the slot for Iceberg write and then
2640+
* runs PostgreSQL constraint checks (NOT NULL, CHECK, etc.).
2641+
*
2642+
* Clamping must happen first so that ExecConstraints sees post-clamp values,
2643+
* e.g. bounded numeric NaN clamped to NULL is caught by NOT NULL.
26692644
*
2670-
* Callers are responsible for running IcebergErrorOrClampSlotInPlace
2671-
* before ExecConstraints so that NOT NULL checks see post-clamp values.
2645+
* PostgreSQL skips constraint checks on foreign tables, so we run them
2646+
* ourselves.
2647+
*/
2648+
static void
2649+
ClampAndCheckConstraints(PgLakeModifyState * fmstate,
2650+
ResultRelInfo *resultRelInfo,
2651+
TupleTableSlot *slot, EState *estate)
2652+
{
2653+
if (fmstate->outOfRangePolicy != ICEBERG_OOR_NONE &&
2654+
fmstate->needsOutOfRangeValidation)
2655+
IcebergErrorOrClampSlotInPlace(slot, fmstate->tupleDesc,
2656+
fmstate->outOfRangePolicy);
2657+
2658+
Relation rel = resultRelInfo->ri_RelationDesc;
2659+
2660+
if (rel->rd_att->constr)
2661+
ExecConstraints(resultRelInfo, slot, estate);
2662+
}
2663+
2664+
2665+
/*
2666+
* WriteInsertRecord forwards the tuple to the insert destination.
26722667
*/
26732668
static void
26742669
WriteInsertRecord(PgLakeModifyState * modifyState, TupleTableSlot *slot)

pg_lake_table/tests/pytests/test_iceberg_validation.py

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2992,3 +2992,93 @@ def test_temporal_clamp_rescues_check_constraint(
29922992
run_command("RESET search_path;", pg_conn)
29932993
run_command(f"DROP SCHEMA IF EXISTS {schema} CASCADE;", pg_conn)
29942994
pg_conn.commit()
2995+
2996+
2997+
def test_temporal_clamp_still_fails_strict_check_constraint(
2998+
pg_conn, extension, s3, with_default_location
2999+
):
3000+
"""Temporal clamping cannot rescue a CHECK that the clamped value still violates.
3001+
3002+
infinity is clamped to 9999-12-31, but CHECK (d < '9999-12-31') uses
3003+
strict less-than, so the clamped value still fails.
3004+
"""
3005+
schema = "test_clamp_check_strict"
3006+
3007+
run_command(f"CREATE SCHEMA {schema};", pg_conn)
3008+
run_command(f"SET search_path TO {schema};", pg_conn)
3009+
run_command("SET TIME ZONE 'UTC';", pg_conn)
3010+
3011+
try:
3012+
run_command(
3013+
"CREATE TABLE target (d date CHECK (d < '9999-12-31'::date))"
3014+
" USING iceberg WITH (out_of_range_values = 'clamp');",
3015+
pg_conn,
3016+
)
3017+
pg_conn.commit()
3018+
3019+
err = run_command(
3020+
"INSERT INTO target VALUES ('infinity'::date);",
3021+
pg_conn,
3022+
raise_error=False,
3023+
)
3024+
assert "check constraint" in str(err).lower()
3025+
pg_conn.rollback()
3026+
finally:
3027+
pg_conn.rollback()
3028+
run_command("RESET TIME ZONE;", pg_conn)
3029+
run_command("RESET search_path;", pg_conn)
3030+
run_command(f"DROP SCHEMA IF EXISTS {schema} CASCADE;", pg_conn)
3031+
pg_conn.commit()
3032+
3033+
3034+
def test_numeric_nan_clamp_succeeds_on_nullable_column(
3035+
pg_conn, extension, s3, with_default_location
3036+
):
3037+
"""Bounded numeric NaN is clamped to NULL and stored when the column is nullable.
3038+
3039+
This is the happy-path counterpart of the NOT NULL tests: clamping
3040+
converts NaN to NULL, which is valid for a nullable column. Covers
3041+
both INSERT and UPDATE.
3042+
"""
3043+
schema = "test_clamp_nan_nullable"
3044+
3045+
run_command(f"CREATE SCHEMA {schema};", pg_conn)
3046+
run_command(f"SET search_path TO {schema};", pg_conn)
3047+
3048+
try:
3049+
run_command(
3050+
"CREATE TABLE target ("
3051+
" id int,"
3052+
" n numeric(18,6)"
3053+
") USING iceberg WITH (out_of_range_values = 'clamp');",
3054+
pg_conn,
3055+
)
3056+
pg_conn.commit()
3057+
3058+
# INSERT: NaN is clamped to NULL and succeeds
3059+
run_command(
3060+
"INSERT INTO target VALUES (1, 'NaN'::numeric(18,6));",
3061+
pg_conn,
3062+
)
3063+
pg_conn.commit()
3064+
3065+
result = run_query("SELECT n FROM target WHERE id = 1;", pg_conn)
3066+
assert result[0][0] is None
3067+
3068+
# UPDATE: NaN is clamped to NULL and succeeds
3069+
run_command("INSERT INTO target VALUES (2, 99.0);", pg_conn)
3070+
pg_conn.commit()
3071+
3072+
run_command(
3073+
"UPDATE target SET n = 'NaN'::numeric(18,6) WHERE id = 2;",
3074+
pg_conn,
3075+
)
3076+
pg_conn.commit()
3077+
3078+
result = run_query("SELECT n FROM target WHERE id = 2;", pg_conn)
3079+
assert result[0][0] is None
3080+
finally:
3081+
pg_conn.rollback()
3082+
run_command("RESET search_path;", pg_conn)
3083+
run_command(f"DROP SCHEMA IF EXISTS {schema} CASCADE;", pg_conn)
3084+
pg_conn.commit()

0 commit comments

Comments
 (0)