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

CockroachDatabase: Limit calls to version() #2556

Merged
merged 1 commit into from
Feb 25, 2022

Conversation

nvoxland
Copy link
Contributor

Description

Rather than making a likely invalid SQL call to select version() regardless of the connection to determine if it's a cockroachdb database, first check that the URL looks like a cockroachdb database before checking whether it is or not.

Checking for both jdbc:postgres: and postgres: without the : to also support jdbc:postgresql: and postgresql: based on https://www.cockroachlabs.com/docs/stable/connection-parameters.html#connect-using-a-url

Fixes #2483

@nvoxland
Copy link
Contributor Author

Testing notes:

  • Using a url like jdbc:h2:./tmp/h2-db creates a tmp/h2-db.trace.db file in your working directory which contains all the SQL executed against the database, including the invalid VERSION() call
  • Without a good way to start up cockroachdb yet, I couldn't ensure that CockroachDatabase.isCorrectDatabaseImplementation() works correctly. The code makes direct calls to the database so I couldn't even add unit tests well. Will rely on the functional tests passing to ensure the cockroachdb support did not break

@XDelphiGrl
Copy link
Contributor

  • Will rely on the functional tests passing to ensure the cockroachdb support did not break

@nvoxland, there are no functional tests that run against CockroachDb. However, the test-harness runs CockroachDb tests; this makes it easy to start up the database in multiple versions. I will run those locally.

CC @kristyldatical

@XDelphiGrl XDelphiGrl self-assigned this Feb 23, 2022
@XDelphiGrl
Copy link
Contributor

Liquibase determines if a JDBC URL connecting to the database through the Postgres driver is H2 or Cockroach. This prevents Liquibase from calling the wrong method to determine the database version, avoiding exceptions from being thrown when connecting to H2 with the Postgres driver.

NOTE On a "fresh" H2 database where DATABASECHANGELOGLOCK does not exist, H2 throws a JdbcSQLSyntaxErrorException when Liquibase queries the to see if the database is locked. This is expected behavior. The JdbcSQLSyntaxErrorException addressed in this PR is specific to calling the incorrect version method on H2.


Validate test-harness suite passes. PASS
CockroachDB 20.1 PASS
CockroachDB 21.1 PASS
CockroachDB 21.2 PASS
H2 2.1.210 PASS
Postgres 9 PASS
Postgres 9.5 PASS
Postgres 11 PASS
Postgres 12 PASS
Postgres 13 PASS
Postgres 14 PASS
EDB 9.5 PASS
EDB 9.6 PASS
EDB 10 PASS
EDB 11 PASS
EDB 12 PASS
EDB 13 PASS
EDB 14 PASS

Validate update calls the correct version() method for CockroachDB. PASS
Validate update does not throw a JdbcSqlSyntaxErrorException on H2 when determining version. PASS

  • The error org.h2.jdbc.JdbcSQLSyntaxErrorException: Function "VERSION" not found; SQL statement: select version() [90022-210] is not in the /tmp/h2-db.trace.db` PASS

Cockroach Setup

As a new-to-crdb tester, I followed the documentation to start a Cockroach demo environment in memory.

  • Note: On Windows, you must use PowerShell to run the command to get the CRDB executable.

To configure Liquibase to interact with Cockroach DB, I used followed these instructions. The liquibase.properties file to connect to the Cockroach demo database is:

#### Target Connection ####
#### Connects to movr database; must refresh page in webpage to see deployed tables. ####
#### http://127.0.0.1:8080/#/database/movr?viewMode=Tables ####
url: jdbc:postgresql://127.0.0.1:26257/movr?password=demo10760&sslmode=require&user=demo
username: demo
password: demo10760
driver: org.postgresql.Driver

The queries Liquibase runs are displayed on the webpage for SQL Activity.

H2 Setup

A local H2 database creates a trace db file where all SQL statements causing an exception are recorded. To create the in-memory H2 database and the trace db, configure the liquibase.properties url as:

liquibase.command.url=jdbc:h2:./tmp/h2-db

The first time Liquibase runs, the /tmp/h2-db.db and /tmp/h2-db.trace.db files are created in the current working directory. Open h2-db.trace.db with Notepad++ to see the SQL exceptions. As noted above, an exception is expected when connecting to a new H2 database where DATABASECHANGELOGLOCK table does not exist.


Test Environment
Liquibase Core: cockroach-limit-version-check/1562/324194, Pro: master/628/c0925a/
Cockroach 21.2.6 (manual tests)
H2 2.1.210 (manual tests)
Postgres 12 (manual tests)
Passing Functional Tests

@nvoxland nvoxland merged commit 4ea5410 into master Feb 25, 2022
@nvoxland nvoxland deleted the cockroach-limit-version-check branch February 25, 2022 20:48
@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
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

org.h2.jdbc.JdbcSQLSyntaxErrorException: Function "VERSION" not found
4 participants