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] CREATE TABLE without a primary key followed by ALTER TABLE adding a primary key should end up with one primary key #1104

Closed
mbautin opened this issue Apr 1, 2019 · 10 comments
Assignees
Labels
area/ysql Yugabyte SQL (YSQL) kind/enhancement This is an enhancement of an existing feature

Comments

@mbautin
Copy link
Contributor

mbautin commented Apr 1, 2019

  1. Currently, the PK specification has to be done inline as part of the CREATE TABLE. In YugabyteDB, the PK columns guides the primary dimension in which the table is laid out in storage (so that retrieval by those PK column lookups is as efficient).

  2. Creating the PK afterwards using an ALTER is not just syntax sugar work, but will actually involve reorganizing the on-disk structure of the table in a way that it is now organized by the PK as the primary dimension. So, with YugabyteDB it'll be advantageous if the system can be told upfront as part of the CREATE TABLE what the PK columns are.

Nevertheless, we should support ALTER TABLE to allow adding a PRIMARY KEY after the fact to be compatible with the broader ecosystem of tools.

@mbautin mbautin self-assigned this Apr 1, 2019
@mbautin mbautin changed the title [YSQL] Optimize CREATE TABLE without a primary key followed by ALTER TABLE adding a primary key should end up with one primary key [YSQL] CREATE TABLE without a primary key followed by ALTER TABLE adding a primary key should end up with one primary key Apr 2, 2019
@ndeodhar ndeodhar added the area/ysql Yugabyte SQL (YSQL) label Apr 3, 2019
@nocaway
Copy link
Contributor

nocaway commented Apr 3, 2019

Once Hector completes the work on partitioning, this enhancement can be done.

@ndeodhar
Copy link
Contributor

One possible approach can be to enhance index backfill for this: #448

Similar to creating index, when we run ALTER TABLE ADD PRIMARY KEY, we should:

  1. Create a new table with the PK.
  2. Any writes to old table should go to both old and new tables.
  3. Backfill new table with entries from old table.
  4. Switch all writes to only write to new table.
  5. Drop old table.
    cc: @amitanandaiyer

@ben-pr-p
Copy link

This currently makes restoring from pg_dump impossible, since pg_dump chooses to add all indexes and constraints (and therefore primary keys as well) in it's "post-data" phase, since they may slow down the copy that might be there

@ndeodhar
Copy link
Contributor

@ben-pr-p You can use ysql_dump utility to create the database dump and then restore it. This will take care of ensuring that schema and constraints are in YB format.
Note that you can use ysql_dump to take a dump of vanilla postgres DB as well.

@ben-pr-p
Copy link

ben-pr-p commented Sep 27, 2019

Oh, neat! I'm having difficulty finding installation instructions for ysql_dump – do you know where those are? Not seeing it in my ./bin/

@ndeodhar
Copy link
Contributor

ndeodhar commented Sep 27, 2019

It'll be in <yugabyte_install_dir>/postgres/bin folder. It's a modified version of pg_dump, so usage will be similar.

@frozenspider
Copy link
Contributor

frozenspider commented Jan 18, 2021

One possible approach can be to enhance index backfill for this: #448

Similar to creating index, when we run ALTER TABLE ADD PRIMARY KEY, we should:

1. Create a new table with the PK.

2. Any writes to old table should go to both old and new tables.

3. Backfill new table with entries from old table.

4. Switch all writes to only write to new table.

5. Drop old table.
   cc: @amitanandaiyer

Original PostgreSQL code takes AccessExclusiveLock on a table for ALTER TABLE ADD PRIMARY KEY, preventing concurrent writes.
Making this an online operation is a part of a larger endeavour, which is outside of this issue's scope and would be done separately at some point forward.

@frozenspider
Copy link
Contributor

frozenspider commented Feb 2, 2021

Note that the current implementation has a few limitations. Specifically:

  • We don't yet support concurrent DMLs while this ALTER is in progress.
  • The following cases are not supported as of now:
    • Colocated tables (including tablegroups).
    • Partitioned tables and table partitions.
    • Table having rules.
    • ALTER TABLE ADD CONSTRAINT PK USING INDEX
  • If the table was created using WITH (table_old = x) (beta feature), that OID is lost without notice.
  • There are minor mismatches between YB and PG error messages when migrating data fails (SQLSTATE is matching):
    • Null value in PK column
    • Duplicate PK

@uwejan
Copy link

uwejan commented Feb 8, 2021

Hello guys, good work. Is there an ETA for this feature? adding PK after column is really important with my project.

polarweasel pushed a commit to lizayugabyte/yugabyte-db that referenced this issue Mar 9, 2021
Summary:
Enables `ALTER TABLE ... ADD PRIMARY KEY (...)` syntax, allowing adding a primary key to a table that had none defined previously.

## Description

Since the primary key is an intrinsic part of a DocDB table, we cannot literally "add" one to an existing table.
Instead, this is implemented by renaming an old table, creating a new one in its stead, migrating data, and dropping an old table.
All of this is performed as a single DDL transaction, so failure on any step would roll back the whole thing.

Operation is logically divided onto the following phases:
1. Rename an old table
2. Create a replacement table
3. Copy CHECK and FK constraints
4. Copy table content (i.e. data itself).
5. Copy indexes (renaming old indexes in the process because of name conflicts)
6. Migrate sequences owned by old table
7. Copy triggers
8. Drop an old table

## Known limitations:

* The following cases are not supported as of now:
  * Colocated tables (including tablegroups).
  * Partitioned tables and table partitions.
  * Table having rules.
  * `ALTER TABLE ADD CONSTRAINT PK USING INDEX`
* If the table was created using `WITH (table_old = x)` (beta feature), that OID is lost without notice.
* There are minor mismatches between YB and PG when migrating data fails (`SQLSTATE` is matching):
  * Null value in PK column
  * Duplicate PK

---

Resolves yugabyte#1104 and yugabyte#6585

Test Plan:
ybd --java-test org.yb.pgsql.TestPgAlterTableAddPrimaryKey
ybd --java-test org.yb.pgsql.TestPgAlterTable#testRenameAndRecreate

Reviewers: neil, nicolas, dmitry, mihnea

Reviewed By: dmitry, mihnea

Subscribers: dsrinivasan, zyu, bogdan, yql

Differential Revision: https://phabricator.dev.yugabyte.com/D10057
@frozenspider
Copy link
Contributor

Sorry @uwejan, missed your comment. It's included in v2.6 onward.

frozenspider added a commit that referenced this issue Jul 29, 2022
Summary:
In #1104 / D10057 we implemented `ALTER TABLE ADD PRIMARY KEY` command.

This diff extends that work to implement `ALTER TABLE DROP CONSTRAINT pkey`.

We cannot drop primary key of:
* Partitioned tables and table partitions.
* Tables having rules.

Note that just like `ALTER TABLE ADD PRIMARY KEY`, this is designed for database setup and ORM migrations, and does not guarantee data safety when the table is being concurrently modified!

---

Resolves #8735

Test Plan:
ybd --java-test 'org.yb.pgsql.TestPgAlterTableChangePrimaryKey'

`TestPgAlterTableAddPrimaryKey` has been tweaked into `TestPgAlterTableChangePrimaryKey` which tests both adds and drops.

Reviewers: mihnea, tverona, neil, jason, myang, dmitry

Reviewed By: dmitry

Subscribers: yql

Differential Revision: https://phabricator.dev.yugabyte.com/D17939
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/enhancement This is an enhancement of an existing feature
Projects
None yet
Development

No branches or pull requests

7 participants