[#22356] YSQL: make yb_get_range_split_clause robust using PG TRY CAT…

…CH block

Summary:
`yb_get_range_split_clause` is used by `ysql_dump` to generate YSQL table and index SPLIT AT VALUES clause for multi-tablet range-partitioned relation during backup.
It fails to decode `GinNull` in GIN Index partition bounds.
With the change in this diff, `yb_get_range_split_clause` checks for the existence of `GinNull` in partition bounds. If `GinNull` exists in partition bounds,
an empty string is returned by `yb_get_range_split_clause`.
Also, there might be other unknown cases where partition bounds decoding can fail, which blocks YB backup.
Thus, protect the decoding in `yb_get_range_split_clause` with PG TRY CATCH block in case of decoding failure for the sake of YB backup.
If decoding fails, a warning is thrown and an empty string is returned by `yb_get_range_split_clause`.
Each time `yb_get_range_split_clause` returns an empty string for a multi-tablet range relation, during restore, the relation is repartitioned with correct partition boundaries.
Jira: DB-11260

Test Plan: ./yb_build.sh --cxx-test yb-backup-cross-feature-test --gtest_filter YBBackupTest.TestYSQLTabletSplitGINIndexWithGinNullInPartitionBounds

Reviewers: jason, mihnea

Reviewed By: jason

Subscribers: fizaa, ybase, yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D34965

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

@@ -2857,7 +2857,7 @@ getSplitPointsInfo(Oid relid, YBCPgTableDesc yb_tabledesc,
 				   YbTableProperties yb_table_properties,
 				   Oid *pkeys_atttypid,
 				   YBCPgSplitDatum *split_datums,
-				   bool *has_null)
+				   bool *has_null, bool *has_gin_null)
 {
 	Assert(yb_table_properties->num_tablets > 1);
 
@@ -2897,7 +2897,8 @@ getSplitPointsInfo(Oid relid, YBCPgTableDesc yb_tabledesc,
 
 	/* Get Split point values as Postgres datums */
 	HandleYBStatus(YBCGetSplitPoints(yb_tabledesc, type_entities,
-									 type_attrs_arr, split_datums, has_null));
+									 type_attrs_arr, split_datums, has_null,
+									 has_gin_null));
 }
 
 /*
@@ -2917,10 +2918,11 @@ rangeSplitClause(Oid relid, YBCPgTableDesc yb_tabledesc,
 	StringInfo prev_split_point = makeStringInfo();
 	StringInfo cur_split_point = makeStringInfo();
 	bool has_null = false;
+	bool has_gin_null = false;
 
 	/* Get Split point values as Postgres datum */
 	getSplitPointsInfo(relid, yb_tabledesc, yb_table_properties, pkeys_atttypid,
-					   split_datums, &has_null);
+					   split_datums, &has_null, &has_gin_null);
 
 	/*
 	 * Check for existence of NULL in split points.
@@ -2939,6 +2941,24 @@ rangeSplitClause(Oid relid, YBCPgTableDesc yb_tabledesc,
 		return;
 	}
 
+	/*
+	 * Check for existence of GinNull in split points.
+	 * We don't support (1) decoding GinNull into Postgres datum and
+	 * (2) specify GinNull in SPLIT AT VALUES clause for both
+	 * CREATE TABLE and CREATE INDEX.
+	 * However, split points of GIN indexes generated by tablet splitting can have
+	 * GinNull in its split points.
+	 */
+	if (has_gin_null)
+	{
+		ereport(WARNING,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("GinNull value present in split points"),
+				 errdetail("Specifying GinNull value in SPLIT AT VALUES clause is "
+						   "not supported.")));
+		return;
+	}
+
 	/* Process Datum and use StringInfo to accumulate c-string data */
 	appendStringInfoString(str, "SPLIT AT VALUES (");
 	for (int split_idx = 0; split_idx < num_splits; ++split_idx)
@@ -3020,10 +3040,11 @@ getRangeSplitPointsList(Oid relid, YBCPgTableDesc yb_tabledesc,
 	Oid pkeys_atttypid[num_range_key_columns];
 	YBCPgSplitDatum split_datums[num_splits * num_range_key_columns];
 	bool has_null;
+	bool has_gin_null;
 
 	/* Get Split point values as YBCPgSplitDatum. */
 	getSplitPointsInfo(relid, yb_tabledesc, yb_table_properties,
-					   pkeys_atttypid, split_datums, &has_null);
+					   pkeys_atttypid, split_datums, &has_null, &has_gin_null);
 
 	/* Construct PartitionRangeDatums split points list. */
 	for (int split_idx = 0; split_idx < num_splits; ++split_idx)
@@ -3099,10 +3120,39 @@ yb_get_range_split_clause(PG_FUNCTION_ARGS)
 	 * Get SPLIT AT VALUES clause for range relations with more than one tablet.
 	 * Skip one-tablet range-partition relations such that this function
 	 * return an empty string for them.
+	 *
+	 * For YB backup, if an error is thrown from a PG backend, ysql_dump will
+	 * exit, generate an empty YSQLDUMP file, and block YB backup workflow.
+	 * Currently, we don't have the functionality to adjust options used for 
+	 * ysql_dump on YBA and YBM, so we don't have a way to to turn on/off
+	 * a backup-related feature used in ysql_dump.
+	 * There are known cases which caused decoding of split points to fail in
+	 * the past and are handled specifically now.
+	 * (1) null values appear in split points after tablet splitting
+	 * (2) GinNull values appear in split points after tablet splitting
+	 * (3) duplicate split points appear after tablet splitting on an
+	 *     index's hidden column
+	 * Thus, for the sake of YB backup, protect the split point decoding with
+	 * TRY CATCH block in case decoding fails due to other unknown cases.
+	 * Return an empty string if decoding fails.
 	 */
 	initStringInfo(&str);
-	if (yb_table_properties.num_tablets > 1)
-		rangeSplitClause(relid, yb_tabledesc, &yb_table_properties, &str);
+	PG_TRY();
+	{
+		if (yb_table_properties.num_tablets > 1)
+			rangeSplitClause(relid, yb_tabledesc, &yb_table_properties, &str);
+	}
+	PG_CATCH();
+	{
+		ereport(WARNING,
+				(errcode(ERRCODE_WARNING),
+				 errmsg("cannot decode split point in SPLIT AT VALUES clause "
+						 "of relation with oid %u", relid),
+				 errdetail("Returning an empty string instead.")));
+		/* Empty string if split point decoding fails. */
+		resetStringInfo(&str);
+	}
+	PG_END_TRY();
 	range_split_clause = str.data;
 
 	PG_RETURN_CSTRING(range_split_clause);

src/yb/tools/yb-backup/yb-backup-cross-feature-test.cc

@@ -920,6 +920,96 @@ TEST_F_EX(YBBackupTest,
   LOG(INFO) << "Test finished: " << CURRENT_TEST_CASE_AND_TEST_NAME_STR();
 }
 
+// Test backup/restore when a range-partitioned GIN index undergoes manual tablet splitting and
+// GinNullItem become part of its tablets' partition bounds.
+// Any kind of GinNull (GinNullKey, GinEmptyItem, and GinNullItem) can be used for this test.
+// In the case where GinNull is part of one partition bound of a GIN index, during backup,
+// yb_get_range_split_clause fails to decode the partition bound, and YSQL_DUMP doesn't dump the
+// SPLIT AT clause of the index.
+// During restore, restoring snapshot to the index with different partition boundaries
+// should be detected and handled by repartitioning the index. See CatalogManager::RepartitionTable.
+// This test exercises that:
+// 1. create a table and a GIN index
+// 2. insert NULL data into the table
+// 3. split the GIN index into 2 tablets to make GinNull become part of partition bounds
+// 4. backup
+// 5. drop table
+// 6. restore
+TEST_F_EX(YBBackupTest,
+          YB_DISABLE_TEST_IN_SANITIZERS(TestYSQLTabletSplitGINIndexWithGinNullInPartitionBounds),
+          YBBackupTestNumTablets) {
+  const string table_name = "mytbl";
+  const string index_name = "my_gin_idx";
+
+  // Create table and index
+  ASSERT_NO_FATALS(CreateTable(Format("CREATE TABLE $0 (v tsvector)", table_name)));
+  ASSERT_NO_FATALS(CreateIndex(Format("CREATE INDEX $0 ON $1 USING ybgin(v)",
+                                      index_name, table_name)));
+
+  // Verify the index has only one tablet
+  auto tablets = ASSERT_RESULT(test_admin_client_->GetTabletLocations(default_db_, index_name));
+  LogTabletsInfo(tablets);
+  ASSERT_EQ(tablets.size(), 1);
+  // Use this function for side effects. It validates the begin and end partitions are empty.
+  ASSERT_OK(GetSplitPoints(tablets));
+
+  // Insert data
+  const string insert_null_sql = Format(R"#(
+    DO $$$$
+    BEGIN
+      FOR i in 1..1000 LOOP
+        INSERT INTO $0 VALUES (NULL);
+      END LOOP;
+    END $$$$;
+  )#", table_name);
+  ASSERT_NO_FATALS(RunPsqlCommand(insert_null_sql, "DO"));
+
+  // Flush index
+  auto index_id = ASSERT_RESULT(GetTableId(index_name, "pre-split"));
+  ASSERT_OK(client_->FlushTables({index_id}, false, 30, false));
+
+  // Split the GIN index into two tablets and wait for its split to complete.
+  // The splits make GinNull become part of its tablets' partition bounds:
+  // tablet-1 boundaries: [ "", (GinNullItem, <ybctid>) )
+  // tablet-2 boundaries: [ (GinNullItem, <ybctid>), "" )
+  const auto num_tablets = 2;
+  ASSERT_OK(test_admin_client_->SplitTabletAndWait(
+      default_db_, index_name, /* wait_for_parent_deletion */ true, tablets[0].tablet_id()));
+
+  // Verify that it has two tablets:
+  tablets = ASSERT_RESULT(test_admin_client_->GetTabletLocations(default_db_, index_name));
+  LogTabletsInfo(tablets);
+  ASSERT_EQ(tablets.size(), num_tablets);
+
+  // Verify GinNull is in the split point.
+  // 'v' represents the KeyEntryType for kGinNull.
+  auto split_points = ASSERT_RESULT(GetSplitPoints(tablets));
+  ASSERT_EQ(split_points.size(), num_tablets - 1);
+  ASSERT_EQ(split_points[0][0], 'v');
+
+  // Backup
+  const string backup_dir = GetTempDir("backup");
+  ASSERT_OK(RunBackupCommand(
+      {"--backup_location", backup_dir, "--keyspace", "ysql.yugabyte", "create"}));
+
+  // Drop the table
+  ASSERT_NO_FATALS(RunPsqlCommand(Format("DROP TABLE $0", table_name), "DROP TABLE"));
+
+  // Restore
+  ASSERT_OK(RunBackupCommand({"--backup_location", backup_dir, "restore"}));
+
+  // Validate
+  tablets = ASSERT_RESULT(test_admin_client_->GetTabletLocations(default_db_, index_name));
+  ASSERT_EQ(tablets.size(), 2);
+  auto post_restore_split_points = ASSERT_RESULT(GetSplitPoints(tablets));
+  // Rely on CatalogManager::RepartitionTable to repartition the GIN index with correct partition
+  // boundaries. Validate if repartition works correctly.
+  ASSERT_EQ(split_points,
+            post_restore_split_points);
+
+  LOG(INFO) << "Test finished: " << CURRENT_TEST_CASE_AND_TEST_NAME_STR();
+}
+
 // Test backup/restore on a colocated database which has colocated tables with tablespaces.
 // (Colocated tables support tablespaces only if preview flag
 // ysql_enable_colocated_tables_with_tablespaces is enabled.)

src/yb/yql/pggate/ybc_pggate.cc

@@ -229,7 +229,7 @@ Status GetSplitPoints(YBCPgTableDesc table_desc,
                       const YBCPgTypeEntity **type_entities,
                       YBCPgTypeAttrs *type_attrs_arr,
                       YBCPgSplitDatum *split_datums,
-                      bool *has_null) {
+                      bool *has_null, bool *has_gin_null) {
   CHECK(table_desc->IsRangePartitioned());
   const Schema& schema = table_desc->schema();
   size_t num_range_key_columns = table_desc->num_range_key_columns();
@@ -254,6 +254,9 @@ Status GetSplitPoints(YBCPgTableDesc table_desc,
       } else if (entry_type == dockv::KeyEntryType::kHighest) {
         column_bounds.consume_byte();
         split_datums[split_datum_idx].datum_kind = YB_YQL_DATUM_LIMIT_MAX;
+      } else if (entry_type == dockv::KeyEntryType::kGinNull) {
+        *has_gin_null = true;
+        return Status::OK();
       } else {
         table_row.Reset();
         RETURN_NOT_OK(dockv::PgKeyDecoder::DecodeEntry(
@@ -1087,9 +1090,9 @@ YBCStatus YBCGetSplitPoints(YBCPgTableDesc table_desc,
                             const YBCPgTypeEntity **type_entities,
                             YBCPgTypeAttrs *type_attrs_arr,
                             YBCPgSplitDatum *split_datums,
-                            bool *has_null) {
+                            bool *has_null, bool *has_gin_null) {
   return ToYBCStatus(GetSplitPoints(table_desc, type_entities, type_attrs_arr, split_datums,
-                                    has_null));
+                                    has_null, has_gin_null));
 }
 
 YBCStatus YBCPgTableExists(const YBCPgOid database_oid,

src/yb/yql/pggate/ybc_pggate.h

@@ -374,7 +374,7 @@ YBCStatus YBCGetSplitPoints(YBCPgTableDesc table_desc,
                             const YBCPgTypeEntity **type_entities,
                             YBCPgTypeAttrs *type_attrs_arr,
                             YBCPgSplitDatum *split_points,
-                            bool *has_null);
+                            bool *has_null, bool *has_gin_null);
 
 // INDEX -------------------------------------------------------------------------------------------
 // Create and drop index "database_name.schema_name.index_name()".