Skip to content

Commit 95fdbab

Browse files
committed
[BACKPORT 2025.1][yugabyte#27201] YSQL: Make CREATE RULE bump catalog version
Summary: Original commit: 8a82a56 / D44279 Currently, `CREATE RULE` does not bump the catalog version under the assumption that creating a rule can only insert entries into system catalog tables but not update or delete them. However, this assumption is false; we need to bump the catalog version in order to maintain behavior parity with vanilla PG. ### What are rules? Rules allow users to specify additional commands that are to be performed automatically whenever an insert/update/delete is executed on a given table. For example: ``` CREATE RULE redirect_to_destination AS ON INSERT TO intermediate_table WHERE NEW.id > 25 DO INSTEAD INSERT INTO destination_table VALUES (NEW.id, NEW.name); ``` This rule redirects inserts from `intermediate_table` to `destination_table` instead when the inserted values fall into the given range. ### Rules internals PG stores rules in the `pg_rewrite` table. Additionally, there is a column in `pg_class` (`relhasrules`) that specifies whether the relation has rules on it. The rules and the relhasrules flag are stored in the relcache. When we build the relcache entry for a table, first we check if `relhasrules` is true. If so, we load the rules from `pg_rewrite`; otherwise, we just set the rules to null: ``` lang=c,name=relcache.c if (relation->rd_rel->relhasrules) RelationBuildRuleLock(relation); else { relation->rd_rules = NULL; relation->rd_rulescxt = NULL; } ``` We read the rules during query planning in `RewriteQuery()`: ``` lang=c,name=rewriteHandler.c locks = matchLocks(event, rt_entry_relation->rd_rules, result_relation, parsetree, &hasUpdate); product_orig_rt_length = list_length(parsetree->rtable); product_queries = fireRules(parsetree, result_relation, event, locks, &instead, &returning, &qual_product); ``` ### How the issue occurs The issue comes up during the following scenario: 1. On backend #1, we build the relcache entry for table `t` before there are any rules on `t`. This could be during relcache preloading, when we build the relcache entries for all user tables, or an ad-hoc load of `t`'s relcache entry. 2. Create the rule on `intermediate_table` on backend #2. This create a row in `pg_rewrite` and flips `relhasrules` to `true`, but the relcache entries in backend #1 stay the same because we don't bump the catalog version. 3. On backend #1, do an insert on `intermediate_table` that should trigger the rule. Because the relcache entry still has `rd_rules=NULL`, none of the rules will fire. Jira: DB-16686 Test Plan: ``` ./yb_build.sh release --sj --cxx-test pg_catalog_version-test --gtest-filter "*CreateRule*" ``` Reviewers: myang Reviewed By: myang Subscribers: yql Differential Revision: https://phorge.dev.yugabyte.com/D44435
1 parent 9a05f4e commit 95fdbab

2 files changed

Lines changed: 46 additions & 1 deletion

File tree

src/postgres/src/backend/utils/misc/pg_yb_utils.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3218,7 +3218,6 @@ YbGetDdlMode(PlannedStmt *pstmt, ProcessUtilityContext context)
32183218
case T_CreatedbStmt:
32193219
case T_DefineStmt: /* CREATE OPERATOR/AGGREGATE/COLLATION/etc */
32203220
case T_CommentStmt: /* COMMENT (create new comment) */
3221-
case T_RuleStmt: /* CREATE RULE */
32223221
case T_YbCreateProfileStmt:
32233222
/*
32243223
* Simple add objects are not breaking changes, and they do not even require
@@ -3228,6 +3227,16 @@ YbGetDdlMode(PlannedStmt *pstmt, ProcessUtilityContext context)
32283227
is_breaking_change = false;
32293228
break;
32303229

3230+
case T_RuleStmt: /* CREATE RULE */
3231+
/*
3232+
* CREATE RULE sets relhasrules to true in the table's pg_class entry.
3233+
* We need to increment the catalog version to ensure this change is
3234+
* picked up by other backends.
3235+
*/
3236+
is_version_increment = true;
3237+
is_breaking_change = false;
3238+
break;
3239+
32313240
case T_TruncateStmt:
32323241
/*
32333242
* TRUNCATE (on YB relations) using the old approach does not

src/yb/yql/pgwrapper/pg_catalog_version-test.cc

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2949,5 +2949,41 @@ TEST_F(PgCatalogVersionTest, CreateFunction) {
29492949
ASSERT_EQ(result, "dom");
29502950
}
29512951

2952+
// Tests that CREATE RULE increments the catalog version.
2953+
TEST_F(PgCatalogVersionTest, CreateRule) {
2954+
auto conn1 = ASSERT_RESULT(ConnectToDB(kYugabyteDatabase));
2955+
auto conn2 = ASSERT_RESULT(ConnectToDB(kYugabyteDatabase));
2956+
2957+
ASSERT_OK(conn1.Execute("CREATE TABLE source_table(id int, name text)"));
2958+
ASSERT_OK(conn1.Execute("CREATE TABLE intermediate_table(id int, name text)"));
2959+
ASSERT_OK(conn1.Execute("CREATE TABLE destination_table(id int, name text)"));
2960+
2961+
// First backend: rule that forwards some inserts from source_table -> intermediate_table
2962+
ASSERT_OK(conn1.Execute(R"(
2963+
CREATE RULE forward_to_intermediate AS ON INSERT TO source_table
2964+
WHERE NEW.id >= 20 AND NEW.id < 30 DO
2965+
INSERT INTO intermediate_table VALUES (NEW.id, NEW.name);
2966+
)"));
2967+
2968+
// Second backend: INSTEAD rule on intermediate_table that forwards to destination_table
2969+
ASSERT_OK(conn2.Execute(R"(
2970+
CREATE RULE redirect_to_destination AS ON INSERT TO intermediate_table
2971+
WHERE NEW.id > 25 DO INSTEAD
2972+
INSERT INTO destination_table VALUES (NEW.id, NEW.name);
2973+
)"));
2974+
2975+
// Back on backend 1: insert should ultimately land in destination_table, not intermediate_table
2976+
// Note that conn1 and conn2 are on the same node, so we don't need to wait for the heartbeat
2977+
// to propagate the new version of the rule.
2978+
ASSERT_OK(conn1.Execute("INSERT INTO intermediate_table VALUES (32,'custom entry')"));
2979+
2980+
auto intermediate_count =
2981+
ASSERT_RESULT(conn1.FetchRow<PGUint64>("SELECT count(*) FROM intermediate_table"));
2982+
auto destination_count =
2983+
ASSERT_RESULT(conn1.FetchRow<PGUint64>("SELECT count(*) FROM destination_table"));
2984+
ASSERT_EQ(intermediate_count, 0);
2985+
ASSERT_EQ(destination_count, 1);
2986+
}
2987+
29522988
} // namespace pgwrapper
29532989
} // namespace yb

0 commit comments

Comments
 (0)