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: don't persist table_oid reloption #9003

Open
jaki opened this issue Jun 21, 2021 · 4 comments
Open

ysql: don't persist table_oid reloption #9003

jaki opened this issue Jun 21, 2021 · 4 comments
Assignees
Labels
area/ysql Yugabyte SQL (YSQL) kind/disk-format-change This feature will change the on-disk format. kind/enhancement This is an enhancement of an existing feature priority/medium Medium priority issue

Comments

@jaki
Copy link
Contributor

jaki commented Jun 21, 2021

Jira Link: [DB-385](https://yugabyte.atlassian.net/browse/DB-385)
The table_oid reloption is not something that needs to be persisted to pg_class because it is only used at create time. In transformRelOptions, the oids reloption can be stripped out if ignoreOids is true (@frozenspider pointed this out to me). Similarly, we can always strip out table_oid options (and any others if appropriate).

DefineIndex calls transformRelOptions so that stmt->options holds the old options before transformation and reloptions holds the new options. Therefore, we can still read table_oid from the old options after transformRelOptions, though I think it may be cleaner to stop using it after transformRelOptions.

However, watch out for compatibility with existing clusters that may already have useless table_oid reloptions persisted.

Also, it could be useful to know that the table was created with the table_oid option, but I can't come up with any reasons. If there is a reason, this issue is invalid. cc: @hulien22

@jaki jaki added area/ysql Yugabyte SQL (YSQL) kind/disk-format-change This feature will change the on-disk format. labels Jun 21, 2021
@frozenspider
Copy link
Contributor

I think the valid reason to know this as of now if YSQL backup/restore for colocated tables (cc @OlegLoginov), where we actually rely on it.
We plan to change this soon though, decoupling table_oid and an internal colocation_id.

@OlegLoginov
Copy link
Contributor

Related issue: #7378

@m-iancu m-iancu added this to YQL-beta Nov 30, 2021
@tverona1 tverona1 changed the title ysql: don't persist table_oid reloption [Phase 2] ysql: don't persist table_oid reloption May 17, 2022
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue labels May 17, 2022
@tverona1
Copy link
Contributor

This is not relevant for colocation

@frozenspider
Copy link
Contributor

frozenspider commented May 31, 2022

UPD: The only area where this reloption is still used in YSQL upgrade (#10651).

We did remove it from colocation (replaced with colocation_id) and backup/restore (piggyback on PG binary upgrade instead)

@frozenspider frozenspider changed the title [Phase 2] ysql: don't persist table_oid reloption ysql: don't persist table_oid reloption May 31, 2022
@yugabyte-ci yugabyte-ci added kind/enhancement This is an enhancement of an existing feature and removed kind/bug This issue is a bug labels Sep 14, 2022
@yugabyte-ci yugabyte-ci assigned tverona1 and unassigned frozenspider Feb 28, 2023
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/disk-format-change This feature will change the on-disk format. kind/enhancement This is an enhancement of an existing feature priority/medium Medium priority issue
Projects
Status: No status
Development

No branches or pull requests

5 participants