diff --git a/src/columnar_metadata.c b/src/columnar_metadata.c index 21efd3add..6c3263945 100644 --- a/src/columnar_metadata.c +++ b/src/columnar_metadata.c @@ -1854,6 +1854,7 @@ PgColumnarUpsertDeleteVector(uint64 storageId, DeleteVectorMetadata *rm) int deletedCount = rm->deletedCount; ItemPointerData replaceTid; bool haveReplace = false; + bool writtenThisCommand = false; delete_vector_lock_chunk_group(storageId, rm->groupNumber, 0); @@ -1945,6 +1946,17 @@ PgColumnarUpsertDeleteVector(uint64 storageId, DeleteVectorMetadata *rm) replaceTid = existing->t_self; haveReplace = true; + + /* + * TWO WRITES OF ONE GROUP IN ONE COMMAND (#1394). A statement that starts a + * scan inside another -- a DELETE in a function the outer DELETE's qual calls + * -- writes the outer marks at its scan start and its own at its end, in the + * same command. heap_update refuses a version written by the current command + * ("attempted to update invisible tuple"), so advance the command first. + */ + writtenThisCommand = + TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(existing->t_data)) && + HeapTupleHeaderGetCmin(existing->t_data) >= GetCurrentCommandId(false); } systable_endscan(scan); @@ -1962,6 +1974,8 @@ PgColumnarUpsertDeleteVector(uint64 storageId, DeleteVectorMetadata *rm) if (haveReplace) { + if (writtenThisCommand) + CommandCounterIncrement(); tuple->t_self = replaceTid; CatalogTupleUpdate(rel, &replaceTid, tuple); } diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index 79611db67..6c59a597d 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -1887,6 +1887,14 @@ native_chunk_length_bound native_chunk_length_bound backend survived the sequent native_chunk_length_bound native_chunk_length_bound backend survived the truncated-length fetch 15;16;17;18;19 never - native_chunk_length_bound native_chunk_length_bound premise: a point lookup uses the index, not a sequential columnar scan 15;16;17;18;19 never - native_chunk_length_bound native_chunk_length_bound premise: that fetch returns the row 15;16;17;18;19 never - +native_delete_visibility_paths native_delete_visibility_paths a DELETE whose qual deletes another row of the table succeeds and leaves heap's rows (#1394) 15;16;17;18;19 2026-10-14 PgColumnarUpsertDeleteVector without the command advance (main before #1394's fix) +native_delete_visibility_paths native_delete_visibility_paths a deleted row is invisible via the index 15;16;17;18;19 never - +native_delete_visibility_paths native_delete_visibility_paths a live row is visible via the index 15;16;17;18;19 never - +native_delete_visibility_paths native_delete_visibility_paths a row deleted in-transaction is invisible in the same transaction 15;16;17;18;19 never - +native_delete_visibility_paths native_delete_visibility_paths aggregate over the whole table sees the live set 15;16;17;18;19 never - +native_delete_visibility_paths native_delete_visibility_paths index / bitmap scan sees the live set 15;16;17;18;19 never - +native_delete_visibility_paths native_delete_visibility_paths index-only scan sees the live set 15;16;17;18;19 never - +native_delete_visibility_paths native_delete_visibility_paths sequential scan sees the live set (row-emitting path) 15;16;17;18;19 never - native_fetch_coalesce native_fetch_coalesce a wide index fetch does not pin once per column 15;16;17;18;19 2026-10-13 the coalesced fetch merging no adjacent ranges, so each column is read on its own native_fetch_coalesce native_fetch_coalesce premise: a point lookup uses the index 15;16;17;18;19 never - native_fetch_coalesce native_fetch_coalesce premise: fetching every projected column touched a measurable number of buffers 15;16;17;18;19 never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index a9d17b994..c726b914c 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -15,7 +15,7 @@ # # Adding a check to a suite that is already covered does not move it, which is # what makes it safe to bound. -suites_not_covered 243 +suites_not_covered 242 # # checks_never_observed_red -- A CENSUS. NOT a ceiling, and it must not become # one. @@ -1979,4 +1979,16 @@ suites_not_covered 243 # never 1895 + 23 = 1918 # dated 577 + 2 = 579 # 1918 + 579 == 2497 -checks_never_observed_red 1918 +# +# #1394, a DELETE whose qual deletes another row of the same table (jdatcmd): 1918 -> +# 1925, and suites_not_covered 243 -> 242. native_delete_visibility_paths had no rows; +# it enters with all eight checks from five-major runs (8/8 on each of 15-19). The new +# arm is dated by main's upsert, which failed with "attempted to update invisible +# tuple" (55000). The other seven arrive never: seeding a suite moves the census up by +# its undated rows. +# +# ledger rows 2497 + 8 = 2505 +# never 1918 + 7 = 1925 +# dated 579 + 1 = 580 +# 1925 + 580 == 2505 +checks_never_observed_red 1925 diff --git a/test/native_delete_visibility_paths.sh b/test/native_delete_visibility_paths.sh index 170b422cc..6cddc1c45 100755 --- a/test/native_delete_visibility_paths.sh +++ b/test/native_delete_visibility_paths.sh @@ -58,4 +58,27 @@ check "a live row is visible via the index" \ check "a row deleted in-transaction is invisible in the same transaction" \ "$(q 'BEGIN; DELETE FROM t WHERE id = 8; SELECT count(*) FROM t WHERE id = 8; ROLLBACK;' | grep -E '^[0-9]+$' | head -1)" "0" +# A DELETE whose qual deletes another row of the same table (#1394). The inner DELETE's +# scan start writes the outer statement's buffered marks, and its statement end writes +# its own mark to the same row group, both in one command. The second write updated a +# delete_vector row the first had written in that command, and failed with "attempted +# to update invisible tuple". Run on heap too; the columnar answer must be heap's. +psql_run "CREATE FUNCTION del_next(i int, t text) RETURNS int LANGUAGE plpgsql AS \$\$ +BEGIN EXECUTE format('DELETE FROM %s WHERE id = %s', t, i + 1); RETURN 1; END \$\$;" +for am in pgcolumnar heap; do + psql_run "CREATE TABLE dn_$am (id int, v int) USING $am; + INSERT INTO dn_$am SELECT g, 0 FROM generate_series(1, 20) g;" +done +dn() { # dn AM -> ok, or the SQLSTATE the DELETE fails with + local out + out=$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" \ + -X -At -v ON_ERROR_STOP=1 -v VERBOSITY=sqlstate \ + -c "DELETE FROM dn_$1 WHERE id % 2 = 0 AND del_next(id, 'dn_$1') IS NOT NULL" 2>&1 >/dev/null) + [ -z "$out" ] && { echo ok; return; } + echo "$out" | sed -n 's/^ERROR: *\([0-9A-Z]\{5\}\).*/\1/p' | head -1 +} +dn_after() { q "SELECT count(*) || '|' || coalesce(sum(id), 0) FROM dn_$1"; } +check "a DELETE whose qual deletes another row of the table succeeds and leaves heap's rows (#1394)" \ + "$(dn pgcolumnar)|$(dn_after pgcolumnar)" "$(dn heap)|$(dn_after heap)" + pgc_summary diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index fca414a22..98acbe2b3 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -5250,6 +5250,7 @@ its row in place, so the in-transaction arm cannot be satisfied by a delete that | test | what it holds | | --- | --- | | `test_native_delete_visibility_paths` | every arm: the row count and group span, then the live set through a Seq Scan, an Index Scan, an Index Only Scan and the columnar aggregate — each with its plan asserted — then one deleted and one live row through the index, and a delete read back inside its own transaction | +| `test_a_delete_whose_qual_deletes_another_row_matches_heap` | a DELETE whose qual calls a function that deletes the next row of the same table succeeds and leaves heap's rows (#1394). Its inner DELETE writes the outer marks at scan start and its own at statement end, in one command, and the second write failed with "attempted to update invisible tuple" | ## 66. test_temporal.py: a columnar table enforces temporal constraints like a heap diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index 85e77a583..6bc6c4a00 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -642,4 +642,6 @@ guard_tests 433 # cluster_tests 539 -> 541 for #1298's test_a_catalog_reading_takes_in_none_of_another_sessions_reads # in test_catalog_delete_index.py and test_a_storage_reading_takes_in_none_of_another_sessions_reads # in test_catalog_plan_index.py, by collection over the cluster files on this tree. -cluster_tests 541 +# cluster_tests 541 -> 542 for #1394's test_a_delete_whose_qual_deletes_another_row_matches_heap +# in test_native_delete_visibility_paths.py, by collection over the cluster files on this tree. +cluster_tests 542 diff --git a/test/pytest/test_native_delete_visibility_paths.py b/test/pytest/test_native_delete_visibility_paths.py index 81d9c2bba..b01072285 100644 --- a/test/pytest/test_native_delete_visibility_paths.py +++ b/test/pytest/test_native_delete_visibility_paths.py @@ -36,6 +36,8 @@ the count. The shell suite is unchanged -- reported rather than edited, since this pull request adds no bash check. """ +import psycopg + ROWS = 5000 GROUP = 1000 LIVE = ROWS - ROWS // 7 # 5000 - 714 @@ -154,3 +156,29 @@ def test_native_delete_visibility_paths(pgc_conn, expect): cur.execute("SELECT count(*) FROM t WHERE id = 8") expect.num(cur.fetchone()[0], 1, "premise: the rolled-back delete left the row in place") + + +def test_a_delete_whose_qual_deletes_another_row_matches_heap(pgc_conn, expect): + """A DELETE whose qual deletes another row of the same table (#1394). The inner + DELETE's scan start writes the outer statement's buffered marks, and its statement + end writes its own mark to the same row group, both in one command. The second + write updated a delete_vector row the first had written in that command, and + failed with "attempted to update invisible tuple". Run on heap too; the columnar + answer must be heap's.""" + with pgc_conn.cursor() as cur: + cur.execute("""CREATE FUNCTION del_next(i int, t text) RETURNS int LANGUAGE plpgsql AS $$ +BEGIN EXECUTE format('DELETE FROM %s WHERE id = %s', t, i + 1); RETURN 1; END $$""") + got = {} + for am in ("pgcolumnar", "heap"): + with pgc_conn.cursor() as cur: + cur.execute(f"CREATE TABLE dn_{am} (id int, v int) USING {am}") + cur.execute(f"INSERT INTO dn_{am} SELECT g, 0 FROM generate_series(1, 20) g") + try: + cur.execute(f"DELETE FROM dn_{am} WHERE id % 2 = 0 AND del_next(id, 'dn_{am}') IS NOT NULL") + state = "ok" + except psycopg.Error as e: + state = e.sqlstate or "?" + cur.execute(f"SELECT count(*) || '|' || coalesce(sum(id), 0) FROM dn_{am}") + got[am] = f"{state}|{cur.fetchone()[0]}" + expect.text(got["pgcolumnar"], got["heap"], + "a DELETE whose qual deletes another row of the table succeeds and leaves heap's rows (#1394)")