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

Add postgreSql Alter Column #4165

Merged
merged 7 commits into from
May 25, 2023

Conversation

griffio
Copy link
Contributor

@griffio griffio commented May 16, 2023

closes #4163

Add the missing ALTER [COLUMN] support for PostgreSql with enough features to be useful in migration scripts.

    ALTER [ COLUMN ] column_name [ SET DATA ] TYPE data_type [ USING expression ]
    ALTER [ COLUMN ] column_name SET DEFAULT expression
    ALTER [ COLUMN ] column_name DROP DEFAULT
    ALTER [ COLUMN ] column_name { SET | DROP } NOT NULL
    ALTER [ COLUMN ] column_name ADD GENERATED { ALWAYS | BY DEFAULT } AS IDENTITY 
    ALTER [ COLUMN ] column_name { SET GENERATED { ALWAYS | BY DEFAULT }
    ALTER [ COLUMN ] column_name DROP IDENTITY [ IF EXISTS ]
    ALTER [ COLUMN ] column_name SET ( attribute_option = value [, ... ] )

Extra πŸ—žοΈ

Add into gradle build src/test/integration-postgresql-migrations - this was never enabled ❓
Fix type that was preventing migrations test to run - possibly this was why 🦺
Add integration migration test for alter table alter column
Add AlterTableAlterColumnMixin not sure what else it needs ❓

@hfhbd
Copy link
Collaborator

hfhbd commented May 19, 2023

Could you add an integration test? I guess, you still need to add a mixing to alter the schema.

@griffio griffio force-pushed the add-postgresql-alter-column branch from ed44e4f to 8a31f93 Compare May 24, 2023 15:09
@griffio griffio marked this pull request as ready for review May 24, 2023 15:48
@griffio griffio force-pushed the add-postgresql-alter-column branch from ea05575 to c0ccfea Compare May 24, 2023 16:21
Basic syntax for alter column.
All changing types
Identity columns
Set and Drop NOT NULL

β€˜TYPE’ and β€˜DATA’ are not in the inherited grammar
Permutions on alter column
Add another default
fix postgres version to support features in test
postgres 9..6 is long unsupported
This was never enabled - perhaps due to failing test
This prevented the migration integration tests from running
Not sure what else to add?
@griffio griffio force-pushed the add-postgresql-alter-column branch from c0ccfea to 9db49c5 Compare May 25, 2023 07:30

@Test fun integrationTestsPostgreSqlMigrations() {
val runner = GradleRunner.create()
.withCommonConfiguration(File("src/test/integration-postgresql-migrations"))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

o_O Thanks.

Copy link
Collaborator

@hfhbd hfhbd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thank you.

@griffio
Copy link
Contributor Author

griffio commented May 25, 2023

@hfhbd can you re-trigger the build - without pushing another commit? thanks.
Otherwise - I will wait for time to rebase

Seems to be transient errors again πŸ‘Ί as the build time is quite long ⏳

build (ubuntu-latest, gradle-plugin-tests)
The hosted runner: GitHub Actions 10 lost communication with the server. Anything in your workflow that terminates the runner process, starves it for CPU/Memory, or blocks its network access can cause this error.

@hfhbd
Copy link
Collaborator

hfhbd commented May 25, 2023

Sure, I restarted the job.

@griffio
Copy link
Contributor Author

griffio commented May 25, 2023

Build runs fairly quickly locally πŸƒ πŸ…

On GitHub the most recent change was to use Linux to run these tests (that should have improved build times?) - maybe try again ❔

@AlecKazakova AlecKazakova merged commit 4c6e654 into cashapp:master May 25, 2023
6 checks passed
@griffio griffio deleted the add-postgresql-alter-column branch May 25, 2023 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for Alter Table Alter Column in PostgreSql Dialect
3 participants