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

Recognize jdbc:edb urls as a distinct EnterpriseDB dialect #2244

Merged
merged 5 commits into from Dec 21, 2021

Conversation

szandany
Copy link
Contributor

@szandany szandany commented Dec 1, 2021

Description

Adds EnterpriseDBDatabase dialect class so that liquibase will correctly recognize "jdbc:edb:..." urls as being like postgresql but potentially different.

There was some stub code around that used a postgersql.getDbType() call to differentiate between COMMUNITY and EDB enum values which I switched to the more standard "instanceof EnterpriseDBDatabase" checks.

With this change, liquibase should correctly recognize the url value jdbc:edb://host:5444/dbname and default to using the edb jdbc driver.

|----|----|----|----|----|
|liquibase lib directory|properties/cli|properties/cli|dialect|changetype|
|EDB driver installed|Driver|url|updateSQL|createPackage|
|no|none|jbdc:edb|FAIL|FAIL|
|no|postgres|jbdc:edb|FAIL|FAIL|
|no|edb|jbdc:edb|edb|pass|
|no|none|jbdc:postgres|postgres|FAIL|
|no|postgres|jbdc:postgres|postgres|FAIL|
|no|edb|jbdc:postgres|postgres|FAIL|
|yes|none|jbdc:edb|edb|pass|
|yes|postgres|jbdc:edb|FAIL|FAIL|
|yes|edb|jbdc:edb|edb|pass|
|yes|none|jbdc:postgres|postgres|FAIL|
|yes|postgres|jbdc:postgres|postgres|FAIL|
|yes|edb|jbdc:postgres|postgres|FAIL|

Testing

Start an enterprisedb container like:

docker run --rm -e POSTGRES_PASSWORD=LiquibasePass1 -p5444:5432 enterprisedb/postgresql

and then connect to it on localhost:5444 with standard postgresql tools to create a test user/database:

create user lbuser;
alter user lbuser password 'LiquibasePass1';

create database lbcat owner lbuser;

With the fix, you can run liquibase operations against --url jdbc:edb://localhost:5444/lbcat --username lbuser --password LiquibasePass1 and it will default to the com.edb.Driver driver. You can see this by not having the edb driver in your lib dir and getting a Cannot find database driver: com.edb.Driver error.

By adding the driver from https://www.enterprisedb.com/software-downloads-postgres to your lib dir, the "cannot find database driver" error goes away and liquibase runs like normal.

Level of EDB Support

This change ONLY makes it so a jdbc:edb url works as users expect. We are not adding official support for edb yet or testing anything beyond that we can connect and do a very simple update operation. Any additional EDB testing or features will be done in future work. This just sets us up for that work and makes basic use easy for end users.


Dev Handoff Notes (Internal Use)

Links

Testing

  • Steps to Reproduce: See above
  • Guidance:
    • Impact: only impacts enterprisedb support.

Dev Verification

Started enterprisedb container and ran operations against it using standard postgresql URL. Saw that jdbc:edb failed with the missing driver as expected.

Test Requirements (Liquibase Internal QA)

Examples of liquibase.properties and changelog files are attached.

Configure liquibase.properties file to connect to your database instance.

Manual Tests.

1 Verify Liquibase behavior when EDB Driver is not in lib diectory and jdbc:edb url is specified.

  • make sure jdbc:postgres url is commented out in liquibase.properties file
  • make sure jdbc:edb url is uncommented in liquibase.properties file
  • 1.1 Verify that Liquibase throws an error if no driver is specified
    • make sure both edb and postgres drivers are commented out in liquibase.properties file
    • run liquibase update-slq -changelog-file lb2185-changelog.xml --labels table,function
      • Unexpected error running Liquibase: java.lang.RuntimeException: Cannot find database driver: com.edb.Driver
  • 1.2 Verify that Liquibase throws an error if postgres driver is specified
    • uncomment postgres driver in liquibase.properties file
    • run liquibase update-slq -changelog-file lb2185-changelog.xml --labels table,function

Unexpected error running Liquibase: Connection could not be created to jdbc:edb://localhost:### with driver org.postgresql.Driver.
Possibly the wrong driver for the given database URL



* _1.3 Verify that Liquibase throws an error if_ **_edb_** _driver is specified_
  * comment out postgres driver in liquibase.properties file
  * uncomment edb driver in liquibase.properties file
  * run `liquibase update-slq -changelog-file lb2185-changelog.xml --labels table,function`
    * `Unexpected error running Liquibase: java.lang.RuntimeException: Cannot find database driver: com.edb.Driver`

_2 Verify Liquibase behavior when EDB Driver is_ **_not_** _in lib diectory and j_**_dbc:postgres_** _url is specified._

* make sure jdbc:edb url is commented out in liquibase.properties file
* make sure jdbc:postgres url is uncommented in liquibase.properties file
* _2.1 Verify that Liquibase works succesfully if_ **_no_** _driver is specified_
  * make sure both edb and postgres drivers are commented out in liquibase.properties file
  * run `liquibase update-slq -changelog-file lb2185-changelog.xml --labels table,function`
    * `udate-sql` is successful
    * ```
-- Changeset lb2185-changelog.xml::1::Liquibase
CREATE TABLE public.test_table (test_id INTEGER NOT NULL, test_column VARCHAR, CONSTRAINT test_table_pkey PRIMARY KEY (test_id));
-- Changeset lb2185-changelog.xml::2::your.name
CREATE OR REPLACE FUNCTION TestFunction()  
RETURNS test_table  
LANGUAGE SQL   
AS   
$$  
    SELECT * FROM test_table;  
$$/
  • run liquibase update -changelog-file lb2185-changelog.xml --labels table,function
    • update is successful
    • test_table is present in database
    • TestFunction is present in database
  • run liquibase update-slq -changelog-file lb2185-changelog.xml --labels package
    • udate-sql is successful

-- Changeset lb2185-changelog.xml::3::Liquibase
CREATE OR REPLACE PACKAGE empinfo
IS
emp_name VARCHAR2(10);
PROCEDURE get_name (
p_empno NUMBER
);
FUNCTION display_counter
RETURN INTEGER;
END/



  * run `liquibase update -changelog-file lb2185-changelog.xml --labels package`
    * `syntax error` is thrown
* _2.2 Verify that Liquibase works succesfully if_ **_postgres_** _driver is specified_
  * uncomment postgres driver in liquibase.properties file
  * run `liquibase update-slq -changelog-file lb2185-changelog.xml --labels table,function`
    * `udate-sql` is successful
  * run `liquibase update -changelog-file lb2185-changelog.xml --labels table,function` 
    * `update` is successful
    * `test_table` is present in database
    * `TestFunction` is present in database
  * run `liquibase update -changelog-file lb2185-changelog.xml --labels package`
    * `syntax error` is thrown
* _2.3 Verify that Liquibase throws an error if_ **_edb_** _driver is specified_
  * comment out postgres driver in liquibase.properties file
  * uncomment edb driver in liquibase.properties file
  * run `liquibase update-slq -changelog-file lb2185-changelog.xml --labels table,function`
    * `Unexpected error running Liquibase: Connection could not be created to jdbc:postgresql://localhost:5506/lbcat with driver com.edb.Driver.  Possibly the wrong driver for the given database URL`

_3_ _Verify Liquibase behavior when EDB Driver_ **_is_** _in lib diectory and j_**_dbc:postgres_** _url is specified._

* make sure jdbc:edb url is commented out in liquibase.properties file
* make sure jdbc:postgres url is uncommented in liquibase.properties file
* _3.1 Verify that Liquibase works succesfully if_ **_no_** _driver is specified_
  * make sure both edb and postgres drivers are commented out in liquibase.properties file
  * run `liquibase update-slq -changelog-file lb2185-changelog.xml --labels table,function`
    * `udate-sql` is successful
  * run `liquibase update -changelog-file lb2185-changelog.xml --labels table,function` 
    * `update` is successful
    * `test_table` is present in database
    * `TestFunction` is present in database
  * run `liquibase update-slq -changelog-file lb2185-changelog.xml --labels package`
    * `udate-sql` is successful
  * run `liquibase update -changelog-file lb2185-changelog.xml --labels package`
    * `syntax error` is thrown
* _3.2 Verify that Liquibase works succesfully if postgres driver is specified_
  * uncomment postgres driver in liquibase.properties file
  * run `liquibase update-slq -changelog-file lb2185-changelog.xml --labels table,function`
    * `udate-sql` is successful
  * run `liquibase update -changelog-file lb2185-changelog.xml --labels table,function` 
    * `update` is successful
    * `test_table` is present in database
    * `TestFunction` is present in database
  * run `liquibase update-slq -changelog-file lb2185-changelog.xml --labels package`
    * `udate-sql` is successful
  * run `liquibase update -changelog-file lb2185-changelog.xml --labels package`
    * `syntax error` is thrown
* _3.3 Verify that Liquibase throws an error if_ **_edb_** _driver is specified_
  * comment out postgres driver in liquibase.properties file
  * uncomment edb driver in liquibase.properties file
  * run `liquibase update-slq -changelog-file lb2185-changelog.xml --labels table,function`
    * `Unexpected error running Liquibase: Connection could not be created to jdbc:postgresql://localhost:5506/lbcat with driver com.edb.Driver.  Possibly the wrong driver for the given database URL`

_4_ _Verify Liquibase behavior when EDB Driver_ **_is_** _in lib diectory and j_**_dbc:edb_** _url is specified._

* make sure jdbc:postgres url is commented out in liquibase.properties file
* make sure jdbc:edb url is uncommented in liquibase.properties file
* _4.1 Verify that Liquibase works succesfully if_ **_no_** _driver is specified_
  * make sure both edb and postgres drivers are commented out in liquibase.properties file
  * run `liquibase update-slq -changelog-file lb2185-changelog.xml --labels table,function`
    * `udate-sql` is successful
  * run `liquibase update -changelog-file lb2185-changelog.xml --labels table,function` 
    * `update` is successful
    * `test_table` is present in database
    * `TestFunction` is present in database
  * run `liquibase update-slq -changelog-file lb2185-changelog.xml --labels package`
    * `udate-sql` is successful
  * run `liquibase update -changelog-file lb2185-changelog.xml --labels package`
    * `update` is successful
    * `empinfo` package is present in database (SELECT * FROM USER_OBJECTS WHERE OBJECT_TYPE = 'PACKAGE') 
* _4.2 Verify that Liquibase throws an error if postgres driver is specified_
  * uncomment postgres driver in liquibase.properties file
  * run `liquibase update-slq -changelog-file lb2185-changelog.xml --labels table,function`
    * `Unexpected error running Liquibase: Connection could not be created to jdbc:edb://localhost:5506/lbcat with driver org.postgresql.Driver.  Possibly the wrong driver for the given database URL`
* _4.3 Verify that Liquibase throws an error if_ **_edb_** _driver is specified_
  * comment out postgres driver in liquibase.properties file
  * uncomment edb driver in liquibase.properties file
  * make sure both edb and postgres drivers are commented out in liquibase.properties file
  * run `liquibase update-slq -changelog-file lb2185-changelog.xml --labels table,function`
    * `udate-sql` is successful
  * run `liquibase update -changelog-file lb2185-changelog.xml --labels table,function` 
    * `update` is successful
    * `test_table` is present in database
    * `TestFunction` is present in database
  * run `liquibase update-slq -changelog-file lb2185-changelog.xml --labels package`
    * `udate-sql` is successful
  * run `liquibase update -changelog-file lb2185-changelog.xml --labels package`
    * `update` is successful
    * `empinfo` package is present in database (SELECT * FROM USER_OBJECTS WHERE OBJECT_TYPE = 'PACKAGE') 

_5 Verify Liquibase behavior when EDB Driver_ **_is_** _specified in classpath and j_**_dbc:edb_** _url is specified._

* repeat TestCases 4.1 - 4.3
* expected results stay the same as in TestCases 4.1 - 4.3



┆Issue is synchronized with this [Jira Bug](https://datical.atlassian.net/browse/LB-2185) by [Unito](https://www.unito.io)

@nvoxland nvoxland added this to To Do in Conditioning++ via automation Dec 1, 2021
@nvoxland nvoxland removed this from To Do in Conditioning++ Dec 1, 2021
@nvoxland nvoxland added this to To Do in Conditioning++ via automation Dec 1, 2021
@r2-lf
Copy link
Contributor

r2-lf commented Dec 2, 2021

What do you think about creating a new EDB extension? @nvoxland created a template that should make that easier: https://github.com/liquibase/liquibase-extension-example.

@szandany
Copy link
Contributor Author

szandany commented Dec 2, 2021

Hi @r2liquibase
I think since this is a db platform and a simple extended PostgresDB class, this should be part of core.

@wwillard7800
Copy link
Contributor

We should make sure we don't re-invent the wheel wrt to the EDB work done in DaticalDB.

@nvoxland nvoxland moved this from To Do to In discussion in Conditioning++ Dec 8, 2021
@molivasdat molivasdat moved this from In discussion to To Do in Conditioning++ Dec 9, 2021
- Removed old/unused Postgresql DbTypes enum
- Updated code to use standard "instanceof EnterpriseDBDatabase" style vs. getDbType() logic
@nvoxland nvoxland moved this from To Do to Code Review in Conditioning++ Dec 13, 2021
@liquibase liquibase deleted a comment from andre15silva Dec 13, 2021
@nvoxland
Copy link
Contributor

I talked with @wwillard7800 about these changes and they are good. It's just some simple support at this point and not re-inventing anything. As we look at future features we can bring in other existing code as needed.

@suryaaki2 suryaaki2 moved this from Code Review to Ready for Handoff (In JIRA) in Conditioning++ Dec 13, 2021
@nvoxland nvoxland changed the title added new edb support Add basic EnterpriseDB support Dec 13, 2021
- Removed old/unused Postgresql DbTypes enum
- Updated code to use standard "instanceof EnterpriseDBDatabase" style vs. getDbType() logic
- Removed old/unused Postgresql DbTypes enum
- Updated code to use standard "instanceof EnterpriseDBDatabase" style vs. getDbType() logic
@nvoxland nvoxland changed the title Add basic EnterpriseDB support Recognize jdbc:edb urls as a distinct EnterpriseDB dialect Dec 15, 2021
@liquibase liquibase deleted a comment from sync-by-unito bot Dec 15, 2021
@sync-by-unito
Copy link

sync-by-unito bot commented Dec 17, 2021

➤ Erzsebet Carmean commented:

karen.a.taggart, hi! Thank you very much for the research. Thank you even more, a thousand times more, for the detail you provide in the last comment. We have what we need to scope the test effort for this ticket.

Jack Odzhubeiskyi, I’m going to assign this one to you for conditioning. My suggestion is to take the table Karen provided and move it into the description so that the entire community can see what we’re going to validate. I am not sure where to get the edb driver; if you cannot find it, please let me know. Additionally, I don’t know of any example packages to use with edb. Nathan Voxland, can you provide those, please?

CC Kristyl Gomes

@sync-by-unito
Copy link

sync-by-unito bot commented Dec 17, 2021

➤ karen.a.taggart commented:

Thanks all – Jack Odzhubeiskyi the edb driver can be found https://www.enterprisedb.com/software-downloads-postgres ( https://www.enterprisedb.com/software-downloads-postgres|smart-link ) (I can’t pretend to be that brilliant, thankfully Tsvi Zandany provided in his original issue description)

@szandany
Copy link
Contributor Author

szandany commented Dec 17, 2021

Confirming - you have the right link to download the edb JDBC driver in your comment, @kataggart (including in the issue description).

@sync-by-unito
Copy link

sync-by-unito bot commented Dec 21, 2021

➤ Jack Odzhubeiskyi commented:

Liquibase version [Core: //add-edb-basic-support/987/8d61b3/2021-12-20 10:39+0000, Pro: add-edb-basic-support/248/3bf2b3/2021-12-13T22:23:28Z] #9871 Verify Liquibase behavior when EDB Driver is not in lib diectory and jdbc:edb url is specified. PASS

  • 1.1 Verify that Liquibase works succesfully if no driver is specified PASS
  • 1.2 Verify that Liquibase throws an error if postgres driver is specified. PASS
  • 1.3 Verify that Liquibase throws an error if edb driver is specified PASS

2 Verify Liquibase behavior when EDB Driver is not in lib diectory and jdbc:postgres url is specified. PASS

  • 2.1 Verify that Liquibase works succesfully if no driver is specified PASS
  • 2.2 Verify that Liquibase throws an error if postgres driver is specified PASS
  • 2.3 Verify that Liquibase throws an error if edb driver is specified PASS

3 Verify Liquibase behavior when EDB Driver is in lib diectory and jdbc:postgres url is specified. PASS

  • 3.1 Verify that Liquibase works succesfully if no driver is specified PASS
  • 3.2 Verify that Liquibase throws an error if postgres driver is specified PASS
  • 3.3 Verify that Liquibase throws an error if edb driver is specified PASS

4 Verify Liquibase behavior when EDB Driver is in lib diectory and jdbc:edb url is specified. PASS

  • 4.1 Verify that Liquibase works succesfully if no driver is specified PASS
  • 4.2 Verify that Liquibase throws an error if postgres driver is specified PASS
  • 4.3 Verify that Liquibase throws an error if edb driver is specified PASS
  1. Verify Liquibase behavior when EDB Driver is specified in classpath and jdbc:edb url is specified. PASS
  • 5.1 Verify that Liquibase works succesfully if no driver is specified PASS
  • 5.2 Verify that Liquibase throws an error if postgres driver is specified PASS
  • 5.3 Verify that Liquibase throws an error if edb driver is specified PASS

@nvoxland nvoxland merged commit 83535ee into master Dec 21, 2021
Conditioning++ automation moved this from Ready for Handoff (In JIRA) to Done Dec 21, 2021
@nvoxland nvoxland deleted the add-edb-basic-support branch December 21, 2021 22:17
@nvoxland nvoxland added this to the v4.7.0 milestone Jan 6, 2022
@nvoxland nvoxland removed this from Done in Conditioning++ Jan 6, 2022
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

6 participants