Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[YSQL] Table rewrite operations fail when the colocated option on the table is specified using "0"/"1" #20914

Closed
1 task done
fizaaluthra opened this issue Feb 2, 2024 · 1 comment
Assignees
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug priority/medium Medium priority issue

Comments

@fizaaluthra
Copy link
Contributor

fizaaluthra commented Feb 2, 2024

Jira Link: DB-9896

Description

yugabyte=# create table x (t int) with (colocation=0);
CREATE TABLE
yugabyte=# alter table x add primary key(t);
ERROR: colocation requires a Boolean value

Applies to both the new and old rewrite approach.
The failure is emitted from defGetBoolean as it tries to read the integer value as a string and expects "true"/"false"/"on"/"off".

Issue Type

kind/bug

Warning: Please confirm that this issue does not contain any sensitive information

  • I confirm this issue does not contain any sensitive information.
@fizaaluthra fizaaluthra added area/ysql Yugabyte SQL (YSQL) status/awaiting-triage Issue awaiting triage labels Feb 2, 2024
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue labels Feb 2, 2024
fizaaluthra added a commit that referenced this issue Feb 5, 2024
…a colocated DB

Summary:
When re-creating the DocDB table as part of table rewrite, the new table is created with the
default colocation setting for the database itself. Fix this to instead use the colocation relopt
of the original table.

Additionally, add a work-around for GH issue #20914 in `xcluster_ysql_test_base.cc`.
Jira: DB-9843

Test Plan: `yb_matview` and `yb_alter_table`

Reviewers: dsrinivasan, xCluster, hsunder

Reviewed By: dsrinivasan

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D32127
@yugabyte-ci yugabyte-ci removed the status/awaiting-triage Issue awaiting triage label Feb 5, 2024
fizaaluthra added a commit that referenced this issue Feb 26, 2024
…es/matviews in a colocated DB

Summary:
Note: the changes in `yb_alter_table` and `xcluster_ysql_test_base.cc` are not backported
as this rewrite path is only used by matviews in 2.20.

When re-creating the DocDB table as part of table rewrite, the new table is created with the
default colocation setting for the database itself. Fix this to instead use the colocation relopt
of the original table.

Additionally, add a work-around for GH issue #20914 in `xcluster_ysql_test_base.cc`.
Jira: DB-9843

Original commit: 2847c04 / D32127

Test Plan: `yb_matview`

Reviewers: dsrinivasan

Reviewed By: dsrinivasan

Subscribers: yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D32362
fizaaluthra added a commit that referenced this issue Feb 26, 2024
…es/matviews in a colocated DB

Summary:
Note: the changes in `yb_alter_table` and `xcluster_ysql_test_base.cc` are not backported
as this rewrite path is only used by matviews in 2.18.

When re-creating the DocDB table as part of table rewrite, the new table is created with the
default colocation setting for the database itself. Fix this to instead use the colocation relopt
of the original table.

Additionally, add a work-around for GH issue #20914 in `xcluster_ysql_test_base.cc`.
Jira: DB-9843

Original commit: 2847c04 / D32127

Test Plan: `yb_matview`

Reviewers: dsrinivasan

Reviewed By: dsrinivasan

Subscribers: yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D32364
yifanguan added a commit that referenced this issue Mar 19, 2024
…ewrite and table partition

Summary:
This diff fixes two issues of the colocation option in table creation.
(1) Table rewrite
YB table rewrite operations like ALTER TABLE ADD PRIMARY KEY and REFRESH MATERIALIZED VIEW create a new YB table under the hood.
`reloptions` field in `pg_class` catalog table is read to maintain the property of tables.
Option value in `reloptions` syntax like `colocation=0` is parsed as integer type by the PG parser,
but when untransformed from `reloptions` field, its type is string.
To solve this issue, extend function `defGetBoolean` to process string `0` and `1` as false and true, respectively.

(2) Table partition
We have a check to prevent tablegroup from using with colocation option mainly serving as a grammar guard,
and this check is placed in an inappropriate place causing table partition creation to fail when the colocation option is specified.
Solve this issue by moving the check to an earlier place in the CREATE TABLE execution path.
Jira: DB-9268, DB-9896

Test Plan: ./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressColocation'

Reviewers: tverona, fizaa

Reviewed By: fizaa

Subscribers: yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D33001
yifanguan added a commit that referenced this issue Apr 2, 2024
…elated to table rewrite and table partition

Summary:
This diff fixes two issues of the colocation option in table creation.
(1) Table rewrite
YB table rewrite operations like ALTER TABLE ADD PRIMARY KEY and REFRESH MATERIALIZED VIEW create a new YB table under the hood.
`reloptions` field in `pg_class` catalog table is read to maintain the property of tables.
Option value in `reloptions` syntax like `colocation=0` is parsed as integer type by the PG parser,
but when untransformed from `reloptions` field, its type is string.
To solve this issue, extend function `defGetBoolean` to process string `0` and `1` as false and true, respectively.

(2) Table partition
We have a check to prevent tablegroup from using with colocation option mainly serving as a grammar guard,
and this check is placed in an inappropriate place causing table partition creation to fail when the colocation option is specified.
Solve this issue by moving the check to an earlier place in the CREATE TABLE execution path.
Jira: DB-9268, DB-9896

Original commit: 2cbb49d / D33001

Test Plan: ./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressColocation'

Reviewers: tverona, fizaa

Reviewed By: fizaa

Subscribers: yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D33344
yifanguan added a commit that referenced this issue Apr 2, 2024
…ated to table rewrite and table partition

Summary:
This diff fixes two issues of the colocation option in table creation.
(1) Table rewrite
YB table rewrite operations like ALTER TABLE ADD PRIMARY KEY and REFRESH MATERIALIZED VIEW create a new YB table under the hood.
`reloptions` field in `pg_class` catalog table is read to maintain the property of tables.
Option value in `reloptions` syntax like `colocation=0` is parsed as integer type by the PG parser,
but when untransformed from `reloptions` field, its type is string.
To solve this issue, extend function `defGetBoolean` to process string `0` and `1` as false and true, respectively.

(2) Table partition
We have a check to prevent tablegroup from using with colocation option mainly serving as a grammar guard,
and this check is placed in an inappropriate place causing table partition creation to fail when the colocation option is specified.
Solve this issue by moving the check to an earlier place in the CREATE TABLE execution path.
Jira: DB-9268, DB-9896

Original commit: 2cbb49d / D33001

Test Plan: ./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressColocation'

Reviewers: tverona, fizaa

Reviewed By: fizaa

Subscribers: yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D33348
yifanguan added a commit that referenced this issue Apr 2, 2024
…ated to table rewrite and table partition

Summary:
This diff fixes two issues of the colocation option in table creation.
(1) Table rewrite
YB table rewrite operations like ALTER TABLE ADD PRIMARY KEY and REFRESH MATERIALIZED VIEW create a new YB table under the hood.
`reloptions` field in `pg_class` catalog table is read to maintain the property of tables.
Option value in `reloptions` syntax like `colocation=0` is parsed as integer type by the PG parser,
but when untransformed from `reloptions` field, its type is string.
To solve this issue, extend function `defGetBoolean` to process string `0` and `1` as false and true, respectively.

(2) Table partition
We have a check to prevent tablegroup from using with colocation option mainly serving as a grammar guard,
and this check is placed in an inappropriate place causing table partition creation to fail when the colocation option is specified.
Solve this issue by moving the check to an earlier place in the CREATE TABLE execution path.
Jira: DB-9268, DB-9896

Original commit: 2cbb49d / D33001

Test Plan: ./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressColocation'

Reviewers: tverona, fizaa

Reviewed By: fizaa

Subscribers: yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D33346
@yifanguan
Copy link
Contributor

Issue resolved by commit 2cbb49d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug priority/medium Medium priority issue
Projects
None yet
Development

No branches or pull requests

3 participants