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] Add support for ALTER TABLE ALTER COLUMN TYPE #1013

Closed
fizaaluthra opened this issue Mar 16, 2019 · 6 comments
Closed

[YSQL] Add support for ALTER TABLE ALTER COLUMN TYPE #1013

fizaaluthra opened this issue Mar 16, 2019 · 6 comments
Labels
area/ysql Yugabyte SQL (YSQL) kind/enhancement This is an enhancement of an existing feature priority/medium Medium priority issue
Projects
Milestone

Comments

@fizaaluthra
Copy link
Contributor

fizaaluthra commented Mar 16, 2019

Jira Link: DB-663

This issue tracks support for ALTER TABLE ALTER COLUMN TYPE that require on-disk changes.

Note that we already support alter column type that do not require on-disk changes. See: [YSQL] Support ALTER COLUMN type that does not require on-disk changes #4424

@fizaaluthra fizaaluthra added the kind/enhancement This is an enhancement of an existing feature label Mar 16, 2019
@fizaaluthra fizaaluthra changed the title [YSQL] Add support for ALTER TABLE ALTER TYPE [YSQL] Add support for ALTER TABLE ALTER COLUMN TYPE Mar 16, 2019
@m-iancu m-iancu added this to To do in YSQL via automation Apr 15, 2019
@kmuthukk kmuthukk added the area/ysql Yugabyte SQL (YSQL) label May 4, 2019
@dobesv
Copy link

dobesv commented May 8, 2020

I guess the workaround currently would be to rename the column to a temporary name, create the new column with the correct type, run an UPDATE to copy/convert the old values to the new column, then DROP the old column.

@ajcaldera1 ajcaldera1 added this to the v2.2.x milestone Aug 3, 2020
@ajcaldera1
Copy link
Contributor

Need this for Django support:

(.venv) $ python manage.py migrate
Operations to perform:
  Apply all migrations: admin, auth, contenttypes, sessions
Running migrations:
  Applying contenttypes.0001_initial... OK
  Applying auth.0001_initial... OK
  Applying admin.0001_initial... OK
  Applying admin.0002_logentry_remove_auto_add... OK
  Applying admin.0003_logentry_add_action_flag_choices... OK
  Applying contenttypes.0002_remove_content_type_name... OK
  Applying auth.0002_alter_permission_name_max_length...Traceback (most recent call last):
  File "/Users/acaldera/code/pyenv/.venv/lib/python3.8/site-packages/django/db/backends/utils.py", line 86, in _execute
    return self.cursor.execute(sql, params)
psycopg2.errors.FeatureNotSupported: ALTER TABLE ALTER column not supported yet
LINE 1: ALTER TABLE "auth_permission" ALTER COLUMN "name" TYPE varch...
                                      ^
HINT:  See https://github.com/YugaByte/yugabyte-db/issues/1124. Click '+' on the description to raise its priority

@kmuthukk
Copy link
Collaborator

kmuthukk commented Aug 23, 2020

Commit for issue #4424 addresses the most commonly used forms of this (such as the ones generated by Django framework) where the old and new column types are on-disk compatible.

@oneumyvakin
Copy link

Required for Laravel (Eloquent) support. Following statement

$table->string('field_name')->nullable()->change();

leads to

SQLSTATE[0A000]: Feature not supported: 7 ERROR:  This ALTER TABLE command is not yet supported. (SQL: ALTER TABLE table_name ALTER field_name TYPE VARCHAR(255))

@aimeos
Copy link

aimeos commented Dec 9, 2022

Also required for Doctrine DBAL: https://github.com/doctrine/dbal

mislam-yb added a commit that referenced this issue Apr 25, 2023
…ys in preparation for altering column type.

Summary:
A significant portion of the logic implemented for adding/dropping primary keys will be reused for altering column type as it will follow the same pattern. This diff has the following purposes:

1. Modularize code that is reusable by alter column type logic.
2. Modularize code specific to adding/dropping primary keys in order to reduce function sizes.
3. Generalize certain variables/parameters so that they are more readable and intuitive within the context of being modularized.

See [part 3](https://phabricator.dev.yugabyte.com/D24227) if more context is needed as to what changes are planned for altering the column type.

Test Plan:
The following tests involve adding or dropping primary keys:

1. TestPgAlterTableChangePrimaryKey
2. TestPgRegressTrigger
3. TestPgRegressPgStat
4. TestPgRegressIndex
5. TestPgRegressPgMiscIndependent
6. TestPgRegressWindow
7. TestPgRegressForeignData
8. TestPgRegressTable
9. TestPgRegressColocation

Their behavior must not change and their tests must pass.

Reviewers: tverona, dsrinivasan, jason, fizaa

Reviewed By: fizaa

Subscribers: yql

Differential Revision: https://phabricator.dev.yugabyte.com/D24286
mislam-yb added a commit that referenced this issue Apr 26, 2023
… to cloning a table.

Summary:
It's purpose is to re-arrange code that is specifically related to cloning a table to the bottom of tablecmds.c to make it more readable. In particular we do the following:

- Move all YbAT* functions to the bottom of the file
- Rearrange the 4 YbAT* functions that are already at the bottom of the file to be in order of use

See [part 3](https://phabricator.dev.yugabyte.com/D24227) if more context is needed as to what changes are planned for altering the column type.

Test Plan:
The following tests involve adding or dropping primary keys:

1. TestPgAlterTableChangePrimaryKey
2. TestPgRegressTrigger
3. TestPgRegressPgStat
4. TestPgRegressIndex
5. TestPgRegressPgMiscIndependent
6. TestPgRegressWindow
7. TestPgRegressForeignData
8. TestPgRegressTable
9. TestPgRegressColocation

Their behavior must not change and their tests must pass.

Reviewers: dsrinivasan, tverona, jason, fizaa

Reviewed By: fizaa

Subscribers: yql

Differential Revision: https://phabricator.dev.yugabyte.com/D24224
mislam-yb added a commit that referenced this issue Apr 26, 2023
… related to cloning a table."

This reverts commit 10bcfed.
mislam-yb added a commit that referenced this issue Apr 26, 2023
…ed to cloning a table.

Summary:
This PR is a clone of D24224.

It's purpose is to re-arrange code that is specifically related to cloning a table to the bottom of tablecmds.c to make it more readable. In particular we do the following:

- Move all YbAT* functions to the bottom of the file
- Rearrange the 4 YbAT* functions that are already at the bottom of the file to be in order of use

See part 3 if more context is needed as to what changes are planned for altering the column type.

Test Plan:
The following tests involve adding or dropping primary keys:

TestPgAlterTableChangePrimaryKey
TestPgRegressTrigger
TestPgRegressPgStat
TestPgRegressIndex
TestPgRegressPgMiscIndependent
TestPgRegressWindow
TestPgRegressForeignData
TestPgRegressTable
TestPgRegressColocation

Their behavior must not change and their tests must pass.

Reviewers: jason, fizaa

Reviewed By: fizaa

Subscribers: yql

Differential Revision: https://phabricator.dev.yugabyte.com/D24843
mislam-yb added a commit that referenced this issue Apr 28, 2023
Summary:
Currently, alter column statements that change the data type of a column are only valid if they don’t require on-disk changes. This is limited to increasing the size of a type, but not changing the type itself. For example, it is valid to change the type of a column from varchar(10) to varchar(20), but not int to varchar(20). However, this is a valid operation in postgres because there is a defined conversion from int to varchar. In postgres, changing a column type from varchar(20) to varchar(10) is also valid if there are no rows with a value of length greater than 10 in that column. The same principles hold for types other than varchar such as char, varbit and bit.

Alter column type statements should be supported for any types that have built-in or user-defined conversions (the latter would be via functions or casts). We will assume that this operation does not need to be online, meaning while the operation is being completed DML and DDL statements on the table will not be executed.

Alter table columns can also not require a rewrite, in which case we may skip cloning the table entirely and instead only update the metadata. This lines up with whether we required on-disk changes prior to this feature being supported, but they do not necessarily imply one another. See yb_collate_icu_utf8.out for examples of alter column type that does and doesn't require rewrites.

Test Plan:
The following existing tests use the alter column type feature:

1. TestPgRegressTablespaces
2. TestPgRegressExtendedStatistics
3. TestPgRegressAuthorization
4. TestPgRegressIndex
5. TestPgRegressPgMiscIndependent
6. TestPgForeignKey
7. TestPgRegressForeignData
8. TestPgRegressTable
9. TestPgRegressPartitions
10. TestPgRegressFeature
11. TestPgRegressTypesString
12. TestPgRegressAuthorization

Their outputs should be changed to reflect altered column types.

Further there is a new test, TestPgAlterColumnType, that tests various scenarios specific to altering the column type.

Reviewers: fizaa, jason

Reviewed By: jason

Subscribers: fizaa, yql

Differential Revision: https://phabricator.dev.yugabyte.com/D24227
@yugabyte-ci yugabyte-ci assigned mislam-yb and unassigned m-iancu Apr 28, 2023
YSQL automation moved this from In progress to Done Apr 28, 2023
@fizaaluthra
Copy link
Contributor Author

Known issues:
#17756 (ALTER TABLE ALTER COLUMN TYPE fails when on range key table with split options)
#18066 (ALTER TYPE fails after DROP COLUMN)

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 priority/medium Medium priority issue
Projects
Status: Done
YSQL
  
Done
Development

No branches or pull requests

10 participants