[BACKPORT 2024.2][#24057] YSQL: ysql_dump generates invalid statement…

…s for colocated table unique constraint Summary: ysql_dump generates invalid statements for colocated table unique constraint. Example: ``` CREATE TABLE tbl (k INT, V INT UNQUE); ``` has the corresponding unique constraint related statements in ysql_dump output: ``` CREATE UNIQUE INDEX NONCONCURRENTLY tbl_v_key ON public.tbl USING lsm (v ASC) WITH (colocation_id=1976786484); ALTER TABLE ONLY public.tbl ADD CONSTRAINT tbl_v_key UNIQUE USING INDEX tbl_v_key WITH (colocation_id='1976786484'); ``` In the dump output, the `WITH (...` part after `USING INDEX` is invalid and redundant, and not supported by grammar. For ysql_dump, D15799 started outputing reloptions and colocation_id for a colocated table's unique constraint in the `ALTER TABLE ADD CONSTRAINT UNIQUE` statement. D19352 changed the logic of dumping unique constraints: `CREATE UNIQUE INDEX` and `ALTER TABLE ADD CONSTRAINT UNIQUE USING INDEX` are used to replace the original single `ALTER TABLE ADD CONSTRAINT UNIQUE` statement. D29672 started outputing the whole index definition including reloptions and YB table properties (e.g. colocaiton_id) in the `CREATE UNIQUE INDEX` statement. This diff solves the issue by removing the ysql_dump colocated table unique constraint change added by D15799 since the `CREATE UNIQUE INDEX` index definition introduced by D29672 is complete. In addition, a C++ backup & restore unit test is added, which would fail without the fix. Jira: DB-12949 Original commit: 326e883 / D38261 Test Plan: ./yb_build.sh --cxx-test yb-backup-test --gtest-filter YBBackupTest.TestColocatedTableUniqueConstraint ./yb_build.sh --java-test 'org.yb.pgsql.TestYsqlDump#ysqlDumpColocatedDB' Reviewers: mihnea, loginov, myang Reviewed By: myang Subscribers: yql, ybase, asrivastava Differential Revision: https://phorge.dev.yugabyte.com/D38402
yugabyte · Sep 30, 2024 · d43bee5 · d43bee5
1 parent b7fa018
commit d43bee5
Show file tree

Hide file tree

Showing 7 changed files with 146 additions and 45 deletions.
diff --git a/src/postgres/src/bin/pg_dump/pg_dump.c b/src/postgres/src/bin/pg_dump/pg_dump.c
@@ -868,7 +868,7 @@ main(int argc, char **argv)
 	pg_yb_tablegroup_exists = catalogTableExists(fout, "pg_yb_tablegroup");
 	pg_tablegroup_exists = catalogTableExists(fout, "pg_tablegroup");
 
-	/* 
+	/*
 	 * Cache (1) whether the dumped database is a colocated database and
 	 * (2) whether the dumped database is a legacy colocated database
 	 * in global variables.
@@ -17279,47 +17279,6 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
 				appendPQExpBufferChar(q, ')');
 			}
 
-			/* Get the table and index properties from YB, if relevant. */
-			YbTableProperties yb_table_properties = NULL;
-			YbTableProperties yb_index_properties = NULL;
-			if (dopt->include_yb_metadata &&
-				(coninfo->contype == 'u'))
-			{
-				yb_table_properties = (YbTableProperties) pg_malloc(sizeof(YbTablePropertiesData));
-				yb_index_properties = (YbTableProperties) pg_malloc(sizeof(YbTablePropertiesData));
-			}
-			PQExpBuffer yb_table_reloptions = createPQExpBuffer();
-			PQExpBuffer yb_index_reloptions = createPQExpBuffer();
-			getYbTablePropertiesAndReloptions(fout, yb_table_properties, yb_table_reloptions,
-				tbinfo->dobj.catId.oid, tbinfo->dobj.name, tbinfo->relkind);
-			getYbTablePropertiesAndReloptions(fout, yb_index_properties, yb_index_reloptions,
-				indxinfo->dobj.catId.oid, indxinfo->dobj.name, tbinfo->relkind);
-
-			/*
-			 * Issue #11600: if tablegroups mismatch between the table and its
-			 * constraint, we cannot currently replicate that.
-			 * We have to fail to prevent inconsistency upon yb_backup restore.
-			 */
-			if (dopt->include_yb_metadata &&
-				OidIsValid(yb_table_properties->tablegroup_oid) &&
-				yb_index_properties->tablegroup_oid != yb_table_properties->tablegroup_oid)
-			{
-				exit_horribly(NULL,
-							  "table %s and its constraint %s have mismatching tablegroups!\n"
-							  "This case cannot currently be handled, see issue "
-							  "https://github.com/yugabyte/yugabyte-db/issues/11600\n",
-							  tbinfo->dobj.name,
-							  coninfo->dobj.name);
-			}
-
-			YbAppendReloptions2(q, false /* newline_before*/,
-				indxinfo->indreloptions, "",
-				yb_index_reloptions->data, "",
-				fout);
-
-			destroyPQExpBuffer(yb_table_reloptions);
-			destroyPQExpBuffer(yb_index_reloptions);
-
 			if (coninfo->condeferrable)
 			{
 				appendPQExpBufferStr(q, " DEFERRABLE");

diff --git a/src/postgres/src/test/regress/data/yb_ysql_dump_colocated_database.data.sql b/src/postgres/src/test/regress/data/yb_ysql_dump_colocated_database.data.sql
@@ -227,6 +227,33 @@ SPLIT INTO 3 TABLETS;
     ALTER TABLE public.tbl4 OWNER TO yugabyte_test;
 \endif
 
+--
+-- Name: tbl5; Type: TABLE; Schema: public; Owner: yugabyte_test
+--
+
+
+-- For binary upgrade, must preserve pg_type oid
+SELECT pg_catalog.binary_upgrade_set_next_pg_type_oid('16418'::pg_catalog.oid);
+
+
+-- For binary upgrade, must preserve pg_type array oid
+SELECT pg_catalog.binary_upgrade_set_next_array_pg_type_oid('16417'::pg_catalog.oid);
+
+
+-- For binary upgrade, must preserve pg_class oids
+SELECT pg_catalog.binary_upgrade_set_next_heap_pg_class_oid('16416'::pg_catalog.oid);
+
+CREATE TABLE public.tbl5 (
+    k integer,
+    v integer
+)
+WITH (colocation_id='20005');
+
+
+\if :use_roles
+    ALTER TABLE public.tbl5 OWNER TO yugabyte_test;
+\endif
+
 --
 -- Data for Name: htest_1; Type: TABLE DATA; Schema: public; Owner: yugabyte_test
 --
@@ -267,6 +294,28 @@ COPY public.tbl4 (k, v, v2) FROM stdin;
 \.
 
 
+--
+-- Data for Name: tbl5; Type: TABLE DATA; Schema: public; Owner: yugabyte_test
+--
+
+COPY public.tbl5 (k, v) FROM stdin;
+\.
+
+
+--
+-- Name: tbl5 tbl5_v_key; Type: CONSTRAINT; Schema: public; Owner: yugabyte_test
+--
+
+
+-- For binary upgrade, must preserve pg_class oids
+SELECT pg_catalog.binary_upgrade_set_next_index_pg_class_oid('16419'::pg_catalog.oid);
+
+CREATE UNIQUE INDEX NONCONCURRENTLY tbl5_v_key ON public.tbl5 USING lsm (v ASC) WITH (colocation_id=20006);
+
+ALTER TABLE ONLY public.tbl5
+    ADD CONSTRAINT tbl5_v_key UNIQUE USING INDEX tbl5_v_key;
+
+
 --
 -- Name: partial_idx; Type: INDEX; Schema: public; Owner: yugabyte_test
 --

diff --git a/src/postgres/src/test/regress/data/yb_ysql_dump_legacy_colocated_database.data.sql b/src/postgres/src/test/regress/data/yb_ysql_dump_legacy_colocated_database.data.sql
@@ -224,6 +224,33 @@ SPLIT INTO 3 TABLETS;
     ALTER TABLE public.tbl4 OWNER TO yugabyte_test;
 \endif
 
+--
+-- Name: tbl5; Type: TABLE; Schema: public; Owner: yugabyte_test
+--
+
+
+-- For binary upgrade, must preserve pg_type oid
+SELECT pg_catalog.binary_upgrade_set_next_pg_type_oid('16417'::pg_catalog.oid);
+
+
+-- For binary upgrade, must preserve pg_type array oid
+SELECT pg_catalog.binary_upgrade_set_next_array_pg_type_oid('16416'::pg_catalog.oid);
+
+
+-- For binary upgrade, must preserve pg_class oids
+SELECT pg_catalog.binary_upgrade_set_next_heap_pg_class_oid('16415'::pg_catalog.oid);
+
+CREATE TABLE public.tbl5 (
+    k integer,
+    v integer
+)
+WITH (colocation_id='20005');
+
+
+\if :use_roles
+    ALTER TABLE public.tbl5 OWNER TO yugabyte_test;
+\endif
+
 --
 -- Data for Name: htest_1; Type: TABLE DATA; Schema: public; Owner: yugabyte_test
 --
@@ -264,6 +291,28 @@ COPY public.tbl4 (k, v, v2) FROM stdin;
 \.
 
 
+--
+-- Data for Name: tbl5; Type: TABLE DATA; Schema: public; Owner: yugabyte_test
+--
+
+COPY public.tbl5 (k, v) FROM stdin;
+\.
+
+
+--
+-- Name: tbl5 tbl5_v_key; Type: CONSTRAINT; Schema: public; Owner: yugabyte_test
+--
+
+
+-- For binary upgrade, must preserve pg_class oids
+SELECT pg_catalog.binary_upgrade_set_next_index_pg_class_oid('16418'::pg_catalog.oid);
+
+CREATE UNIQUE INDEX NONCONCURRENTLY tbl5_v_key ON public.tbl5 USING lsm (v ASC) WITH (colocation_id=20006);
+
+ALTER TABLE ONLY public.tbl5
+    ADD CONSTRAINT tbl5_v_key UNIQUE USING INDEX tbl5_v_key;
+
+
 --
 -- Name: partial_idx; Type: INDEX; Schema: public; Owner: yugabyte_test
 --

diff --git a/src/postgres/src/test/regress/expected/yb_ysql_dump_describe_colocated_database.out b/src/postgres/src/test/regress/expected/yb_ysql_dump_describe_colocated_database.out
@@ -7,7 +7,8 @@
  public | tbl2    | table | yugabyte_test
  public | tbl3    | table | yugabyte_test
  public | tbl4    | table | yugabyte_test
-(6 rows)
+ public | tbl5    | table | yugabyte_test
+(7 rows)
 
                              List of tablegroups
   Name   |  Owner   | Access privileges | Description | Tablespace | Options
@@ -26,5 +27,7 @@
  default    | postgres    |                   |                   |                  |               | partial_idx        | index | yugabyte_test |                 |
  default    | postgres    |                   |                   |                  |               | htest              | table | yugabyte_test |                 |
  default    | postgres    |                   |                   |                  |               | htest_1            | table | yugabyte_test |                 |
-(8 rows)
+ default    | postgres    |                   |                   |                  |               | tbl5               | table | yugabyte_test |                 |
+ default    | postgres    |                   |                   |                  |               | tbl5_v_key         | index | yugabyte_test |                 |
+(10 rows)
 
diff --git a/src/postgres/src/test/regress/expected/yb_ysql_dump_describe_legacy_colocated_database.out b/src/postgres/src/test/regress/expected/yb_ysql_dump_describe_legacy_colocated_database.out
@@ -7,7 +7,8 @@
  public | tbl2    | table | yugabyte_test
  public | tbl3    | table | yugabyte_test
  public | tbl4    | table | yugabyte_test
-(6 rows)
+ public | tbl5    | table | yugabyte_test
+(7 rows)
 
                           List of tablegroups
  Name | Owner | Access privileges | Description | Tablespace | Options

diff --git a/src/postgres/src/test/regress/sql/yb_ysql_dump_colocated_database.sql b/src/postgres/src/test/regress/sql/yb_ysql_dump_colocated_database.sql
@@ -34,3 +34,6 @@ WITH (colocation_id='123456');
 CREATE TABLE htest_1 PARTITION OF htest
 FOR VALUES WITH (modulus 2, remainder 0)
 WITH (colocation_id='234567');
+
+-- UNIQUE constraint
+CREATE TABLE tbl5 (k INT, v INT UNIQUE);
diff --git a/src/yb/tools/yb-backup/yb-backup-test.cc b/src/yb/tools/yb-backup/yb-backup-test.cc
@@ -933,5 +933,42 @@ TEST_F(YBBackupTest,
   LOG(INFO) << "Test finished: " << CURRENT_TEST_CASE_AND_TEST_NAME_STR();
 }
 
+TEST_F(YBBackupTest,
+       YB_DISABLE_TEST_IN_SANITIZERS(TestColocatedTableUniqueConstraint)) {
+  const string table_name = "mytbl";
+  // Create colocated database.
+  ASSERT_RESULT(RunPsqlCommand("CREATE DATABASE demo WITH colocation=true"));
+
+  // Create table.
+  SetDbName("demo");
+  ASSERT_NO_FATALS(CreateTable(Format("CREATE TABLE $0 (k INT, v INT UNIQUE)", table_name)));
+  ASSERT_NO_FATALS(InsertRows(
+      Format("INSERT INTO $0 SELECT i, i FROM generate_series(1, 1000) i", table_name), 1000));
+
+  const string backup_dir = GetTempDir("backup");
+  ASSERT_OK(RunBackupCommand(
+      {"--backup_location", backup_dir, "--keyspace", "ysql.demo", "create"}));
+
+  ASSERT_OK(RunBackupCommand(
+      {"--backup_location", backup_dir, "--keyspace", "ysql.demo_new", "restore"}));
+
+  SetDbName("demo_new");
+  ASSERT_NO_FATALS(RunPsqlCommand(Format(
+      "/*+IndexOnlyScan($0)*/ SELECT v FROM $1 WHERE v <= 5 ORDER BY v", table_name, table_name),
+      R"#(
+         v
+        ---
+         1
+         2
+         3
+         4
+         5
+        (5 rows)
+      )#"
+  ));
+
+  LOG(INFO) << "Test finished: " << CURRENT_TEST_CASE_AND_TEST_NAME_STR();
+}
+
 }  // namespace tools
 }  // namespace yb