diff --git a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgForeignKey.java b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgForeignKey.java index 814221156f84..62b711f3f346 100644 --- a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgForeignKey.java +++ b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgForeignKey.java @@ -322,4 +322,42 @@ public void testInsertConcurrencySerializableIsolation() throws Exception { testInsertConcurrency(Connection.TRANSACTION_SERIALIZABLE); } + @Test + public void testAddForeignKeyConcurrent() throws Exception { + try (Statement stmt = connection.createStatement(); + Connection ddlConnection = getConnectionBuilder().connect(); + Statement ddlStmt = ddlConnection.createStatement()) { + // Create two tables + ddlStmt.execute("CREATE TABLE parent(k int PRIMARY KEY)"); + ddlStmt.execute("CREATE TABLE child(k int)"); + + // Insert data into parent table + stmt.execute("INSERT INTO parent VALUES (1), (2), (3)"); + + // Start inserting data into child table + stmt.execute("INSERT INTO child VALUES (1)"); + stmt.execute("INSERT INTO child VALUES (2)"); + + // On DDL connection, add foreign key constraint + ddlStmt.execute("ALTER TABLE child ADD CONSTRAINT fk_child_parent " + + "FOREIGN KEY (k) REFERENCES parent(k)"); + + // Try inserting a valid row on DML connection + stmt.execute("INSERT INTO child VALUES (3)"); + + // Try inserting an invalid row on DML connection + runInvalidQuery(stmt, + "INSERT INTO child VALUES (4)", + "violates foreign key constraint \"fk_child_parent\""); + + // Try inserting an invalid row on the DDL connection + runInvalidQuery(ddlStmt, + "INSERT INTO child VALUES (4)", + "violates foreign key constraint \"fk_child_parent\""); + + // Verify the data + assertQuery(stmt, "SELECT k FROM child ORDER BY k", + new Row(1), new Row(2), new Row(3)); + } + } } diff --git a/src/postgres/src/backend/utils/cache/relcache.c b/src/postgres/src/backend/utils/cache/relcache.c index a53792c1e508..895f649d2997 100644 --- a/src/postgres/src/backend/utils/cache/relcache.c +++ b/src/postgres/src/backend/utils/cache/relcache.c @@ -90,12 +90,14 @@ #include "storage/smgr.h" #include "utils/array.h" #include "utils/builtins.h" +#include "utils/catcache.h" #include "utils/datum.h" #include "utils/fmgroids.h" #include "utils/inval.h" #include "utils/lsyscache.h" #include "utils/memutils.h" #include "utils/partcache.h" +#include "utils/relcache.h" #include "utils/relmapper.h" #include "utils/resowner_private.h" #include "utils/snapmgr.h" @@ -1907,7 +1909,7 @@ YbCompleteAttrProcessingImpl(const YbAttrProcessorState *state) { pfree(constr); relation->rd_att->constr = NULL; - } + } /* Fetch rules and triggers that affect this relation */ if (relation->rd_rel->relhasrules) @@ -2354,7 +2356,7 @@ YbGetPrefetchableTableInfoImpl(YbPFetchTable table) { RelationRelationId, RELOID, RELNAMENSP }; case YB_PFETCH_TABLE_PG_CONSTRAINT: return (YbPFetchTableInfo) - { ConstraintRelationId, CONSTROID, YB_INVALID_CACHE_ID }; + { ConstraintRelationId, CONSTROID, YBCONSTRAINTRELIDTYPIDNAME }; case YB_PFETCH_TABLE_PG_DATABASE: return (YbPFetchTableInfo) { DatabaseRelationId, DATABASEOID, YB_INVALID_CACHE_ID }; @@ -6145,6 +6147,8 @@ RelationGetFKeyList(Relation relation) HeapTuple htup; List *oldlist; MemoryContext oldcxt; + YbCatCListIterator iterator; + bool use_catcache; /* Quick exit if we already computed the list. */ if (relation->rd_fkeyvalid) @@ -6163,17 +6167,24 @@ RelationGetFKeyList(Relation relation) */ result = NIL; - /* Prepare to scan pg_constraint for entries having conrelid = this rel. */ - ScanKeyInit(&skey, + use_catcache = IsYugaByteEnabled() && yb_enable_fkey_catcache; + + if (use_catcache) + { + iterator = YbCatCListIteratorBegin(SearchSysCacheList1(YBCONSTRAINTRELIDTYPIDNAME, RelationGetRelid(relation))); + } + else + { + /* Prepare to scan pg_constraint for entries having conrelid = this rel. */ + ScanKeyInit(&skey, Anum_pg_constraint_conrelid, BTEqualStrategyNumber, F_OIDEQ, ObjectIdGetDatum(RelationGetRelid(relation))); - - conrel = heap_open(ConstraintRelationId, AccessShareLock); - conscan = systable_beginscan(conrel, ConstraintRelidTypidNameIndexId, true, + conrel = heap_open(ConstraintRelationId, AccessShareLock); + conscan = systable_beginscan(conrel, ConstraintRelidTypidNameIndexId, true, NULL, 1, &skey); - - while (HeapTupleIsValid(htup = systable_getnext(conscan))) + } + while (HeapTupleIsValid(htup = use_catcache ? YbCatCListIteratorGetNext(&iterator) : systable_getnext(conscan))) { Form_pg_constraint constraint = (Form_pg_constraint) GETSTRUCT(htup); ForeignKeyCacheInfo *info; @@ -6197,8 +6208,13 @@ RelationGetFKeyList(Relation relation) result = lappend(result, info); } - systable_endscan(conscan); - heap_close(conrel, AccessShareLock); + if (use_catcache) + YbCatCListIteratorFree(&iterator); + else + { + systable_endscan(conscan); + heap_close(conrel, AccessShareLock); + } /* Now save a copy of the completed list in the relcache entry. */ oldcxt = MemoryContextSwitchTo(CacheMemoryContext); diff --git a/src/postgres/src/backend/utils/cache/syscache.c b/src/postgres/src/backend/utils/cache/syscache.c index 4a123b8ae4cb..040980a21cca 100644 --- a/src/postgres/src/backend/utils/cache/syscache.c +++ b/src/postgres/src/backend/utils/cache/syscache.c @@ -1269,6 +1269,26 @@ YbPreloadCatalogCache(int cache_id, int idx_cache_id) } break; } + case CONSTROID: + { + /* + * Add a cache list for YBCONSTRAINTRELIDTYPIDNAME for lookup by conrelid only. + */ + if (!yb_enable_fkey_catcache) + { + is_add_to_list_required = false; + break; + } + if (dest_list) + { + HeapTuple ltp = llast(dest_list); + Form_pg_constraint ltp_struct = (Form_pg_constraint) GETSTRUCT(ltp); + Form_pg_constraint ntp_struct = (Form_pg_constraint) GETSTRUCT(ntp); + if (ntp_struct->conrelid != ltp_struct->conrelid) + dest_list = NIL; + } + break; + } default: is_add_to_list_required = false; break; @@ -1302,6 +1322,7 @@ YbPreloadCatalogCache(int cache_id, int idx_cache_id) switch(cache_id) { case PROCOID: + case CONSTROID: Assert(idx_cache); dest_cache = idx_cache; break; diff --git a/src/postgres/src/backend/utils/misc/guc.c b/src/postgres/src/backend/utils/misc/guc.c index 942fc1bb9871..843b7846cf7c 100644 --- a/src/postgres/src/backend/utils/misc/guc.c +++ b/src/postgres/src/backend/utils/misc/guc.c @@ -2395,6 +2395,17 @@ static struct config_bool ConfigureNamesBool[] = NULL, NULL, NULL }, + { + {"yb_enable_fkey_catcache", PGC_USERSET, DEVELOPER_OPTIONS, + gettext_noop("Enable preloading of foreign key information into the relation cache."), + NULL, + GUC_NOT_IN_SAMPLE + }, + &yb_enable_fkey_catcache, + true, + NULL, NULL, NULL + }, + /* End-of-list marker */ { {NULL, 0, 0, NULL, NULL}, NULL, false, NULL, NULL, NULL diff --git a/src/postgres/src/backend/utils/misc/pg_yb_utils.c b/src/postgres/src/backend/utils/misc/pg_yb_utils.c index 5c154c8e20cf..cde1850116e6 100644 --- a/src/postgres/src/backend/utils/misc/pg_yb_utils.c +++ b/src/postgres/src/backend/utils/misc/pg_yb_utils.c @@ -1346,6 +1346,7 @@ bool yb_enable_base_scans_cost_model = false; int yb_wait_for_backends_catalog_version_timeout = 5 * 60 * 1000; /* 5 min */ bool yb_prefer_bnl = false; bool yb_explain_hide_non_deterministic_fields = false; +bool yb_enable_fkey_catcache = true; //------------------------------------------------------------------------------ // YB Debug utils. diff --git a/src/postgres/src/include/pg_yb_utils.h b/src/postgres/src/include/pg_yb_utils.h index 989d9d7056a5..19910856225f 100644 --- a/src/postgres/src/include/pg_yb_utils.h +++ b/src/postgres/src/include/pg_yb_utils.h @@ -526,6 +526,11 @@ extern bool yb_prefer_bnl; */ extern bool yb_explain_hide_non_deterministic_fields; +/* + * Enable preloading of foreign key information into the relation cache. + */ +extern bool yb_enable_fkey_catcache; + //------------------------------------------------------------------------------ // GUC variables needed by YB via their YB pointers. extern int StatementTimeout; diff --git a/src/postgres/src/include/utils/guc.h b/src/postgres/src/include/utils/guc.h index 8c87f92723a0..cc8263f4c04a 100644 --- a/src/postgres/src/include/utils/guc.h +++ b/src/postgres/src/include/utils/guc.h @@ -265,6 +265,7 @@ extern bool yb_bnl_optimize_first_batch; extern bool yb_bnl_enable_hashing; extern bool yb_lock_pk_single_rpc; +extern bool yb_enable_fkey_catcache; extern int temp_file_limit; diff --git a/src/postgres/src/test/regress/expected/yb_dml.out b/src/postgres/src/test/regress/expected/yb_dml.out index 4bdbf71eb37c..cf28130cdbf5 100644 --- a/src/postgres/src/test/regress/expected/yb_dml.out +++ b/src/postgres/src/test/regress/expected/yb_dml.out @@ -18,3 +18,78 @@ EXPLAIN (COSTS OFF) INSERT INTO GH_22967 VALUES ((EXISTS(SELECT 1) in (SELECT tr (6 rows) INSERT INTO GH_22967 VALUES ((EXISTS(SELECT 1) in (SELECT true))::INT4), (-10); +-- Test that foreign key constraints are enforced +CREATE TABLE customers ( + customer_id SERIAL PRIMARY KEY, + name VARCHAR(100) NOT NULL, + email VARCHAR(100) UNIQUE NOT NULL +); +CREATE TABLE orders ( + order_id SERIAL PRIMARY KEY, + order_date DATE NOT NULL, + amount DECIMAL(10, 2) NOT NULL, + customer_id INTEGER NOT NULL, + FOREIGN KEY (customer_id) REFERENCES customers(customer_id) +); +INSERT INTO customers (name, email) VALUES +('Alice Johnson', 'alice@example.com'), +('Bob Smith', 'bob@example.com'); +INSERT INTO orders (order_date, amount, customer_id) VALUES +('2023-10-01', 250.00, 1), +('2023-10-02', 450.50, 2); +-- Attempt to insert an order with a non-existent customer +INSERT INTO orders (order_date, amount, customer_id) VALUES +('2023-10-03', 300.00, 3); +ERROR: insert or update on table "orders" violates foreign key constraint "orders_customer_id_fkey" +DETAIL: Key (customer_id)=(3) is not present in table "customers". +-- Attempt to delete a customer that still has orders +DELETE FROM customers WHERE customer_id = 2; +ERROR: update or delete on table "customers" violates foreign key constraint "orders_customer_id_fkey" on table "orders" +DETAIL: Key (customer_id)=(2) is still referenced from table "orders". +-- Test cascading deletes +DROP TABLE orders; +CREATE TABLE orders_cascade ( + order_id SERIAL PRIMARY KEY, + order_date DATE NOT NULL, + amount DECIMAL(10, 2) NOT NULL, + customer_id INTEGER NOT NULL, + FOREIGN KEY (customer_id) REFERENCES customers(customer_id) ON DELETE CASCADE +); +INSERT INTO orders_cascade (order_date, amount, customer_id) VALUES +('2023-10-01', 250.00, 1), +('2023-10-02', 450.50, 2); +DELETE FROM customers WHERE customer_id = 2; +SELECT * FROM orders_cascade; + order_id | order_date | amount | customer_id +----------+------------+--------+------------- + 1 | 10-01-2023 | 250.00 | 1 +(1 row) + +-- Test adding foreign key constraint using ALTER TABLE ADD CONSTRAINT +CREATE TABLE customers_test ( + customer_id SERIAL PRIMARY KEY, + name VARCHAR(100) NOT NULL +); +CREATE TABLE orders_test ( + order_id SERIAL PRIMARY KEY, + customer_id INTEGER, + name VARCHAR(100) NOT NULL +); +-- Add foreign key constraint using ALTER TABLE +ALTER TABLE orders_test +ADD CONSTRAINT fk_orders_customers +FOREIGN KEY (customer_id) REFERENCES customers_test(customer_id); +-- Insert valid data +INSERT INTO customers_test (name) VALUES ('Customer 1'), ('Customer 2'); +INSERT INTO orders_test (customer_id, name) VALUES (1, 'Order 1'), (2, 'Order 2'); +-- Attempt to insert a child with a non-existent customer +INSERT INTO orders_test (customer_id, name) VALUES (3, 'Order 3'); +ERROR: insert or update on table "orders_test" violates foreign key constraint "fk_orders_customers" +DETAIL: Key (customer_id)=(3) is not present in table "customers_test". +-- Attempt to delete a customer that still has orders +DELETE FROM customers_test WHERE customer_id = 2; +ERROR: update or delete on table "customers_test" violates foreign key constraint "fk_orders_customers" on table "orders_test" +DETAIL: Key (customer_id)=(2) is still referenced from table "orders_test". +-- Test that invalidation of the foreign key cache works +ALTER TABLE orders_test DROP CONSTRAINT fk_orders_customers; +INSERT INTO orders_test (customer_id, name) VALUES (3, 'Order 3'); diff --git a/src/postgres/src/test/regress/sql/yb_dml.sql b/src/postgres/src/test/regress/sql/yb_dml.sql index 8fd3188e9cc2..2be808ee7098 100644 --- a/src/postgres/src/test/regress/sql/yb_dml.sql +++ b/src/postgres/src/test/regress/sql/yb_dml.sql @@ -9,3 +9,81 @@ CREATE TABLE GH_22967 (k INT4 PRIMARY KEY); EXPLAIN (COSTS OFF) INSERT INTO GH_22967 VALUES ((EXISTS(SELECT 1) in (SELECT true))::INT4), (-10); INSERT INTO GH_22967 VALUES ((EXISTS(SELECT 1) in (SELECT true))::INT4), (-10); + + +-- Test that foreign key constraints are enforced +CREATE TABLE customers ( + customer_id SERIAL PRIMARY KEY, + name VARCHAR(100) NOT NULL, + email VARCHAR(100) UNIQUE NOT NULL +); + +CREATE TABLE orders ( + order_id SERIAL PRIMARY KEY, + order_date DATE NOT NULL, + amount DECIMAL(10, 2) NOT NULL, + customer_id INTEGER NOT NULL, + FOREIGN KEY (customer_id) REFERENCES customers(customer_id) +); + +INSERT INTO customers (name, email) VALUES +('Alice Johnson', 'alice@example.com'), +('Bob Smith', 'bob@example.com'); + +INSERT INTO orders (order_date, amount, customer_id) VALUES +('2023-10-01', 250.00, 1), +('2023-10-02', 450.50, 2); + +-- Attempt to insert an order with a non-existent customer +INSERT INTO orders (order_date, amount, customer_id) VALUES +('2023-10-03', 300.00, 3); + +-- Attempt to delete a customer that still has orders +DELETE FROM customers WHERE customer_id = 2; + +-- Test cascading deletes +DROP TABLE orders; +CREATE TABLE orders_cascade ( + order_id SERIAL PRIMARY KEY, + order_date DATE NOT NULL, + amount DECIMAL(10, 2) NOT NULL, + customer_id INTEGER NOT NULL, + FOREIGN KEY (customer_id) REFERENCES customers(customer_id) ON DELETE CASCADE +); +INSERT INTO orders_cascade (order_date, amount, customer_id) VALUES +('2023-10-01', 250.00, 1), +('2023-10-02', 450.50, 2); +DELETE FROM customers WHERE customer_id = 2; +SELECT * FROM orders_cascade; + + +-- Test adding foreign key constraint using ALTER TABLE ADD CONSTRAINT +CREATE TABLE customers_test ( + customer_id SERIAL PRIMARY KEY, + name VARCHAR(100) NOT NULL +); + +CREATE TABLE orders_test ( + order_id SERIAL PRIMARY KEY, + customer_id INTEGER, + name VARCHAR(100) NOT NULL +); + +-- Add foreign key constraint using ALTER TABLE +ALTER TABLE orders_test +ADD CONSTRAINT fk_orders_customers +FOREIGN KEY (customer_id) REFERENCES customers_test(customer_id); + +-- Insert valid data +INSERT INTO customers_test (name) VALUES ('Customer 1'), ('Customer 2'); +INSERT INTO orders_test (customer_id, name) VALUES (1, 'Order 1'), (2, 'Order 2'); + +-- Attempt to insert a child with a non-existent customer +INSERT INTO orders_test (customer_id, name) VALUES (3, 'Order 3'); + +-- Attempt to delete a customer that still has orders +DELETE FROM customers_test WHERE customer_id = 2; + +-- Test that invalidation of the foreign key cache works +ALTER TABLE orders_test DROP CONSTRAINT fk_orders_customers; +INSERT INTO orders_test (customer_id, name) VALUES (3, 'Order 3'); diff --git a/src/yb/yql/pgwrapper/pg_catalog_perf-test.cc b/src/yb/yql/pgwrapper/pg_catalog_perf-test.cc index 0534998eee31..9faff75869ee 100644 --- a/src/yb/yql/pgwrapper/pg_catalog_perf-test.cc +++ b/src/yb/yql/pgwrapper/pg_catalog_perf-test.cc @@ -606,4 +606,36 @@ TEST_F_EX(PgCatalogPerfTest, ASSERT_OK(conn.Execute("INSERT INTO t VALUES (my_func(1))")); } +TEST_F_EX(PgCatalogPerfTest, ForeignKeyRelcachePreloadTest, PgPreloadAdditionalCatBothTest) { + auto ddl_conn = ASSERT_RESULT(Connect()); + ASSERT_OK(ddl_conn.Execute("CREATE TABLE primary_table(k INT PRIMARY KEY)")); + ASSERT_OK( + ddl_conn.Execute("CREATE TABLE foreign_table(k INT, region INT) PARTITION BY LIST (region)")); + // Add 10 child tables for the foreign_table + for (int i = 1; i <= 10; ++i) { + ASSERT_OK(ddl_conn.ExecuteFormat( + "CREATE TABLE foreign_table_$0 PARTITION OF foreign_table FOR VALUES IN ($0)", i)); + } + ASSERT_OK( + ddl_conn.Execute("ALTER TABLE foreign_table ADD CONSTRAINT fk_foreign_table_primary_table " + "FOREIGN KEY (k) REFERENCES primary_table(k)")); + // Dummy connection to build the relcache init file + { auto unused_conn = ASSERT_RESULT(Connect()); } + + auto select_conn = ASSERT_RESULT(Connect()); + + const auto startup_rpc_count = ASSERT_RESULT(RPCCountOnStartUp()); + ASSERT_EQ(startup_rpc_count, 3); + + const auto select_rpc_count = + ASSERT_RESULT(RPCCountAfterCacheRefresh([&](PGConn* conn) -> Status { + RETURN_NOT_OK(conn->Fetch( + "SELECT * FROM primary_table JOIN foreign_table ON primary_table.k = foreign_table.k")); + return Status::OK(); + })); + // With yb_enable_fkey_catcache turned off, we would see more than 8 RPCs + // because we have to look up the foreign keys from master. + ASSERT_EQ(select_rpc_count, 8); +} + } // namespace yb::pgwrapper diff --git a/src/yb/yql/pgwrapper/pg_wrapper.cc b/src/yb/yql/pgwrapper/pg_wrapper.cc index 9d7a7b7325ae..a7a0de9b83ba 100644 --- a/src/yb/yql/pgwrapper/pg_wrapper.cc +++ b/src/yb/yql/pgwrapper/pg_wrapper.cc @@ -215,6 +215,9 @@ DEFINE_NON_RUNTIME_bool(enable_ysql_conn_mgr_stats, true, DEFINE_test_flag(bool, ysql_yb_enable_replication_commands, false, "Enable logical replication commands for Publication and Replication Slots"); +DEFINE_RUNTIME_PG_FLAG(bool, yb_enable_fkey_catcache, true, + "Enable preloading of foreign key information into the relation cache."); + static bool ValidateXclusterConsistencyLevel(const char* flagname, const std::string& value) { if (value != "database" && value != "tablet") { fprintf(