Skip to content

Commit 9d88571

Browse files
committed
[BACKPORT 2024.1][yugabyte#23708] YSQL: Fix bug in create colocated table
Summary: To reproduce the bug: (1) create a local cluster with the following command ``` ./bin/yb-ctl create --rf 1 --tserver_flags=report_ysql_ddl_txn_status_to_master=false ``` (2) Run the following SQL via ysqlsh ``` $ ./bin/ysqlsh ysqlsh (11.2-YB-2.23.1.0-b0) Type "help" for help. yugabyte=# CREATE DATABASE colocation_db colocation = true; CREATE DATABASE yugabyte=# \c colocation_db You are now connected to database "colocation_db" as user "yugabyte". colocation_db=# CREATE TABLE foo(id INT); ``` The `CREATE TABLE` statement hangs until timed out after 600 seconds. From the yb-master log, I saw the following error: ``` W0829 00:29:56.928934 2215 ysql_ddl_handler.cc:669] YsqlTableSchemaChecker failed: Illegal state (yb/master/ysql_ddl_handler.cc:174): Find Transaction Status for table 00004000000030008000000000004003.colocation.parent.tablename [id=00004000000030008000000000004003.colocation.parent.uuid] txn: f274cc3c-3ee2-4570-b089-6e254ba81168 failed with Not found (yb/common/schema.cc:602): Couldn't find column relkind in the schema ``` The bug is that in function `PgSchemaCheckerWithReadTime` we try to read `relkind` column unconditionally from a catalog table which can be either `pg_yb_tablegroup` or `pg_class`. But `relkind` is only a column of `pg_class`, not a column of `pg_yb_tablegroup`. I fixed the bug by only read `relkind` column from `pg_class`, and skip reading relkind column if the catalog table is `pg_yb_tablegroup`. Added a new unit test, which would fail without the fix. Jira: DB-12618 Original commit: 31a6d4f / D37635 NOTE: Merge conflicts resolved. The bug only existed in master branch so there is no code fix backport needed. However I backported the unit test TestCreateColocatedTable from the original diff, plus an additional unit test added in the master branch TestPartitionedTableSchemaVerification to 2024.1 to ensure we don't regress on 2024.1. Test Plan: ./yb_build.sh debug --cxx-test pg_ddl_atomicity-test --gtest_filter PgDdlAtomicityTest.TestCreateColocatedTable Reviewers: fizaa Reviewed By: fizaa Subscribers: yql, ybase Differential Revision: https://phorge.dev.yugabyte.com/D37709
1 parent e282b15 commit 9d88571

File tree

1 file changed

+53
-0
lines changed

1 file changed

+53
-0
lines changed

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1722,5 +1722,58 @@ TEST_F(PgDdlAtomicityMiniClusterTest, ClearTableMetadataOnDrop) {
17221722
ASSERT_FALSE(table->LockForRead()->has_ysql_ddl_txn_verifier_state());
17231723
}
17241724

1725+
// Test that the schema verification works correctly for partition tables and its children.
1726+
TEST_F(PgDdlAtomicityTest, TestPartitionedTableSchemaVerification) {
1727+
auto conn = ASSERT_RESULT(Connect());
1728+
auto client = ASSERT_RESULT(cluster_->CreateClient());
1729+
// Set report_ysql_ddl_txn_status_to_master to false, so that we can test the schema verification
1730+
// codepaths on master.
1731+
ASSERT_OK(cluster_->SetFlagOnTServers(
1732+
"report_ysql_ddl_txn_status_to_master", "false"));
1733+
// Create a parent partitioned table.
1734+
ASSERT_OK(conn.ExecuteFormat(
1735+
"CREATE TABLE test_parent (key INT PRIMARY KEY, value TEXT, num real, serialcol SERIAL) "
1736+
"PARTITION BY LIST(key)"));
1737+
// Create a child partition.
1738+
ASSERT_OK(conn.ExecuteFormat(
1739+
"CREATE TABLE test_child PARTITION OF test_parent FOR VALUES IN (1)"));
1740+
1741+
// Perform an unsuccessful alter table operation.
1742+
ASSERT_OK(conn.TestFailDdl("ALTER TABLE test_parent DROP COLUMN value"));
1743+
ASSERT_OK(cluster_->SetFlagOnMasters("TEST_pause_ddl_rollback", "true"));
1744+
ASSERT_OK(conn.ExecuteFormat("SET yb_test_fail_table_rewrite_after_creation=true"));
1745+
// Perform an unsuccessful alter table rewrite operation.
1746+
ASSERT_NOK(conn.ExecuteFormat("ALTER TABLE test_parent ADD COLUMN col1 SERIAL"));
1747+
1748+
ASSERT_EQ(ASSERT_RESULT(client->ListTables("test_parent")).size(), 1);
1749+
// Verify that the failed alter table rewrite operation created an orphaned child table.
1750+
ASSERT_EQ(ASSERT_RESULT(client->ListTables("test_child")).size(), 2);
1751+
ASSERT_OK(cluster_->SetFlagOnMasters("TEST_pause_ddl_rollback", "false"));
1752+
ASSERT_OK(conn.ExecuteFormat("SET yb_test_fail_table_rewrite_after_creation=false"));
1753+
ASSERT_OK(LoggedWaitFor([&]() -> Result<bool> {
1754+
return VERIFY_RESULT(client->ListTables("test_child")).size() == 1;
1755+
}, MonoDelta::FromSeconds(60), "Wait for orphaned child table to be cleaned up."));
1756+
1757+
// Perform a successful alter table operation.
1758+
ASSERT_OK(conn.ExecuteFormat("ALTER TABLE test_parent ADD COLUMN col1 int"));
1759+
// Perform a successful alter table rewrite operation.
1760+
ASSERT_OK(conn.ExecuteFormat("ALTER TABLE test_parent ADD COLUMN col2 SERIAL"));
1761+
1762+
// Perform a successful drop table operation.
1763+
ASSERT_OK(conn.ExecuteFormat("DROP TABLE test_parent"));
1764+
ASSERT_EQ(ASSERT_RESULT(client->ListTables("test_parent")).size(), 0);
1765+
ASSERT_EQ(ASSERT_RESULT(client->ListTables("test_child")).size(), 0);
1766+
}
1767+
1768+
TEST_F(PgDdlAtomicityTest, TestCreateColocatedTable) {
1769+
auto conn = ASSERT_RESULT(Connect());
1770+
auto client = ASSERT_RESULT(cluster_->CreateClient());
1771+
ASSERT_OK(cluster_->SetFlagOnTServers(
1772+
"report_ysql_ddl_txn_status_to_master", "false"));
1773+
ASSERT_OK(conn.Execute("CREATE DATABASE colocated_db colocation = true"));
1774+
conn = ASSERT_RESULT(ConnectToDB("colocated_db"));
1775+
ASSERT_OK(conn.Execute("CREATE TABLE foo(id int)"));
1776+
}
1777+
17251778
} // namespace pgwrapper
17261779
} // namespace yb

0 commit comments

Comments
 (0)