Skip to content

Commit

Permalink
[BACKPORT 2.20][#24217] YSQL: fill definition of a shell type require…
Browse files Browse the repository at this point in the history
…s catalog version increment

Summary:
Steps to repro:

1. In one ysqlsh session:
```
yugabyte=# CREATE TYPE base_type; -- create shell type
CREATE TYPE
yugabyte=# CREATE FUNCTION base_type_in(cstring) RETURNS base_type
yugabyte-#    LANGUAGE internal IMMUTABLE STRICT PARALLEL SAFE AS 'int2in';
NOTICE:  return type base_type is only a shell
CREATE FUNCTION
yugabyte=# CREATE FUNCTION base_type_out(base_type) RETURNS cstring
yugabyte-#    LANGUAGE internal IMMUTABLE STRICT PARALLEL SAFE AS 'int2out';
NOTICE:  argument type base_type is only a shell
CREATE FUNCTION
yugabyte=# CREATE FUNCTION base_type_recv(internal) RETURNS base_type
yugabyte-#    LANGUAGE internal IMMUTABLE STRICT PARALLEL SAFE AS 'int2recv';
NOTICE:  return type base_type is only a shell
CREATE FUNCTION
yugabyte=# CREATE FUNCTION base_type_send(base_type) RETURNS bytea
yugabyte-#    LANGUAGE internal IMMUTABLE STRICT PARALLEL SAFE AS 'int2send';
NOTICE:  argument type base_type is only a shell
CREATE FUNCTION
```

2. In a second session:
```
yugabyte=# CREATE TABLE default_test (f1 base_type, f2 int);
ERROR:  type "base_type" is only a shell
LINE 1: CREATE TABLE default_test (f1 base_type, f2 int);
```
This ERROR is expected.

3. Back to the first session to fill the definition of the shell type

```
yugabyte=# CREATE TYPE base_type (
yugabyte(#    INPUT = base_type_in,
yugabyte(#    OUTPUT = base_type_out,
yugabyte(#    RECEIVE = base_type_recv,
yugabyte(#    SEND = base_type_send,
yugabyte(#    LIKE = smallint,
yugabyte(#    CATEGORY = 'N',
yugabyte(#    PREFERRED = FALSE,
yugabyte(#    DELIMITER = ',',
yugabyte(#    COLLATABLE = FALSE
yugabyte(# ); -- fill definition
CREATE TYPE
```

4. Back to the second session
```
yugabyte=# CREATE TABLE default_test (f1 base_type, f2 int);
ERROR:  type "base_type" is only a shell
LINE 1: CREATE TABLE default_test (f1 base_type, f2 int);
```

This ERROR is not expected because the first session has already filled the
definition of the shell type.

To fix this bug I added code to detect the update of an existing shell type and
make this DDL to increment the catalog version.

A new unit test is added.
Jira: DB-13103

Original commit: 17c45ff / D38575

Test Plan: ./yb_build.sh debug --cxx-test pgwrapper_pg_libpq-test --gtest_filter=PgLibPqTest.FillShellTypeDefinition

Reviewers: kfranz, mihnea

Reviewed By: kfranz

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D38697
  • Loading branch information
myang2021 committed Oct 4, 2024
1 parent c503f2e commit e809d4e
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 2 deletions.
8 changes: 8 additions & 0 deletions src/postgres/src/backend/catalog/pg_type.c
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,14 @@ TypeCreate(Oid newTypeOid,
replaces);

CatalogTupleUpdate(pg_type_desc, &tup->t_self, tup);
if (IsYugaByteEnabled())
/*
* Update existing shell type requires catalog version increment
* so that if a session has cached the shell type it can get
* refreshed to get the newly defined type.
*/
YBAddModificationAspects(true /* is_catalog_version_increment */,
false /* is_breaking_catalog_change */);

typeObjectId = HeapTupleGetOid(tup);

Expand Down
11 changes: 9 additions & 2 deletions src/postgres/src/backend/utils/misc/pg_yb_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -1515,6 +1515,13 @@ void YbSetIsGlobalDDL() {
ddl_transaction_state.is_global_ddl = true;
}

void
YBAddModificationAspects(bool is_catalog_version_increment,
bool is_breaking_catalog_change) {
ddl_transaction_state.is_catalog_version_increment |= is_catalog_version_increment;
ddl_transaction_state.is_breaking_catalog_change |= is_breaking_catalog_change;
}

void
YBIncrementDdlNestingLevel(bool is_catalog_version_increment,
bool is_breaking_catalog_change)
Expand All @@ -1533,8 +1540,8 @@ YBIncrementDdlNestingLevel(bool is_catalog_version_increment,
* The is_catalog_version_increment and is_breaking_catalog_change flags
* should only be set if it is not already true.
*/
ddl_transaction_state.is_catalog_version_increment |= is_catalog_version_increment;
ddl_transaction_state.is_breaking_catalog_change |= is_breaking_catalog_change;
YBAddModificationAspects(is_catalog_version_increment,
is_breaking_catalog_change);
}

void
Expand Down
2 changes: 2 additions & 0 deletions src/postgres/src/include/pg_yb_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,8 @@ bool YBIsInitDbAlreadyDone();

int YBGetDdlNestingLevel();
void YbSetIsGlobalDDL();
void YBAddModificationAspects(bool is_catalog_version_increment,
bool is_breaking_catalog_change);
void YBIncrementDdlNestingLevel(bool is_catalog_version_increment,
bool is_breaking_catalog_change);
void YBDecrementDdlNestingLevel();
Expand Down
44 changes: 44 additions & 0 deletions src/yb/yql/pgwrapper/pg_libpq-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4240,5 +4240,49 @@ TEST_F(PgPostmasterExitTest, SignalIdleBackendOnPostmasterDeath) {
ASSERT_OK(TestPostmasterExit());
}

TEST_F(PgLibPqTest, FillShellTypeDefinition) {
PGConn conn1 = ASSERT_RESULT(Connect());
PGConn conn2 = ASSERT_RESULT(Connect());
// Create a shell type.
ASSERT_OK(conn1.Execute(
R"#(
CREATE TYPE base_type; -- create shell type
CREATE FUNCTION base_type_in(cstring) RETURNS base_type
LANGUAGE internal IMMUTABLE STRICT PARALLEL SAFE AS 'int2in';
CREATE FUNCTION base_type_out(base_type) RETURNS cstring
LANGUAGE internal IMMUTABLE STRICT PARALLEL SAFE AS 'int2out';
CREATE FUNCTION base_type_recv(internal) RETURNS base_type
LANGUAGE internal IMMUTABLE STRICT PARALLEL SAFE AS 'int2recv';
CREATE FUNCTION base_type_send(base_type) RETURNS bytea
LANGUAGE internal IMMUTABLE STRICT PARALLEL SAFE AS 'int2send';
)#"));

// Using a shell type as column type is an error. Note that shell type is now
// cached in conn2.
ASSERT_NOK(conn2.Execute("CREATE TABLE default_test (f1 base_type, f2 int)"));

// Fill the definition of the shell type.
ASSERT_OK(conn1.Execute(
R"#(
CREATE TYPE base_type (
INPUT = base_type_in,
OUTPUT = base_type_out,
RECEIVE = base_type_recv,
SEND = base_type_send,
LIKE = smallint,
CATEGORY = 'N',
PREFERRED = FALSE,
DELIMITER = ',',
COLLATABLE = FALSE
); -- fill definition
)#"));
// Wait for heartbeat to propagate the new catalog version to trigger
// catalog cache refresh on conn2.
SleepFor(2s);

// Now that the shell type definition is filled, we should not get an error any more.
ASSERT_OK(conn2.Execute("CREATE TABLE default_test (f1 base_type, f2 int)"));
}

} // namespace pgwrapper
} // namespace yb

0 comments on commit e809d4e

Please sign in to comment.