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

4464 fix generated default column value using in PostgreSQL and Oracle for char/clob data types #5202

Conversation

LonwoLonwo
Copy link
Contributor

@LonwoLonwo LonwoLonwo commented Nov 13, 2023

Impact

  • Bug fix (non-breaking change which fixes expected existing functionality)
  • Enhancement/New feature (adds functionality without impacting existing logic)
  • Breaking change (fix or feature that would cause existing functionality to change)

TypeBug - bug fixes

Description

So, I'm trying to fix the case from #4464

The first part of this PR - is avoiding duplication of the GENERATED ALWAYS AS part.
And second - an attempt to not add quotes around the function GENERATED ALWAYS AS for character/clob columns.
I understand that this is a terrible solution - trying to guess by the string beginning - if this default value is a function or not.
But I do not see other good options.
I tried to add to the ClobType#objectToSql, but it is not general enough.
So, I am open for discussion.

Things to be aware of

I'm not sure about the test part, also.

Things to worry about

Can users use the "GENERATED ALWAYS AS" syntax just for default values? I really do not know.

@MalloD12 MalloD12 self-assigned this Nov 23, 2023
@MalloD12 MalloD12 self-requested a review November 23, 2023 19:43
@@ -225,7 +225,7 @@ public void testChangeLogGenerationForTableWithGeneratedColumn() throws Exceptio
String textToTest = "GENERATED ALWAYS AS (QTY * PRICE)";

Scope.getCurrentScope().getSingleton(ExecutorService.class).getExecutor("jdbc", getDatabase()).execute(new RawSqlStatement(
String.format("CREATE TABLE GENERATED_COLUMN_TEST(QTY INT, PRICE INT, TOTALVALUE INT %s);", textToTest)));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Almost there :)
The assertion failed as it is not able to find our sql fragment. The generated SQL below is:

<column defaultValueComputed="GENERATED ALWAYS AS (&quot;QTY&quot;*&quot;PRICE&quot;)" name="TOTALVALUE" type="NUMBER(*, 0)"/>

So in my next commit I'm removing " from virtColumnDef variable... and seems it works. I wonder if this is the best solution @LonwoLonwo @MalloD12 , but at least it is working now :)

Copy link
Contributor

@MalloD12 MalloD12 left a comment

Choose a reason for hiding this comment

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

Approved.

Thanks @LonwoLonwo for this PR, for applying the required changes, and for your patience. Code changes look good to me. Build and all tests have been successfully executed.

It would be good if we can test running the Oracle test added in some different version than the one use for OracleIntegrationTest suite (oracle-xe:18-slim). @rberezen could you please do that when you review this PR? This test I'm talking about was failing on the mentioned version, but using the generated SQL and executing it in a newer version was successfully executed, so I would like to make sure that now it pass on version 18 it also works in newer versions.

@filipelautert filipelautert changed the title 4464 fix generated default column value using in PostgreSQL for char/clob data types 4464 fix generated default column value using in PostgreSQL and Oracle for char/clob data types Jan 5, 2024
@filipelautert filipelautert added this to the 1NEXT milestone Jan 12, 2024
@filipelautert filipelautert merged commit e5511b9 into liquibase:master Jan 12, 2024
30 of 31 checks passed
@LonwoLonwo LonwoLonwo deleted the 4464-fix-generated-default-column-value-using-in-PG branch February 27, 2024 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants