Skip to content

Commit

Permalink
[#24260] YSQL: ysql_dump will not emit separate CREATE INDEX statemen…
Browse files Browse the repository at this point in the history
…ts for unique constraints for partitioned tables

Summary:
Postgres does not dump the CREATE INDEX separately for
constraints, losing information present in CREATE INDEX like index
type (ASC/HASH/DESC). This is acceptable for PG since it only
supports a specific INDEX type for constraints (ASC). However, YB supports
such index types (ASC & HASH) for constraints.
To preserve this in a dump, YB emits the CREATE INDEX separately (#13603).

However, this causes an issue for partitioned tables (#24260), because
they do not support ALTER TABLE .. ADD CONSTRAINT .. USING INDEX and also the index on the parent
partitioned table is created in an invalid state in the first place before the index partitions are
attached in subsequent lines of the dump.

This diff changes it so that separate CREATE INDEX is not emitted for constraints on parent
partitioned tables.

Separately, also removed a YB_TODO related to emitting oids for primary key indexes for child partition tables.

The YB_TODO was concerned about emitting the oid for PRIMARY KEY clauses
of (child) partition tables because we had special handling for that
case earlier (we would omit such clauses for child tables when such keys
were inherited from the parent partition). However, this behavior was
changed by 180e7ae, so the YB_TODO is
no longer relevant. Also added a test case to make sure that oid is
emitted correctly for primary key clauses on child partitions when it
comes from the parent partition table.

Test Plan:
1. Add such a test case to the java unit test TestYbBackup - it fails before this fix and passes afterwards.
2. Added this test case to the java unit test TestYsqlDump. This test did not import the dump it was creating, so it doesn't catch the failure. I changed some of the tests to import the dumps created. This is not possible in all cases (ysql_dumpall emits `CREATE ROLE postgres;`) but I've fixed most of the tests to import the ysql dumps they generate.
3. Added a test case for primary keys on partition tables to verify that the index oid is correctly generated in this case.

Reviewers: myang, loginov, yguan

Reviewed By: myang

Subscribers: fizaa, yql

Differential Revision: https://phorge.dev.yugabyte.com/D40517
  • Loading branch information
iSignal committed Dec 26, 2024
1 parent f14450e commit 77f4fab
Show file tree
Hide file tree
Showing 13 changed files with 950 additions and 187 deletions.
47 changes: 47 additions & 0 deletions java/yb-pgsql/src/test/java/org/yb/pgsql/TestYbBackup.java
Original file line number Diff line number Diff line change
Expand Up @@ -2665,6 +2665,53 @@ public void testColocatedPartialIndex() throws Exception {
}
}

@Test
public void testPartitionsWithConstaints() throws Exception {
String backupDir = null;
try (Statement stmt = connection.createStatement()) {
// Create partitioned tables with a unique constraint
stmt.execute("CREATE TABLE part_uniq_const(v1 INT, v2 INT) PARTITION BY RANGE(v1);");
stmt.execute("CREATE TABLE part_uniq_const_50_100 PARTITION OF " +
"part_uniq_const FOR VALUES FROM (50) TO (100)");
stmt.execute("CREATE TABLE part_uniq_const_30_50 PARTITION OF " +
"part_uniq_const FOR VALUES FROM (30) TO (50);");
stmt.execute("CREATE TABLE part_uniq_const_default PARTITION OF part_uniq_const DEFAULT;");
stmt.execute("ALTER TABLE part_uniq_const ADD CONSTRAINT " +
"part_uniq_const_unique UNIQUE (v1, v2);");
stmt.execute("INSERT INTO part_uniq_const VALUES (51, 100), (31, 200), (1, 1000);");

backupDir = YBBackupUtil.getTempBackupDir();
String output = YBBackupUtil.runYbBackupCreate("--backup_location", backupDir,
"--keyspace", "ysql.yugabyte");
if (!TestUtils.useYbController()) {
backupDir = new JSONObject(output).getString("snapshot_url");
}
}

YBBackupUtil.runYbBackupRestore(backupDir, "--keyspace", "ysql.yb2");

try (Connection connection2 = getConnectionBuilder().withDatabase("yb2").connect();
Statement stmt = connection2.createStatement()) {
assertQuery(stmt, "SELECT * FROM part_uniq_const_default", new Row(1, 1000));
assertQuery(stmt, "SELECT * FROM part_uniq_const_50_100", new Row(51, 100));
assertQuery(stmt, "SELECT * FROM part_uniq_const_30_50", new Row(31, 200));

assertQuery(stmt,
"select tablename, indexname from pg_indexes where schemaname = 'public'",
new Row("part_uniq_const", "part_uniq_const_unique"),
new Row("part_uniq_const_30_50", "part_uniq_const_30_50_v1_v2_key"),
new Row("part_uniq_const_50_100", "part_uniq_const_50_100_v1_v2_key"),
new Row("part_uniq_const_default", "part_uniq_const_default_v1_v2_key")
);

assertQuery(stmt,
"select conname from pg_constraint where conrelid = 'part_uniq_const'::regclass::oid;",
new Row("part_uniq_const_unique")
);
}

}

/**
* Disabling geo partitioning tests when backups are run using YB Controller.
* This is because yb-controller-cli currently doesn't support such backups
Expand Down
Loading

0 comments on commit 77f4fab

Please sign in to comment.