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

UniqueConstraintSnapshotGenerator - Add table name to OracleDB query #2139

Merged
merged 2 commits into from Jan 24, 2022

Conversation

wziebicki
Copy link
Contributor

@wziebicki wziebicki commented Oct 5, 2021

Environment

Liquibase Version: 4.5.0

Liquibase Integration & Version: <Pick one: CLI, maven, gradle, spring boot, servlet, etc.>

Liquibase Extension(s) & Version:

Database Vendor & Version:

Operating System Type & Version:

Pull Request Type

  • 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

The query that is generated when retrieving all keys (ORACLE DB) does not get a table name, while the code that creates the map for the cache gets this name from the query result, which is always null. When reading the cache, the table name is used which results in the cache always returning an empty result.

Steps To Reproduce

List the steps to reproduce the behavior.

  • standard diff generation operation that creates a snaphot of the reference and target base

Liquibase maven executions:

<executions>
    <execution>
        <id>generate-schema</id>
        <phase>generate-test-resources</phase>
        <goals>
            <goal>dropAll</goal>
            <goal>update</goal>
            <goal>diff</goal>
        </goals>
    </execution>
    <execution>
        <id>generate-data</id>
        <phase>generate-test-resources</phase>
        <goals>
            <goal>diff</goal>
        </goals>
        <configuration>
            <diffTypes>data</diffTypes>
        </configuration>
    </execution>
</executions>

Actual Behavior

Query result will preapre cache key like that:

row.get("CONSTRAINT_CONTAINER") + "_" + row.get("TABLE_NAME") + "_" + row.get("CONSTRAINT_NAME"); // prepare cache Table name is always null
...
String lookupKey = schema.getName() + "_" + table + "_" + example.getName(); // read cache by key table name is not null

in above row.get("TABLE_NAME") is always NULL, as a result we have the effect as if no keys (Unique constraints) exist, which causes that they are added to the diff file and they should not because in fact they exist in DB.

Expected/Desired Behavior

Table name should be added to sql query. Then cache key will contain it and later when cache is used and table name will be given unique constraint will be read from cache.


Dev Handoff Notes (Internal Use)

Links

Testing

  • Steps to Reproduce: Perform a snapshot against a database with unique constraints and see that it returns them
  • Guidance:
    • Impact: Only impacts oracle. Constraints were returned before, but this change fixes some of the internal caching we do

Dev Verification

Reviewed code and saw automated tests pass

Test Requirements (Liquibase Internal QA)

Setup

You will need two Oracle database instances.

On the first instance execute next sql script

CREATE TABLE test_table1
( id numeric(10) NOT NULL,
  first_name varchar2(50) NOT NULL,
  last_name varchar2(50),
  CONSTRAINT unique_id UNIQUE (id),
  CONSTRAINT unique_person UNIQUE (first_name, last_name)
);

INSERT ALL
  INTO test_table1 (id, first_name, last_name) VALUES (10, 'John', 'Doe')
  INTO test_table1 (id, first_name, last_name) VALUES (20, 'Jane', 'Dane')
SELECT * FROM dual;

CREATE TABLE test_table2
( id numeric(10) NOT NULL,
  first_name varchar2(50) NOT NULL,
  last_name varchar2(50),
  CONSTRAINT empty_id UNIQUE (id),
  CONSTRAINT empty_person UNIQUE (first_name, last_name)
);

Execute liquibase generate-changelog --changelog-file lb2206-generated.xml --diff-types tables,columns,uniqueconstraints,data

Execute uptade against second instance: liquibase generate-changelog --changelog-file lb2206-generated.xml

Make sure that second database instance was populated correctly.

Manual Tests

Verify diff shows no difference in unique constraints.

  • liquibase diff
  • no missing or unexpected unique constraints are found

Verify diff-changelog doesn’t ganerate any changes to unique constraints.

  • liquibase diff-changelog --changelog-file lb2206-diff.xml
  • generated changelog shows no difference between two instances

Automated Tests

No new functional tests required for this fix.

┆Issue is synchronized with this Jira Bug by Unito

The query that is generated when retrieving all keys does not get a table name, while the code that creates the map for  the cache gets this name from the query result, which is always null. When reading the cache, the table name is used which  results in the cache always returning an empty result.
@molivasdat
Copy link
Contributor

Hi @wziebicki Thanks for creating this PR. We will add it to the list of issues that we are processing.

A member of the Liquibase team will take a look at your contribution and may suggest:

The PR will be prioritized according to our internal development and testing capacity.

We’ll let you know when it’s ready to move to the next step or if any changes are needed.

@molivasdat molivasdat added BKeyPlatform Key Platform DBOracle ImpactLow IntegrationAny RiskMedium Changes that require more testing or that affect many different code paths. Severity3 TypeBug labels Nov 23, 2021
@molivasdat molivasdat added this to To Do in Conditioning++ via automation Nov 23, 2021
@nvoxland nvoxland changed the base branch from master to 1_9 January 5, 2022 17:49
@nvoxland nvoxland changed the base branch from 1_9 to master January 5, 2022 17:49
@nvoxland nvoxland moved this from To Do to Code Review in Conditioning++ Jan 5, 2022
@suryaaki2 suryaaki2 moved this from Code Review to Ready for Handoff (In JIRA) in Conditioning++ Jan 6, 2022
@nvoxland nvoxland merged commit 254a5a9 into liquibase:master Jan 24, 2022
Conditioning++ automation moved this from Ready for Handoff (In JIRA) to Done Jan 24, 2022
@nvoxland nvoxland added this to the v4.8.0 milestone Feb 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BKeyPlatform Key Platform DBOracle ImpactLow IntegrationAny RiskMedium Changes that require more testing or that affect many different code paths. Severity3 SyncTicket TypeBug
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants