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] YB_AT_REWRITE_ALTER_PRIMARY_KEY clashes with other flags #22086

Closed
1 task done
jasonyb opened this issue Apr 19, 2024 · 0 comments
Closed
1 task done

[YSQL] YB_AT_REWRITE_ALTER_PRIMARY_KEY clashes with other flags #22086

jasonyb opened this issue Apr 19, 2024 · 0 comments
Assignees
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug priority/medium Medium priority issue status/awaiting-triage Issue awaiting triage

Comments

@jasonyb
Copy link
Contributor

jasonyb commented Apr 19, 2024

Jira Link: DB-11005

Description

Commit d5d7363 adds YB_AT_REWRITE_ALTER_PRIMARY_KEY in event_trigger.h:

#define AT_REWRITE_ALTER_PERSISTENCE    0x01
#define AT_REWRITE_DEFAULT_VAL          0x02
#define AT_REWRITE_COLUMN_REWRITE       0x04
#define AT_REWRITE_ALTER_OID            0x08
#define YB_AT_REWRITE_ALTER_PRIMARY_KEY 0x16

Notice 0x16 is decimal 22, binary 00010110, which conflicts with AT_REWRITE_DEFAULT_VAL and AT_REWRITE_COLUMN_REWRITE. This is used in the code like tab->rewrite |= YB_AT_REWRITE_ALTER_PRIMARY_KEY;, where tab->rewrite is int AlteredTableInfo.rewrite. I have no specific bug, but it looks like risky code.

Furthermore, upstream PG can continue to add more items here, so it is better practice for YB items to go high-to-low to avoid clashing during merge. For example, it could be set to 0x8000 which is comfortably within int boundaries and offers decent space for further fields to come in. See src/postgres/src/include/executor/executor.h EXEC_FLAG_YB_AGG_PARENT for example.

Backports may be needed.

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.
@jasonyb jasonyb added area/ysql Yugabyte SQL (YSQL) status/awaiting-triage Issue awaiting triage labels Apr 19, 2024
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue labels Apr 19, 2024
fizaaluthra added a commit that referenced this issue Apr 22, 2024
Summary:
Currently the decimal value of YB_AT_REWRITE_ALTER_PRIMARY_KEY is 22, which clashes with
AT_REWRITE_DEFAULT_VAL and AT_REWRITE_COLUMN_REWRITE.
Set YB_AT_REWRITE_ALTER_PRIMARY_KEY to 0x8000 to eliminate the clashing bits and provide
enough room for new flags that could be added in upstream PG.

Backport-to: 2024.1
Jira: DB-11005

Test Plan: Jenkins: test regex: .*TestPgRegressTable.*

Reviewers: jason

Reviewed By: jason

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D34347
fizaaluthra added a commit that referenced this issue Apr 22, 2024
…ER_PRIMARY_KEY

Summary:
Currently the decimal value of YB_AT_REWRITE_ALTER_PRIMARY_KEY is 22, which clashes with
AT_REWRITE_DEFAULT_VAL and AT_REWRITE_COLUMN_REWRITE.
Set YB_AT_REWRITE_ALTER_PRIMARY_KEY to 0x8000 to eliminate the clashing bits and provide
enough room for new flags that could be added in upstream PG.

Backport-to: 2024.1
Jira: DB-11005

Original commit: ddd6c8c / D34347

Test Plan: Jenkins: test regex: .*TestPgRegressTable.*

Reviewers: jason

Reviewed By: jason

Subscribers: yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D34381
svarnau pushed a commit that referenced this issue May 25, 2024
Summary:
Currently the decimal value of YB_AT_REWRITE_ALTER_PRIMARY_KEY is 22, which clashes with
AT_REWRITE_DEFAULT_VAL and AT_REWRITE_COLUMN_REWRITE.
Set YB_AT_REWRITE_ALTER_PRIMARY_KEY to 0x8000 to eliminate the clashing bits and provide
enough room for new flags that could be added in upstream PG.

Backport-to: 2024.1
Jira: DB-11005

Test Plan: Jenkins: test regex: .*TestPgRegressTable.*

Reviewers: jason

Reviewed By: jason

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D34347
ZhenYongFan pushed a commit to ZhenYongFan/yugabyte-db that referenced this issue Jun 15, 2024
…RITE_ALTER_PRIMARY_KEY

Summary:
Currently the decimal value of YB_AT_REWRITE_ALTER_PRIMARY_KEY is 22, which clashes with
AT_REWRITE_DEFAULT_VAL and AT_REWRITE_COLUMN_REWRITE.
Set YB_AT_REWRITE_ALTER_PRIMARY_KEY to 0x8000 to eliminate the clashing bits and provide
enough room for new flags that could be added in upstream PG.

Backport-to: 2024.1
Jira: DB-11005

Original commit: ddd6c8c / D34347

Test Plan: Jenkins: test regex: .*TestPgRegressTable.*

Reviewers: jason

Reviewed By: jason

Subscribers: yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D34381
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 status/awaiting-triage Issue awaiting triage
Projects
Status: Done
Development

No branches or pull requests

3 participants