Skip to content

Commit

Permalink
[#24649] YSQL: enable INSERT ON CONFLICT batching for FK
Browse files Browse the repository at this point in the history
Summary:
Previously, the presence of after row insert triggers prevented INSERT
ON CONFLICT read batching.  Loosen this restriction to allow for after
row insert triggers as long as they are for foreign key constraint
checking.  No further code changes should be needed because the current
design is sufficient.  Foreign key constraints are checked at the end of
the _statement_ or _transaction_, making them easy to reason with when
it comes to batching.
Jira: DB-13713

Test Plan:
On Almalinux 8:

    #!/usr/bin/env bash
    set -euo pipefail
    ./yb_build.sh fastdebug --gcc11
    find java/yb-pgsql/src/test/java/org/yb/pgsql -name 'TestPgRegressInsertOnConflict*' \
    | grep -oE 'TestPgRegress\w+' \
    | while read -r testname; do
      ./yb_build.sh fastdebug --gcc11 --java-test "$testname" --sj
    done

Close: #24649

Reviewers: kramanathan, #db-approvers

Reviewed By: kramanathan

Subscribers: svc_phabricator, yql

Differential Revision: https://phorge.dev.yugabyte.com/D41108
  • Loading branch information
jaki committed Jan 30, 2025
1 parent 1601c75 commit 16f2042
Show file tree
Hide file tree
Showing 4 changed files with 383 additions and 43 deletions.
46 changes: 33 additions & 13 deletions src/postgres/src/backend/executor/ybModifyTable.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,12 @@
#include "catalog/pg_auth_members_d.h"
#include "catalog/pg_shseclabel_d.h"
#include "catalog/pg_tablespace_d.h"
#include "catalog/pg_trigger.h"
#include "catalog/pg_type.h"
#include "catalog/pg_yb_role_profile.h"
#include "catalog/pg_yb_role_profile_d.h"
#include "catalog/yb_type.h"
#include "commands/trigger.h"
#include "commands/yb_profile.h"
#include "utils/relcache.h"
#include "utils/rel.h"
Expand Down Expand Up @@ -560,19 +562,37 @@ YbIsInsertOnConflictReadBatchingPossible(ResultRelInfo *resultRelInfo)
/*
* TODO(jason): figure out how to enable triggers.
*/
return (yb_insert_on_conflict_read_batch_size > 0 &&
IsYBRelation(resultRelInfo->ri_RelationDesc) &&
!IsCatalogRelation(resultRelInfo->ri_RelationDesc) &&
!(resultRelInfo->ri_TrigDesc &&
(resultRelInfo->ri_TrigDesc->trig_delete_after_row ||
resultRelInfo->ri_TrigDesc->trig_delete_before_row ||
resultRelInfo->ri_TrigDesc->trig_delete_instead_row ||
resultRelInfo->ri_TrigDesc->trig_insert_after_row ||
resultRelInfo->ri_TrigDesc->trig_insert_before_row ||
resultRelInfo->ri_TrigDesc->trig_insert_instead_row ||
resultRelInfo->ri_TrigDesc->trig_update_after_row ||
resultRelInfo->ri_TrigDesc->trig_update_before_row ||
resultRelInfo->ri_TrigDesc->trig_update_instead_row)));
if (yb_insert_on_conflict_read_batch_size == 0 ||
!IsYBRelation(resultRelInfo->ri_RelationDesc) ||
IsCatalogRelation(resultRelInfo->ri_RelationDesc) ||
(resultRelInfo->ri_TrigDesc &&
(resultRelInfo->ri_TrigDesc->trig_delete_before_row ||
resultRelInfo->ri_TrigDesc->trig_delete_instead_row ||
resultRelInfo->ri_TrigDesc->trig_insert_before_row ||
resultRelInfo->ri_TrigDesc->trig_insert_instead_row ||
resultRelInfo->ri_TrigDesc->trig_update_before_row ||
resultRelInfo->ri_TrigDesc->trig_update_instead_row)))
{
return false;
}

TriggerDesc *trigdesc = resultRelInfo->ri_TrigDesc;
if (!(trigdesc && (trigdesc->trig_delete_after_row ||
trigdesc->trig_insert_after_row ||
trigdesc->trig_update_after_row)))
return true;

/* Any non-FK after row triggers make batching not supported. */
for (int i = 0; i < trigdesc->numtriggers; i++)
{
Trigger *trig = &trigdesc->triggers[i];

if (TRIGGER_FOR_AFTER(trig->tgtype) &&
TRIGGER_FOR_ROW(trig->tgtype) &&
RI_FKey_trigger_type(trig->tgfoid) == RI_TRIGGER_NONE)
return false;
}
return true;
}

/*
Expand Down
156 changes: 144 additions & 12 deletions src/postgres/src/test/regress/expected/yb_insert_on_conflict.out
Original file line number Diff line number Diff line change
Expand Up @@ -1138,29 +1138,161 @@ TABLE trigger_test;
(4 rows)

--- Foreign key
CREATE TABLE parent_table (n numeric, t text, b bool, PRIMARY KEY (t, n));
CREATE TABLE child_table (b bool PRIMARY KEY, n numeric, t text, FOREIGN KEY (t, n) REFERENCES parent_table);
CREATE TABLE parent_table (n numeric, t text, b bool, PRIMARY KEY (t, n ASC));
CREATE TABLE child_table (k numeric, n numeric, t text, CONSTRAINT fk FOREIGN KEY (t, n) REFERENCES parent_table, PRIMARY KEY (k ASC));
INSERT INTO parent_table VALUES (1, '1', true), (2, '2', true);
INSERT INTO child_table VALUES (false, 1, '1') ON CONFLICT DO NOTHING;
INSERT INTO child_table VALUES (false, 1, '1') ON CONFLICT (b) DO UPDATE SET b = true;
INSERT INTO child_table VALUES (0, 1, '1') ON CONFLICT DO NOTHING;
INSERT INTO child_table VALUES (0, 1, '1') ON CONFLICT (k) DO UPDATE SET k = 1;
TABLE child_table;
b | n | t
k | n | t
---+---+---
t | 1 | 1
1 | 1 | 1
(1 row)

INSERT INTO child_table VALUES (false, 2, '1') ON CONFLICT (b) DO UPDATE SET b = true;
ERROR: insert or update on table "child_table" violates foreign key constraint "child_table_t_n_fkey"
INSERT INTO child_table VALUES (0, 2, '1') ON CONFLICT (k) DO UPDATE SET k = 1;
ERROR: insert or update on table "child_table" violates foreign key constraint "fk"
DETAIL: Key (t, n)=(1, 2) is not present in table "parent_table".
INSERT INTO child_table VALUES (true, 2, '1') ON CONFLICT (b) DO UPDATE SET t = '2';
ERROR: insert or update on table "child_table" violates foreign key constraint "child_table_t_n_fkey"
INSERT INTO child_table VALUES (1, 2, '1') ON CONFLICT (k) DO UPDATE SET t = '2';
ERROR: insert or update on table "child_table" violates foreign key constraint "fk"
DETAIL: Key (t, n)=(2, 1) is not present in table "parent_table".
TABLE child_table;
b | n | t
k | n | t
---+---+---
t | 1 | 1
1 | 1 | 1
(1 row)

INSERT INTO parent_table VALUES (2, '2', false), (1, '1', false) ON CONFLICT (t, n) DO UPDATE SET t = EXCLUDED.t || EXCLUDED.t;
ERROR: update or delete on table "parent_table" violates foreign key constraint "fk" on table "child_table"
DETAIL: Key (t, n)=(1, 1) is still referenced from table "child_table".
INSERT INTO parent_table VALUES (2, '2', false) ON CONFLICT (t, n) DO UPDATE SET t = EXCLUDED.t || EXCLUDED.t;
SELECT * FROM parent_table ORDER BY t, n;
n | t | b
---+----+---
1 | 1 | t
2 | 22 | t
(2 rows)

TRUNCATE parent_table CASCADE;
NOTICE: truncate cascades to table "child_table"
INSERT INTO parent_table (n, t) SELECT g, '' FROM generate_series(0, 5) g;
INSERT INTO child_table (n, t, k) SELECT g, '', g FROM generate_series(0, 4) g;
-- NO ACTION
ALTER TABLE child_table DROP CONSTRAINT fk;
ALTER TABLE child_table ADD CONSTRAINT fk FOREIGN KEY (t, n) REFERENCES parent_table (t, n) ON DELETE NO ACTION ON UPDATE NO ACTION;
BEGIN;
INSERT INTO parent_table (n, t) SELECT g, '' FROM generate_series(0, 5) g
ON CONFLICT (n, t) DO UPDATE SET n = EXCLUDED.n - 1;
TABLE parent_table;
n | t | b
----+---+---
-1 | |
0 | |
1 | |
2 | |
3 | |
4 | |
(6 rows)

TABLE child_table;
k | n | t
---+---+---
0 | 0 |
1 | 1 |
2 | 2 |
3 | 3 |
4 | 4 |
(5 rows)

ABORT;
-- RESTRICT
ALTER TABLE child_table DROP CONSTRAINT fk;
ALTER TABLE child_table ADD CONSTRAINT fk FOREIGN KEY (t, n) REFERENCES parent_table (t, n) ON DELETE RESTRICT ON UPDATE RESTRICT;
BEGIN;
INSERT INTO parent_table (n, t) SELECT g, '' FROM generate_series(0, 5) g
ON CONFLICT (n, t) DO UPDATE SET n = EXCLUDED.n - 1;
ERROR: update or delete on table "parent_table" violates foreign key constraint "fk" on table "child_table"
DETAIL: Key (t, n)=(, 0) is still referenced from table "child_table".
TABLE parent_table;
ERROR: current transaction is aborted, commands ignored until end of transaction block
TABLE child_table;
ERROR: current transaction is aborted, commands ignored until end of transaction block
ABORT;
-- CASCADE + self-referential NO ACTION
ALTER TABLE child_table DROP CONSTRAINT fk;
ALTER TABLE child_table ADD CONSTRAINT fk FOREIGN KEY (t, n) REFERENCES parent_table (t, n) ON DELETE CASCADE ON UPDATE CASCADE;
ALTER TABLE child_table ADD CONSTRAINT u UNIQUE (n);
ALTER TABLE child_table ADD CONSTRAINT self_fk FOREIGN KEY (k) REFERENCES child_table (n) ON DELETE NO ACTION ON UPDATE NO ACTION;
BEGIN;
INSERT INTO parent_table (n, t) SELECT g, '' FROM generate_series(0, 5) g
ON CONFLICT (n, t) DO UPDATE SET n = EXCLUDED.n - 1;
ERROR: update or delete on table "child_table" violates foreign key constraint "self_fk" on table "child_table"
DETAIL: Key (n)=(4) is still referenced from table "child_table".
TABLE parent_table;
ERROR: current transaction is aborted, commands ignored until end of transaction block
TABLE child_table;
ERROR: current transaction is aborted, commands ignored until end of transaction block
ABORT;
-- CASCADE + self-referential CASCADE
ALTER TABLE child_table DROP CONSTRAINT self_fk;
ALTER TABLE child_table ADD CONSTRAINT self_fk FOREIGN KEY (k) REFERENCES child_table (n) ON DELETE CASCADE ON UPDATE CASCADE;
BEGIN;
INSERT INTO parent_table (n, t) SELECT g, '' FROM generate_series(0, 5) g
ON CONFLICT (n, t) DO UPDATE SET n = EXCLUDED.n - 1;
TABLE parent_table;
n | t | b
----+---+---
-1 | |
0 | |
1 | |
2 | |
3 | |
4 | |
(6 rows)

TABLE child_table;
k | n | t
----+----+---
-1 | -1 |
0 | 0 |
1 | 1 |
2 | 2 |
3 | 3 |
(5 rows)

ABORT;
-- CASCADE + no-update/no-delete self-referential
ALTER TABLE child_table DROP CONSTRAINT self_fk;
ALTER TABLE child_table DROP CONSTRAINT u;
ALTER TABLE child_table ADD CONSTRAINT self_fk FOREIGN KEY (n) REFERENCES child_table (k) ON DELETE CASCADE ON UPDATE CASCADE;
BEGIN;
INSERT INTO parent_table (n, t) VALUES (-1, 'other');
INSERT INTO child_table (n, t, k) VALUES (-1, 'other', -1);
INSERT INTO parent_table (n, t) SELECT g, '' FROM generate_series(0, 5) g
ON CONFLICT (n, t) DO UPDATE SET n = EXCLUDED.n - 1;
TABLE parent_table;
n | t | b
----+-------+---
-1 | |
0 | |
1 | |
2 | |
3 | |
4 | |
-1 | other |
(7 rows)

TABLE child_table;
k | n | t
----+----+-------
-1 | -1 | other
0 | -1 |
1 | 0 |
2 | 1 |
3 | 2 |
4 | 3 |
(6 rows)

ABORT;
--- GH-25070
CREATE TABLE main (a INT, b TEXT, PRIMARY KEY (a, b));
CREATE TABLE copy (a INT, b TEXT, PRIMARY KEY (b, a));
Expand Down
Loading

0 comments on commit 16f2042

Please sign in to comment.