Skip to content

Commit 6ff74f1

Browse files
committed
Fix NOT NULL bypass when clamp mode converts values to NULL
ExecConstraints was running before IcebergErrorOrClampSlotInPlace, so values like bounded numeric NaN and multidimensional arrays passed the NOT NULL check in their original (non-null) form, then got clamped to NULL and silently stored in NOT NULL columns. Move clamping before ExecConstraints in both postgresExecForeignInsert and postgresExecForeignUpdate so constraint checks see post-clamp values. Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>
1 parent f59754e commit 6ff74f1

2 files changed

Lines changed: 176 additions & 7 deletions

File tree

pg_lake_table/src/fdw/pg_lake_table.c

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,8 @@ static void merge_fdw_options(PgLakeRelationInfo * fpinfo,
502502
const PgLakeRelationInfo * fpinfo_o,
503503
const PgLakeRelationInfo * fpinfo_i);
504504

505+
static void IcebergErrorOrClampSlotInPlace(TupleTableSlot *slot, TupleDesc tupleDesc,
506+
IcebergOutOfRangePolicy policy);
505507
static void WriteInsertRecord(PgLakeModifyState * modifyState, TupleTableSlot *slot);
506508
static void PrepareDeletionSlot(PgLakeFileModifyState * fileModifyState,
507509
uint64 fileRowNumber,
@@ -2220,6 +2222,16 @@ postgresExecForeignInsert(EState *estate,
22202222
{
22212223
PgLakeModifyState *fmstate = (PgLakeModifyState *) resultRelInfo->ri_FdwState;
22222224

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+
22232235
/*
22242236
* Constraint checks are skipped by PostgreSQL itself, since it assumes
22252237
* them to be unenforceable as data can change underneath.
@@ -2268,6 +2280,16 @@ postgresExecForeignUpdate(EState *estate,
22682280
return NULL;
22692281
}
22702282

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+
22712293
/*
22722294
* Constraint checks are skipped by PostgreSQL itself, since it assumes
22732295
* them to be unenforceable as data can change underneath.
@@ -2643,19 +2665,16 @@ IcebergErrorOrClampSlotInPlace(TupleTableSlot *slot, TupleDesc tupleDesc,
26432665

26442666

26452667
/*
2646-
* WriteInsertRecord validates the tuple against Iceberg write constraints
2647-
* (when applicable) and forwards it to the insert destination.
2668+
* WriteInsertRecord forwards the tuple to the insert destination.
2669+
*
2670+
* Callers are responsible for running IcebergErrorOrClampSlotInPlace
2671+
* before ExecConstraints so that NOT NULL checks see post-clamp values.
26482672
*/
26492673
static void
26502674
WriteInsertRecord(PgLakeModifyState * modifyState, TupleTableSlot *slot)
26512675
{
26522676
DestReceiver *insertDest = modifyState->insertDest;
26532677

2654-
if (modifyState->outOfRangePolicy != ICEBERG_OOR_NONE &&
2655-
modifyState->needsOutOfRangeValidation)
2656-
IcebergErrorOrClampSlotInPlace(slot, modifyState->tupleDesc,
2657-
modifyState->outOfRangePolicy);
2658-
26592678
if (modifyState->insertedRowCount == 0)
26602679
{
26612680
/* incoming inserts have the tuple descriptor of the table */

pg_lake_table/tests/pytests/test_iceberg_validation.py

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2844,3 +2844,153 @@ def test_explain_shows_pg_nullify_nested_list_in_list_transform(
28442844
run_command("RESET search_path;", pg_conn)
28452845
run_command(f"DROP SCHEMA IF EXISTS {schema} CASCADE;", pg_conn)
28462846
pg_conn.commit()
2847+
2848+
2849+
# =====================================================================
2850+
# NOT NULL constraint enforcement after clamping
2851+
#
2852+
# When out_of_range_values = 'clamp', values like bounded numeric NaN
2853+
# and multidimensional arrays are clamped to NULL. The NOT NULL
2854+
# constraint must still be enforced on the post-clamp value.
2855+
# =====================================================================
2856+
2857+
2858+
def test_clamp_to_null_enforces_not_null(
2859+
pg_conn, extension, s3, with_default_location
2860+
):
2861+
"""NOT NULL constraint is enforced on values clamped to NULL.
2862+
2863+
Covers both value types that clamp to NULL (bounded numeric NaN and
2864+
multidimensional arrays) and both DML paths (INSERT and UPDATE).
2865+
2866+
+-------+---------------------+--------+----------------------------+
2867+
| case | value | op | expected |
2868+
+-------+---------------------+--------+----------------------------+
2869+
| NaN | 'NaN'::numeric | INSERT | not-null constraint error |
2870+
| NaN | 'NaN'::numeric | UPDATE | not-null constraint error |
2871+
| mdim | ARRAY[ARRAY[1,2,3]] | INSERT | not-null constraint error |
2872+
| mdim | ARRAY[ARRAY[1,2,3]] | UPDATE | not-null constraint error |
2873+
+-------+---------------------+--------+----------------------------+
2874+
"""
2875+
schema = "test_clamp_null_nn"
2876+
2877+
run_command(f"CREATE SCHEMA {schema};", pg_conn)
2878+
run_command(f"SET search_path TO {schema};", pg_conn)
2879+
2880+
try:
2881+
run_command(
2882+
"CREATE TABLE num_target ("
2883+
" id int,"
2884+
" n numeric(18,6) NOT NULL"
2885+
") USING iceberg WITH (out_of_range_values = 'clamp');",
2886+
pg_conn,
2887+
)
2888+
run_command(
2889+
"CREATE TABLE arr_target ("
2890+
" id int,"
2891+
" vals int[] NOT NULL"
2892+
") USING iceberg WITH (out_of_range_values = 'clamp');",
2893+
pg_conn,
2894+
)
2895+
pg_conn.commit()
2896+
2897+
# -- NaN INSERT into NOT NULL numeric --
2898+
err = run_command(
2899+
"INSERT INTO num_target VALUES (1, 'NaN'::numeric(18,6));",
2900+
pg_conn,
2901+
raise_error=False,
2902+
)
2903+
assert "not-null constraint" in str(err)
2904+
pg_conn.rollback()
2905+
2906+
# -- NaN UPDATE on NOT NULL numeric --
2907+
run_command("INSERT INTO num_target VALUES (1, 42.0);", pg_conn)
2908+
pg_conn.commit()
2909+
2910+
err = run_command(
2911+
"UPDATE num_target SET n = 'NaN'::numeric(18,6) WHERE id = 1;",
2912+
pg_conn,
2913+
raise_error=False,
2914+
)
2915+
assert "not-null constraint" in str(err)
2916+
pg_conn.rollback()
2917+
2918+
result = run_query("SELECT n FROM num_target WHERE id = 1;", pg_conn)
2919+
assert result[0][0] is not None
2920+
2921+
# -- Multidimensional array INSERT into NOT NULL column --
2922+
err = run_command(
2923+
"INSERT INTO arr_target VALUES (1, ARRAY[ARRAY[1,2,3]]);",
2924+
pg_conn,
2925+
raise_error=False,
2926+
)
2927+
assert "not-null constraint" in str(err)
2928+
pg_conn.rollback()
2929+
2930+
# -- Multidimensional array UPDATE on NOT NULL column --
2931+
run_command("INSERT INTO arr_target VALUES (1, ARRAY[10,20,30]);", pg_conn)
2932+
pg_conn.commit()
2933+
2934+
err = run_command(
2935+
"UPDATE arr_target SET vals = ARRAY[ARRAY[1,2],ARRAY[3,4]] WHERE id = 1;",
2936+
pg_conn,
2937+
raise_error=False,
2938+
)
2939+
assert "not-null constraint" in str(err)
2940+
pg_conn.rollback()
2941+
2942+
result = run_query("SELECT vals FROM arr_target WHERE id = 1;", pg_conn)
2943+
assert result[0][0] == [10, 20, 30]
2944+
finally:
2945+
pg_conn.rollback()
2946+
run_command("RESET search_path;", pg_conn)
2947+
run_command(f"DROP SCHEMA IF EXISTS {schema} CASCADE;", pg_conn)
2948+
pg_conn.commit()
2949+
2950+
2951+
# =====================================================================
2952+
# CHECK constraint interaction with clamping
2953+
#
2954+
# When clamping runs before constraint checks, CHECK constraints
2955+
# evaluate the post-clamp value. This means clamping can "rescue"
2956+
# values that would otherwise violate a CHECK. These tests document
2957+
# the expected behavior: the CHECK validates what is actually stored.
2958+
# =====================================================================
2959+
2960+
2961+
def test_temporal_clamp_rescues_check_constraint(
2962+
pg_conn, extension, s3, with_default_location
2963+
):
2964+
"""Temporal clamping can satisfy a CHECK that the original value would violate.
2965+
2966+
Year 10000 violates CHECK (d <= '9999-12-31'), but clamping brings it
2967+
to exactly 9999-12-31, which satisfies the CHECK.
2968+
"""
2969+
schema = "test_clamp_check_temporal"
2970+
2971+
run_command(f"CREATE SCHEMA {schema};", pg_conn)
2972+
run_command(f"SET search_path TO {schema};", pg_conn)
2973+
run_command("SET TIME ZONE 'UTC';", pg_conn)
2974+
2975+
try:
2976+
run_command(
2977+
"CREATE TABLE target (d date CHECK (d <= '9999-12-31'::date))"
2978+
" USING iceberg WITH (out_of_range_values = 'clamp');",
2979+
pg_conn,
2980+
)
2981+
pg_conn.commit()
2982+
2983+
run_command(
2984+
"INSERT INTO target VALUES ('infinity'::date);",
2985+
pg_conn,
2986+
)
2987+
pg_conn.commit()
2988+
2989+
result = run_query("SELECT d::text FROM target;", pg_conn)
2990+
assert result[0][0] == "9999-12-31"
2991+
finally:
2992+
pg_conn.rollback()
2993+
run_command("RESET TIME ZONE;", pg_conn)
2994+
run_command("RESET search_path;", pg_conn)
2995+
run_command(f"DROP SCHEMA IF EXISTS {schema} CASCADE;", pg_conn)
2996+
pg_conn.commit()

0 commit comments

Comments
 (0)