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

[LB-1373] fixed precision for time type for PostgreSQL #1894

Merged
merged 7 commits into from Feb 25, 2022
Merged

Conversation

KushnirykOleh
Copy link
Contributor

@KushnirykOleh KushnirykOleh commented Jun 8, 2021

Environment

Liquibase Version: 4.3.5

Liquibase Integration & Version:

Liquibase Extension(s) & Version:

Database Vendor & Version: postgres 9-13

Operating System Type & Version: MacOS Big Sur 11.4

Pull Request Type

closes #1774
https://datical.atlassian.net/browse/LB-1373

  • Bug fix (non-breaking change which fixes an issue.)
  • Enhancement/New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

Precision in changeSet for Time Datatype was missing for PostreSQL in generated query (and in DB column properties respectfully)

Steps To Reproduce

create xml changelog with next changeset

<changeSet id="1" author="any">
        <createTable tableName="datatypes_test_table">
            <column name="time" type="time"/>
            <column name="timeewtz" type="time with time zone"/>
            <column name="time0" type="time (0)"/>
            <column name="timee0wtz" type="time (0) with time zone"/>
            <column name="time1" type="time (1)"/>
            <column name="time1wtz" type="time (1) with time zone"/>
            <column name="time2" type="time (2)"/>
            <column name="time2wtz" type="time (2) with time zone"/>
            <column name="time3" type="time (3)"/>
            <column name="time3wtz" type="time (3) with time zone"/>
            <column name="time4" type="time (4)"/>
            <column name="time4wtz" type="time (4) with time zone"/>
            <column name="time5" type="time (5)"/>
            <column name="time5wtz" type="time (5) with time zone"/>
            <column name="time6" type="time (6)"/>
            <column name="time6wtz" type="time (6) with time zone"/> 
        </createTable>
    </changeSet>

Run Liquibase updateSQL command to observe SQL generated for changeLog, notice precision for time is missing

CREATE TABLE datatypes_test_table
(
    time      TIME WITHOUT TIME ZONE,
    timeewtz  TIME WITH TIME ZONE,
    time0     TIME WITHOUT TIME ZONE,
    timee0wtz TIME WITH TIME ZONE,
    time1     TIME WITHOUT TIME ZONE,
    time1wtz  TIME WITH TIME ZONE,
    time2     TIME WITHOUT TIME ZONE,
    time2wtz  TIME WITH TIME ZONE,
    time3     TIME WITHOUT TIME ZONE,
    time3wtz  TIME WITH TIME ZONE,
    time4     TIME WITHOUT TIME ZONE,
    time4wtz  TIME WITH TIME ZONE,
    time5     TIME WITHOUT TIME ZONE,
    time5wtz  TIME WITH TIME ZONE,
    time6     TIME WITHOUT TIME ZONE,
    time6wtz  TIME WITH TIME ZONE
)

Actual Behavior

for mentioned changeSet Liquibase generated query like this

CREATE TABLE datatypes_test_table
(
    time      TIME WITHOUT TIME ZONE,
    timeewtz  TIME WITH TIME ZONE,
    time0     TIME WITHOUT TIME ZONE,
    timee0wtz TIME WITH TIME ZONE,
    time1     TIME WITHOUT TIME ZONE,
    time1wtz  TIME WITH TIME ZONE,
    time2     TIME WITHOUT TIME ZONE,
    time2wtz  TIME WITH TIME ZONE,
    time3     TIME WITHOUT TIME ZONE,
    time3wtz  TIME WITH TIME ZONE,
    time4     TIME WITHOUT TIME ZONE,
    time4wtz  TIME WITH TIME ZONE,
    time5     TIME WITHOUT TIME ZONE,
    time5wtz  TIME WITH TIME ZONE,
    time6     TIME WITHOUT TIME ZONE,
    time6wtz  TIME WITH TIME ZONE
)

with missing precision argument

Expected/Desired Behavior

With this fix for mentioned changeSet Liquibase generates query like this

CREATE TABLE datatypes_test_table
(
    time      time WITHOUT TIME ZONE,
    timeewtz  time WITH TIME ZONE,
    time0     time(0) WITHOUT TIME ZONE,
    timee0wtz time(0) WITH TIME ZONE,
    time1     time(1) WITHOUT TIME ZONE,
    time1wtz  time(1) WITH TIME ZONE,
    time2     time(2) WITHOUT TIME ZONE,
    time2wtz  time(2) WITH TIME ZONE,
    time3     time(3) WITHOUT TIME ZONE,
    time3wtz  time(3) WITH TIME ZONE,
    time4     time(4) WITHOUT TIME ZONE,
    time4wtz  time(4) WITH TIME ZONE,
    time5     time(5) WITHOUT TIME ZONE,
    time5wtz  time(5) WITH TIME ZONE,
    time6     time(6) WITHOUT TIME ZONE,
    time6wtz  time(6) WITH TIME ZONE
)

Run changeSet against PostgreSQL DB and check column properties - it also shows precision

Test Requirements (Liquibase Internal QA)

Prior to manual testing:

Manual Tests

The manual tests need only be executed against one version of Postgres.

Verify update-sql correctly generates Postgres SQL to create columns of datatype TIMEZONE with varying precision.

liquibase update-sql --changelog-file lb1798-changelog.xml

  • Columns have the expected precision on the time datatype; ie time(3).
  • Columns have the expected TIMEZONE setting; ie WITHOUT TIME ZONE

Verify a successful update of a Postgres database to create a table with TIMEZONE and precision datatype columns.

liquibase update --changelog-file lb1798-changelog.xml

  • There are no errors during update.
  • The table is created on the database.
    • Columns have the expected precision on the time datatype; ie time(3).
    • Columns have the expected TIMEZONE setting; ie WITHOUT TIME ZONE

Verify generate-changelog includes a createTable changeset with TIMEZONE datatype columns.

liquibase generate-changelog --changelog-file gcl-1798-changelog.xml

  • Columns have the expected precision on the time datatype; ie time(3).
  • Columns have the expected TIMEZONE setting; ie WITHOUT TIME ZONE

Automated Tests

  • No new functional automated tests are required for this ticket.
  • Test harness test already created.
  • Dev also added a unit test.

Fast Track PR Acceptance Checklist:

  • Build is successful and all new and existing tests pass
  • Added [Unit Test(s)]
  • Added [Test Harness Test(s)] - Added previously by Robert, test-harness helped to find this bug.

┆Issue is synchronized with this Jira Bug by Unito

@nvoxland nvoxland changed the base branch from master to 4.4.x June 21, 2021 19:14
@molivasdat
Copy link
Contributor

Thanks @KushnirykOleh for creating the fix.

@molivasdat molivasdat added this to To Do in Conditioning++ via automation Oct 29, 2021
@mariochampion mariochampion moved this from To Do to Ready for Handoff (In JIRA) in Conditioning++ Nov 8, 2021
@suryaaki2 suryaaki2 changed the base branch from 4.4.x to master November 12, 2021 19:55
@sync-by-unito
Copy link

sync-by-unito bot commented Nov 17, 2021

➤ Steven Massaro commented:

Erzsebet Carmean Nathan Voxland Surya Aki This ticket is dev complete. This PR fixes the issue and I’ve verified it. This is ready for QA sizing.

@sync-by-unito
Copy link

sync-by-unito bot commented Nov 19, 2021

➤ Erzsebet Carmean commented:

Liquibase Core: master/812/1e5e49/2021-11-18 13:23+0000, Pro: master/146/bfaa6a/2021-11-19T05:05:28Z] #812Build: https://github.com/liquibase/liquibase/actions/runs/1478003258 ( https://github.com/liquibase/liquibase/actions/runs/1478003258 )

Manual Tests

Verify update-sql correctly generates Postgres SQL to create columns of datatype TIMEZONE with varying precision.

liquibase update-sql --changelog-file lb1798-changelog.xml

  • Columns have the expected precision on the time datatype; ie time(3).
  • Columns have the expected TIMEZONE setting; ie WITHOUT TIME ZONE

Verify a successful update of a Postgres database to create a table with TIMEZONE and precision datatype columns.

liquibase update --changelog-file lb1798-changelog.xml

  • There are no errors during update.
  • The table is created on the database.
    • Columns have the expected precision on the time datatype; ie time(3).
    • Columns have the expected TIMEZONE setting; ie WITHOUT TIME ZONE

Verify generate-changelog includes a createTable changeset with TIMEZONE datatype columns.

liquibase generate-changelog --changelog-file gcl-1798-changelog.xml

  • Columns have the expected precision on the time datatype; ie time(3).
  • Columns have the expected TIMEZONE setting; ie WITHOUT TIME ZONE

@nvoxland nvoxland moved this from Ready for Handoff (In JIRA) to In Development in Conditioning++ Dec 7, 2021
@nvoxland
Copy link
Contributor

I fixed up the time precision handling

@XDelphiGrl
Copy link
Contributor

This PR addresses issues related to snapshotting precision for Postgres time and time with timezone datatypes.


  • Execute the liquibase/liquibase-test-harness test for liquibase-test-harness/datatypes.datetime.xml against:
    • Postgres 9 PASS
    • Postgres 9.5 PASS
    • Postgres 10 PASS
    • Postgres 11 PASS
    • Postgres 12 PASS
    • Postgres 13 PASS
    • Postgres 14 PASS

Verify update-sql correctly generates Postgres SQL to create columns of datatype TIMEZONE with varying precision. PASS
liquibase update-sql --changelog-file lb1798-changelog.xml

  • Columns have the expected precision on the time datatype; ie time(3). PASS
  • Columns have the expected TIMEZONE setting; ie WITHOUT TIME ZONE PASS
-- Changeset lb1798-changelog.xml::1::lb1798::Liquibase User
CREATE TABLE public.datatypes_test_table (time time WITHOUT TIME ZONE, timeewtz time WITH TIME ZONE, time0 time(0) WITHOUT TIME ZONE, timee0wtz time(0) WITH TIME ZONE, time1 time(1) WITHOUT TIME ZONE, time1wtz time(1) WITH TIME ZONE, time2 time(2) WITHOUT TIME ZONE, time2wtz time(2) WITH TIME ZONE, time3 time(3) WITHOUT TIME ZONE, time3wtz time(3) WITH TIME ZONE, time4 time(4) WITHOUT TIME ZONE, time4wtz time(4) WITH TIME ZONE, time5 time(5) WITHOUT TIME ZONE, time5wtz time(5) WITH TIME ZONE, time6 time(6) WITHOUT TIME ZONE, time6wtz time(6) WITH TIME ZONE);

Verify a successful update of a Postgres database to create a table with TIMEZONE and precision datatype columns. PASS
liquibase update --changelog-file lb1798-changelog.xml

  • There are no errors during update. PASS
  • The table is created on the database. PASS
    • Columns have the expected precision on the time datatype; ie time(3). PASS
    • Columns have the expected TIMEZONE setting; ie WITHOUT TIME ZONE PASS

Verify generate-changelog includes a createTable changeset with TIMEZONE datatype columns. PASS
liquibase generate-changelog --changelog-file gcl-1798-changelog.xml

  • Columns have the expected precision on the time datatype; ie time(3). PASS
  • Columns have the expected TIMEZONE setting; ie WITHOUT TIME ZONE PASS
<changeSet author="erz (generated)" id="1645634755981-1">
        <createTable tableName="datatypes_test_table">
            <column name="time" type="time(6) WITHOUT TIME ZONE"/>
            <column name="timeewtz" type="time(6) WITH TIME ZONE"/>
            <column name="time0" type="time(0) WITHOUT TIME ZONE"/>
            <column name="timee0wtz" type="time(0) WITH TIME ZONE"/>
            <column name="time1" type="time(1) WITHOUT TIME ZONE"/>
            <column name="time1wtz" type="time(1) WITH TIME ZONE"/>
            <column name="time2" type="time(2) WITHOUT TIME ZONE"/>
            <column name="time2wtz" type="time(2) WITH TIME ZONE"/>
            <column name="time3" type="time(3) WITHOUT TIME ZONE"/>
            <column name="time3wtz" type="time(3) WITH TIME ZONE"/>
            <column name="time4" type="time(4) WITHOUT TIME ZONE"/>
            <column name="time4wtz" type="time(4) WITH TIME ZONE"/>
            <column name="time5" type="time(5) WITHOUT TIME ZONE"/>
            <column name="time5wtz" type="time(5) WITH TIME ZONE"/>
            <column name="time6" type="time(6) WITHOUT TIME ZONE"/>
            <column name="time6wtz" type="time(6) WITH TIME ZONE"/>
        </createTable>
    </changeSet>

Test Environment
Liquibase Core: LB-1373/1611/89a691, Pro: master/645/8151fa
Postgres 12 (for manual testing)
Passing Functional Tests
Test-Harness PR

@XDelphiGrl XDelphiGrl self-assigned this Feb 23, 2022
@nvoxland nvoxland merged commit b6896fc into master Feb 25, 2022
Conditioning++ automation moved this from In Development to Done Feb 25, 2022
@nvoxland nvoxland deleted the LB-1373 branch February 25, 2022 14:33
@kataggart kataggart modified the milestones: On Deck, NEXT Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

SQL Generator bug. Arguments for TIME and INTERVAL datatypes.
9 participants