[#4873] YSQL: Support backup/restore for multi-tablet range-split rel…

…ations Summary: After 96beb9e, restoring a backup will re-partition relations to match the splits from their actual tablet snapshots if they differ from the pre-created ones. As a result, even if the split information from the ysql_dump file is missing or not accurate, restore should go through correctly. Therefore, as a stop-gap fix for backup-restore make the error when dumping a multi-tablet range-split relation (table or index) just a warning instead. This diff also fixes an issue in TestYSQLRangeSplitConstraint index-data validation code. Previously that data read was actually using the table instead of the index. Fully supporting range-split tables in ysql_dump (SPLIT AT clause) is still needed for regular ysql_dump export flow and may also improve performance of restore. So this will be addressed in a follow-up commit. Test Plan: YBBackupTest.TestYSQLRangeSplitConstraint org.yb.pgsql.TestYsqlDump Reviewers: yguan, jason Reviewed By: yguan, jason Subscribers: yql Differential Revision: https://phabricator.dev.yugabyte.com/D15700
yugabyte · Mar 1, 2022 · 3156825 · 3156825
1 parent 523361d
commit 3156825
Show file tree

Hide file tree

Showing 6 changed files with 262 additions and 20 deletions.
diff --git a/ent/src/yb/tools/yb-backup-test_ent.cc b/ent/src/yb/tools/yb-backup-test_ent.cc
@@ -894,9 +894,11 @@ TEST_F(YBBackupTest, YB_DISABLE_TEST_IN_SANITIZERS_OR_MAC(TestYCQLBackupWithDefi
 // constraint as 1 hash tablet. Restore should restore the unique constraint index as 3 tablets
 // since the tablet snapshot files are already split into 3 tablets.
 //
-// TODO(jason): enable test when issue #4873 ([YSQL] Support backup for pre-split multi-tablet range
-// tables) is fixed.
-TEST_F(YBBackupTest, YB_DISABLE_TEST(TestYSQLRangeSplitConstraint)) {
+// TODO(yguan): after the SPLIT AT clause is fully supported by ysql_dump this test needs to
+//              be revisited as the table may no longer need re-partitioning.
+//              Therefore, to exercise CatalogManager::RepartitionTable this test may need
+//              to be updated similar to TestYSQLManualTabletSplit.
+TEST_F(YBBackupTest, YB_DISABLE_TEST_IN_SANITIZERS_OR_MAC(TestYSQLRangeSplitConstraint)) {
   const string table_name = "mytbl";
   const string index_name = "myidx";
 
@@ -906,21 +908,29 @@ TEST_F(YBBackupTest, YB_DISABLE_TEST(TestYSQLRangeSplitConstraint)) {
   ASSERT_NO_FATALS(CreateIndex(
       Format("CREATE UNIQUE INDEX $0 ON $1 (v ASC) SPLIT AT VALUES (('foo'), ('qux'))",
              index_name, table_name)));
+
+  // Commenting out the ALTER .. ADD UNIQUE constraint case as this case is not supported.
+  // Vanilla Postgres disallows adding indexes with non-default (DESC) sort option as constraints.
+  // In YB we have added HASH and changed default (for first column) from ASC to HASH.
+  //
+  // See #11583 for details -- we should revisit this test after that is fixed.
+  /*
   ASSERT_NO_FATALS(RunPsqlCommand(
       Format("ALTER TABLE $0 ADD UNIQUE USING INDEX $1", table_name, index_name),
       "ALTER TABLE"));
+  */
 
   // Write data in each partition of the index.
   ASSERT_NO_FATALS(InsertRows(
-      Format("INSERT INTO $0 (v) VALUES ('bar'), ('jar'), ('tar')", table_name), 3));
+      Format("INSERT INTO $0 (v) VALUES ('tar'), ('bar'), ('jar')", table_name), 3));
   ASSERT_NO_FATALS(RunPsqlCommand(
-      Format("SELECT * FROM $0 ORDER BY k", table_name),
+      Format("SELECT * FROM $0 ORDER BY v", table_name),
       R"#(
          k |  v
         ---+-----
-         1 | bar
-         2 | jar
-         3 | tar
+         2 | bar
+         3 | jar
+         1 | tar
         (3 rows)
       )#"
   ));
@@ -940,13 +950,13 @@ TEST_F(YBBackupTest, YB_DISABLE_TEST(TestYSQLRangeSplitConstraint)) {
 
   // Verify data.
   ASSERT_NO_FATALS(RunPsqlCommand(
-      Format("SELECT * FROM $0 ORDER BY k", table_name),
+      Format("SELECT * FROM $0 ORDER BY v", table_name),
       R"#(
          k |  v
         ---+-----
-         1 | bar
-         2 | jar
-         3 | tar
+         2 | bar
+         3 | jar
+         1 | tar
         (3 rows)
       )#"
   ));

diff --git a/src/postgres/src/backend/utils/adt/ruleutils.c b/src/postgres/src/backend/utils/adt/ruleutils.c
@@ -1495,10 +1495,16 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
 				/* For range-partitioned tables */
 				if (yb_table_properties.num_tablets > 1)
 				{
-					ereport(ERROR,
+					ereport(WARNING,
 							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-							 errmsg("Exporting SPLIT clause for range-partitioned "
-									"tables is not yet supported")));
+							 errmsg("exporting SPLIT clause for range-split relations is not yet "
+									"supported"),
+							 errdetail("Index '%s' will be created with default (1) tablets "
+									   "instead of %" PRIu64 ".",
+									   generate_relation_name(indrelid, NIL),
+									   yb_table_properties.num_tablets),
+							 errhint("See https://github.com/yugabyte/yugabyte-db/issues/4873."
+									 " Click '+' on the description to raise its priority.")));
 				}
 			}
 		}

diff --git a/src/postgres/src/bin/pg_dump/pg_dump.c b/src/postgres/src/bin/pg_dump/pg_dump.c
@@ -16334,8 +16334,10 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
 			else if (properties.num_tablets > 1)
 			{
 				/* For range-table. */
-				fprintf(stderr, "Pre-split range tables are not supported yet.\n");
-				exit_nicely(1);
+				write_msg(NULL, "WARNING: exporting SPLIT clause for range-split relations is not "
+								"yet supported. Table '%s' will be created with default (1) "
+								"tablets instead of %" PRIu64 ".\n",
+						  qualrelname, properties.num_tablets);
 			}
 			/* else - single shard table - supported, no need to add anything */
 		}

diff --git a/src/postgres/src/test/regress/expected/yb_ysql_dump.out b/src/postgres/src/test/regress/expected/yb_ysql_dump.out
@@ -2,8 +2,8 @@
 -- YSQL database dump
 --
 
--- Dumped from database version 11.2-YB-2.11.3.0-b0
--- Dumped by ysql_dump version 11.2-YB-2.11.3.0-b0
+-- Dumped from database version 11.2-YB-2.13.0.0-b0
+-- Dumped by ysql_dump version 11.2-YB-2.13.0.0-b0
 
 SET yb_binary_restore = true;
 SET statement_timeout = 0;
@@ -616,6 +616,118 @@ TABLEGROUP grp2;
 
 ALTER TABLE public.tgroup_options_and_tgroup OWNER TO tablegroup_test_user;
 
+--
+-- Name: th1; 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('16512'::pg_catalog.oid);
+
+
+-- For binary upgrade, must preserve pg_type array oid
+SELECT pg_catalog.binary_upgrade_set_next_array_pg_type_oid('16511'::pg_catalog.oid);
+
+CREATE TABLE public.th1 (
+    a integer,
+    b text,
+    c double precision
+)
+SPLIT INTO 2 TABLETS;
+
+
+ALTER TABLE public.th1 OWNER TO yugabyte_test;
+
+--
+-- Name: th2; 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('16515'::pg_catalog.oid);
+
+
+-- For binary upgrade, must preserve pg_type array oid
+SELECT pg_catalog.binary_upgrade_set_next_array_pg_type_oid('16514'::pg_catalog.oid);
+
+CREATE TABLE public.th2 (
+    a integer NOT NULL,
+    b text NOT NULL,
+    c double precision,
+    CONSTRAINT th2_pkey PRIMARY KEY((a) HASH, b ASC)
+)
+SPLIT INTO 3 TABLETS;
+
+
+ALTER TABLE public.th2 OWNER TO yugabyte_test;
+
+--
+-- Name: th3; 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('16520'::pg_catalog.oid);
+
+
+-- For binary upgrade, must preserve pg_type array oid
+SELECT pg_catalog.binary_upgrade_set_next_array_pg_type_oid('16519'::pg_catalog.oid);
+
+CREATE TABLE public.th3 (
+    a integer NOT NULL,
+    b text NOT NULL,
+    c double precision,
+    CONSTRAINT th3_pkey PRIMARY KEY((a, b) HASH)
+)
+SPLIT INTO 4 TABLETS;
+
+
+ALTER TABLE public.th3 OWNER TO yugabyte_test;
+
+--
+-- Name: tr1; 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('16525'::pg_catalog.oid);
+
+
+-- For binary upgrade, must preserve pg_type array oid
+SELECT pg_catalog.binary_upgrade_set_next_array_pg_type_oid('16524'::pg_catalog.oid);
+
+CREATE TABLE public.tr1 (
+    a integer NOT NULL,
+    b text,
+    c double precision,
+    CONSTRAINT tr1_pkey PRIMARY KEY(a ASC)
+);
+
+
+ALTER TABLE public.tr1 OWNER TO yugabyte_test;
+
+--
+-- Name: tr2; 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('16530'::pg_catalog.oid);
+
+
+-- For binary upgrade, must preserve pg_type array oid
+SELECT pg_catalog.binary_upgrade_set_next_array_pg_type_oid('16529'::pg_catalog.oid);
+
+CREATE TABLE public.tr2 (
+    a integer NOT NULL,
+    b text NOT NULL,
+    c double precision NOT NULL,
+    CONSTRAINT tr2_pkey PRIMARY KEY(a DESC, b ASC, c DESC)
+);
+
+
+ALTER TABLE public.tr2 OWNER TO yugabyte_test;
+
 --
 -- Name: uaccount; Type: TABLE; Schema: public; Owner: regress_rls_alice
 --
@@ -845,6 +957,46 @@ COPY public.tgroup_options_and_tgroup (a) FROM stdin;
 \.
 
 
+--
+-- Data for Name: th1; Type: TABLE DATA; Schema: public; Owner: yugabyte_test
+--
+
+COPY public.th1 (a, b, c) FROM stdin;
+\.
+
+
+--
+-- Data for Name: th2; Type: TABLE DATA; Schema: public; Owner: yugabyte_test
+--
+
+COPY public.th2 (a, b, c) FROM stdin;
+\.
+
+
+--
+-- Data for Name: th3; Type: TABLE DATA; Schema: public; Owner: yugabyte_test
+--
+
+COPY public.th3 (a, b, c) FROM stdin;
+\.
+
+
+--
+-- Data for Name: tr1; Type: TABLE DATA; Schema: public; Owner: yugabyte_test
+--
+
+COPY public.tr1 (a, b, c) FROM stdin;
+\.
+
+
+--
+-- Data for Name: tr2; Type: TABLE DATA; Schema: public; Owner: yugabyte_test
+--
+
+COPY public.tr2 (a, b, c) FROM stdin;
+\.
+
+
 --
 -- Data for Name: uaccount; Type: TABLE DATA; Schema: public; Owner: regress_rls_alice
 --
@@ -902,6 +1054,34 @@ CREATE INDEX tbl8_idx4 ON public.tbl8 USING lsm (b DESC);
 CREATE INDEX tbl8_idx5 ON public.tbl8 USING lsm (c HASH) SPLIT INTO 3 TABLETS;
 
 
+--
+-- Name: th2_c_b_idx; Type: INDEX; Schema: public; Owner: yugabyte_test
+--
+
+CREATE INDEX th2_c_b_idx ON public.th2 USING lsm (c HASH, b DESC) SPLIT INTO 4 TABLETS;
+
+
+--
+-- Name: th3_c_b_idx; Type: INDEX; Schema: public; Owner: yugabyte_test
+--
+
+CREATE INDEX th3_c_b_idx ON public.th3 USING lsm ((c, b) HASH) SPLIT INTO 3 TABLETS;
+
+
+--
+-- Name: tr2_c_b_a_idx; Type: INDEX; Schema: public; Owner: yugabyte_test
+--
+
+CREATE INDEX tr2_c_b_a_idx ON public.tr2 USING lsm (c ASC, b DESC, a ASC);
+
+
+--
+-- Name: tr2_c_idx; Type: INDEX; Schema: public; Owner: yugabyte_test
+--
+
+CREATE INDEX tr2_c_idx ON public.tr2 USING lsm (c DESC);
+
+
 --
 -- Name: uaccount account_policies; Type: POLICY; Schema: public; Owner: regress_rls_alice
 --

diff --git a/src/postgres/src/test/regress/expected/yb_ysql_dump_verifier.out b/src/postgres/src/test/regress/expected/yb_ysql_dump_verifier.out
@@ -25,8 +25,13 @@
  public | tgroup_one_option_and_tgroup | table    | tablegroup_test_user
  public | tgroup_options               | table    | tablegroup_test_user
  public | tgroup_options_and_tgroup    | table    | tablegroup_test_user
+ public | th1                          | table    | yugabyte_test
+ public | th2                          | table    | yugabyte_test
+ public | th3                          | table    | yugabyte_test
+ public | tr1                          | table    | yugabyte_test
+ public | tr2                          | table    | yugabyte_test
  public | uaccount                     | table    | regress_rls_alice
-(25 rows)
+(30 rows)
 
        List of tablespaces
     Name    |  Owner   | Location 

diff --git a/src/postgres/src/test/regress/sql/yb_ysql_dump.sql b/src/postgres/src/test/regress/sql/yb_ysql_dump.sql
@@ -77,3 +77,42 @@ BEGIN;
     UPDATE pg_class SET reloptions = array_prepend('a=b', reloptions) WHERE relname = 'tgroup_after_options';
     UPDATE pg_class SET reloptions = array_prepend('a=b', reloptions) WHERE relname = 'tgroup_in_between_options';
 COMMIT;
+
+RESET SESSION AUTHORIZATION;
+
+------------------------------------------------
+-- Test table and index explicit splitting.
+------------------------------------------------
+
+------------------------------------
+-- Tables
+
+-- Table without a primary key
+CREATE TABLE th1 (a int, b text, c float) SPLIT INTO 2 TABLETS;
+
+-- Hash-partitioned table with range components
+CREATE TABLE th2 (a int, b text, c float, PRIMARY KEY (a HASH, b ASC)) SPLIT INTO 3 TABLETS;
+
+-- Hash-partitioned table without range components
+CREATE TABLE th3 (a int, b text, c float, PRIMARY KEY ((a, b) HASH)) SPLIT INTO 4 TABLETS;
+
+-- Range-partitioned table with single-column key
+CREATE TABLE tr1 (a int, b text, c float, PRIMARY KEY (a ASC)) SPLIT AT VALUES ((1), (100));
+
+-- Range-partitioned table with multi-column key
+CREATE TABLE tr2 (a int, b text, c float, PRIMARY KEY (a DESC, b ASC, c DESC)) SPLIT AT VALUES ((100, 'a', 2.5), (50, 'n'), (1, 'z', -5.12));
+
+------------------------------------
+-- Indexes
+
+-- Hash-partitioned table with range components
+CREATE INDEX on th2(c HASH, b DESC) SPLIT INTO 4 TABLETS;
+
+-- Hash-partitioned table without range components
+CREATE INDEX on th3((c, b) HASH) SPLIT INTO 3 TABLETS;
+
+-- Range-partitioned table with single-column key
+CREATE INDEX ON tr2(c DESC) SPLIT AT VALUES ((100.5), (1.5));
+
+-- Range-partitioned table with multi-column key
+CREATE INDEX ON tr2(c ASC, b DESC, a ASC) SPLIT AT VALUES ((-5.12, 'z', 1), (-0.75, 'l'), (2.5, 'a', 100));