From f5a7faa3ae42079c12b80731a9d06cdf58b9dc6a Mon Sep 17 00:00:00 2001 From: ChronicallyJD Date: Mon, 27 Jul 2026 17:15:23 -0600 Subject: [PATCH 1/2] Re-aim the stripe-boundary trigger check at what it actually tests The case claimed to cover rows on both sides of a stripe boundary. It did not, and it cannot: the failure depends only on whether a stripe flushed during the statement, and the threshold is exactly stripe_row_limit -- 999 rows at a limit of 1000 fail, 1000 fire, and 1500 rows at a limit of 100000 fail. Crossing the boundary is what rescues the statement, so the case could not be made to fail by growing it. Kept as an honest control, with the measurements recorded in the file, and paired with the discriminating case it was standing in for: more than one row and fewer than the stripe limit, so every trigger has to reach a buffered row. Against the pre-fix build the file is now 11 fail 3 pass, the three passes being the crossing case, the BEFORE trigger and the deferred constraint trigger. --- test/row_triggers.sh | 42 ++++++++++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/test/row_triggers.sh b/test/row_triggers.sh index 00476cf..668b3cf 100755 --- a/test/row_triggers.sh +++ b/test/row_triggers.sh @@ -80,18 +80,21 @@ psql_run "INSERT INTO tr_c2 SELECT g, g * 2 FROM generate_series(1, $ROWS) g;" > psql_run "INSERT INTO tr_h2 SELECT g, g * 2 FROM generate_series(1, $ROWS) g;" >/dev/null both "a multi-row insert fires once per row with the right values" 2 -# More rows than fit one stripe, so some are flushed when the trigger runs and -# some are still buffered -- the two sides of the branch being fixed. 1000 is the -# floor for stripe_row_limit; an earlier version of this file asked for 100, the -# SET failed, the table was never created, and the check compared 0 against 450 -# rather than exercising anything. +# A mid-statement stripe flush rescues the whole statement, and that bounds the +# defect, so it gets a case of its own. # -# The two INSERTs are issued separately and on purpose. Run in one psql_run they -# share ON_ERROR_STOP: against the unfixed build the columnar INSERT errors, the -# script stops, the heap INSERT never runs, and the comparison finds 0 against 0 -# and passes. That is a check satisfied by both outcomes, which is the trap this -# project keeps hitting; every heap/columnar pair below is issued as two calls -# for that reason. +# This file first claimed to cover "rows on both sides of a stripe boundary". +# It did not, and it cannot: measured against the unfixed build, the failure +# depends only on whether a flush happened during the statement, and the +# threshold is exactly stripe_row_limit. +# +# 999 rows at stripe_row_limit=1000 -> fails +# 1000 rows at stripe_row_limit=1000 -> all 1000 fire +# 1500 rows at stripe_row_limit=100000 -> fails +# +# An insert that crosses the limit therefore cannot be made to fail by making it +# bigger -- crossing is what rescues it. The case is kept and now says what it +# is: a control that passed before the fix and has to keep passing after it. psql_run "DROP TABLE IF EXISTS tr_c3; DROP TABLE IF EXISTS tr_h3; SET pgcolumnar.stripe_row_limit = 1000; CREATE TABLE tr_c3 (id int, val int) USING pgcolumnar; @@ -101,7 +104,22 @@ psql_run "CREATE TABLE tr_h3 (id int, val int); psql_run "SET pgcolumnar.stripe_row_limit = 1000; INSERT INTO tr_c3 SELECT g, g FROM generate_series(1, 4500) g;" >/dev/null 2>&1 || true psql_run "INSERT INTO tr_h3 SELECT g, g FROM generate_series(1, 4500) g;" >/dev/null -both "rows on both sides of a stripe boundary all fire" 3 +both "an insert crossing a stripe boundary fires for every row" 3 + +# The discriminating multi-row case: more than one row and fewer than the stripe +# limit, so nothing flushes mid-statement and every trigger has to reach a +# buffered row. This is the shape an ordinary insert has, the default +# stripe_row_limit being far larger than a typical statement. +psql_run "DROP TABLE IF EXISTS tr_c7; DROP TABLE IF EXISTS tr_h7; + SET pgcolumnar.stripe_row_limit = 100000; + CREATE TABLE tr_c7 (id int, val int) USING pgcolumnar; + CREATE TRIGGER tr_c7_t AFTER INSERT ON tr_c7 FOR EACH ROW EXECUTE FUNCTION tr_f();" >/dev/null +psql_run "CREATE TABLE tr_h7 (id int, val int); + CREATE TRIGGER tr_h7_t AFTER INSERT ON tr_h7 FOR EACH ROW EXECUTE FUNCTION tr_f();" >/dev/null +psql_run "SET pgcolumnar.stripe_row_limit = 100000; + INSERT INTO tr_c7 SELECT g, g FROM generate_series(1, 1500) g;" >/dev/null 2>&1 || true +psql_run "INSERT INTO tr_h7 SELECT g, g FROM generate_series(1, 1500) g;" >/dev/null +both "1500 rows with no mid-statement flush all fire" 7 # --- 2. the other row events ------------------------------------------------ From 742cba44e7ae9cac5f5aa6f05320bca0d648927e Mon Sep 17 00:00:00 2001 From: ChronicallyJD Date: Mon, 27 Jul 2026 17:20:15 -0600 Subject: [PATCH 2/2] Record why crossing a stripe boundary rescued the statement One successful fetch is enough: the first trigger's body is a statement, and the ExecutorEnd hook flushes all pending writes when it ends, so the outer INSERT's remaining buffered rows reach disk before the later fetches. Confirmed by removing the SQL from the trigger body -- 1200 rows at a stripe limit of 1000 then give 1000 successful fetches and a failure on row 1001, the first buffered row. --- test/row_triggers.sh | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/test/row_triggers.sh b/test/row_triggers.sh index 668b3cf..3388c75 100755 --- a/test/row_triggers.sh +++ b/test/row_triggers.sh @@ -93,8 +93,25 @@ both "a multi-row insert fires once per row with the right values" 2 # 1500 rows at stripe_row_limit=100000 -> fails # # An insert that crosses the limit therefore cannot be made to fail by making it -# bigger -- crossing is what rescues it. The case is kept and now says what it -# is: a control that passed before the fix and has to keep passing after it. +# bigger -- crossing is what rescues it, and the mechanism is worth writing down +# because it is not obvious: +# +# 1. the stripe fills mid-statement and flushes, so the earliest rows are on +# disk before any trigger runs +# 2. the first trigger's fetch therefore succeeds, and its body runs +# 3. the body is itself a statement, and pgColumnar's ExecutorEnd hook calls +# ColumnarFlushAllPendingWrites when it ends -- flushing the outer INSERT's +# remaining buffered rows +# 4. every later fetch then finds its row on disk +# +# So one successful fetch rescues the whole statement, through the trigger body. +# Confirmed by removing the SQL from the body: with a trigger that only does +# RETURN NULL, no ExecutorEnd fires between rows, and 1200 rows at a limit of +# 1000 give 1000 successful fetches and then a failure on row 1001 -- exactly +# the first buffered row. +# +# The case is kept and now says what it is: a control that passed before the fix +# and has to keep passing after it. psql_run "DROP TABLE IF EXISTS tr_c3; DROP TABLE IF EXISTS tr_h3; SET pgcolumnar.stripe_row_limit = 1000; CREATE TABLE tr_c3 (id int, val int) USING pgcolumnar;