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

feat: add support for function as default value #6441

Open
wants to merge 2 commits into
base: 4.1.x
Choose a base branch
from

Conversation

vencakrecl
Copy link

Q A
Type feature
Fixed issues

Summary

Added support for function as default value, for example id UUID NOT NULL DEFAULT gen_random_uuid()

Copy link
Member

@derrabus derrabus left a comment

Choose a reason for hiding this comment

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

Thank you. Please add functional tests that cover the creation, introspection and comparison of columns with this kind of default value. The test that you provided never hits a database, so I don't really know if your change actually works.

@derrabus
Copy link
Member

Also, please keep in mind that we only allow bugfixes on the 3.8.x branch. Your change however adds new functionality. For that, please target 4.1.x.

@derrabus derrabus changed the base branch from 3.8.x to 4.1.x June 20, 2024 07:37
@vencakrecl vencakrecl force-pushed the default-value branch 6 times, most recently from d61d985 to 24a2b4a Compare July 7, 2024 21:51
@vencakrecl
Copy link
Author

@derrabus I added a functional test, see 24a2b4a

@vencakrecl vencakrecl force-pushed the default-value branch 2 times, most recently from 6e4e4ea to fee7382 Compare July 8, 2024 07:25
@derrabus
Copy link
Member

derrabus commented Jul 8, 2024

Schema comparison is still not covered.

Comment on lines +58 to +62
self::assertTrue(
$sm->createComparator()
->compareTables($table, $onlineTable)
->isEmpty(),
);
Copy link
Member

Choose a reason for hiding this comment

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

This is good! Now, what happens if the introspected table…

  • … has no default?
  • … has a literal value as default?
  • … has NULL as default?
  • … has a different expression as default?

In that case, we would expect the schema manager to execute the proper SQL to migrate the deployed table and we need to verify that this migration performed successfully.

This has to be covered with tests as well.

Sames goes for the opposite direction: The schema deployed to the database has a column with a default expression and we want to change that to a literal, NULL, no default or a different expression.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants