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

add support for default-catalog-name in SQL Server (DAT-10484) #2940

Merged
merged 2 commits into from Jun 17, 2022

Conversation

StevenMassaro
Copy link
Contributor

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)

Description

Adds support for --default-catalog-name when using SQL Server.

Things to be aware of

Things to worry about

  • This breaks the existing trend of treating schemas identically to catalogs. I believe this to be correct for SQL Server, but will that be confusing for users?

Additional Context

@github-actions
Copy link

github-actions bot commented Jun 14, 2022

Unit Test Results

  4 548 files  ±0    4 548 suites  ±0   33m 47s ⏱️ +24s
  4 508 tests ±0    4 294 ✔️ ±0     214 💤 ±0  0 ±0 
53 376 runs  ±0  48 368 ✔️ ±0  5 008 💤 ±0  0 ±0 

Results for commit aafc235. ± Comparison against base commit fc5ccbd.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@nvoxland nvoxland left a comment

Choose a reason for hiding this comment

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

The change makes sense, thanks

@StevenMassaro StevenMassaro merged commit 1e231ee into master Jun 17, 2022
Conditioning++ automation moved this from To Do to Done Jun 17, 2022
@StevenMassaro StevenMassaro deleted the DAT-10484 branch June 17, 2022 17:33
@chandlerkent
Copy link

@StevenMassaro @nvoxland We upgraded from 4.10 to 4.15, which includes this change, and that broke our Liquibase setup. Here is the error we are now getting:

[2022-08-17 15:13:31] SEVERE [liquibase.integration] Incorrect syntax near the keyword 'null'. [Failed SQL: (156) USE null]
liquibase.exception.CommandExecutionException: liquibase.exception.LiquibaseException: Unexpected error running Liquibase: Incorrect syntax near the keyword 'null'. [Failed SQL: (156) USE null]
	at liquibase.command.CommandScope.execute(CommandScope.java:162)
	at liquibase.integration.commandline.CommandRunner.call(CommandRunner.java:51)
	at liquibase.integration.commandline.CommandRunner.call(CommandRunner.java:21)
	at picocli.CommandLine.executeUserObject(CommandLine.java:1953)
	at picocli.CommandLine.access$1300(CommandLine.java:145)
	at picocli.CommandLine$RunLast.executeUserObjectOfLastSubcommandWithSameParent(CommandLine.java:2358)
	at picocli.CommandLine$RunLast.handle(CommandLine.java:2352)
	at picocli.CommandLine$RunLast.handle(CommandLine.java:2314)
	at picocli.CommandLine$AbstractParseResultHandler.execute(CommandLine.java:2179)
	at picocli.CommandLine$RunLast.execute(CommandLine.java:2316)
	at picocli.CommandLine.execute(CommandLine.java:2078)
	at liquibase.integration.commandline.LiquibaseCommandLine.lambda$execute$1(LiquibaseCommandLine.java:334)
	at liquibase.Scope.child(Scope.java:189)
	at liquibase.Scope.child(Scope.java:165)
	at liquibase.integration.commandline.LiquibaseCommandLine.execute(LiquibaseCommandLine.java:299)
	at liquibase.integration.commandline.LiquibaseCommandLine.main(LiquibaseCommandLine.java:84)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
	at java.lang.reflect.Method.invoke(Unknown Source)
	at liquibase.integration.commandline.LiquibaseLauncher.main(LiquibaseLauncher.java:91)
Caused by: liquibase.exception.LiquibaseException: Unexpected error running Liquibase: Incorrect syntax near the keyword 'null'. [Failed SQL: (156) USE null]
	at liquibase.integration.commandline.Main$1.run(Main.java:440)
	at liquibase.integration.commandline.Main$1.run(Main.java:218)
	at liquibase.Scope.child(Scope.java:189)
	at liquibase.Scope.child(Scope.java:165)
	at liquibase.integration.commandline.Main.run(Main.java:218)
	at liquibase.command.AbstractCliWrapperCommandStep.run(AbstractCliWrapperCommandStep.java:33)
	at liquibase.command.CommandScope.execute(CommandScope.java:156)
	... 20 more
Caused by: liquibase.exception.DatabaseException: liquibase.exception.DatabaseException: Incorrect syntax near the keyword 'null'. [Failed SQL: (156) USE null]
	at liquibase.integration.commandline.CommandLineUtils.createDatabaseObject(CommandLineUtils.java:138)
	at liquibase.integration.commandline.Main.doMigration(Main.java:1479)
	at liquibase.integration.commandline.Main$1.lambda$run$0(Main.java:395)
	at liquibase.Scope.lambda$child$0(Scope.java:180)
	at liquibase.Scope.child(Scope.java:189)
	at liquibase.Scope.child(Scope.java:179)
	at liquibase.Scope.child(Scope.java:158)
	at liquibase.integration.commandline.Main$1.run(Main.java:394)
	... 26 more
Caused by: liquibase.exception.DatabaseException: Incorrect syntax near the keyword 'null'. [Failed SQL: (156) USE null]
	at liquibase.executor.jvm.JdbcExecutor$ExecuteStatementCallback.doInStatement(JdbcExecutor.java:434)
	at liquibase.executor.jvm.JdbcExecutor.execute(JdbcExecutor.java:77)
	at liquibase.executor.jvm.JdbcExecutor.execute(JdbcExecutor.java:160)
	at liquibase.executor.jvm.JdbcExecutor.execute(JdbcExecutor.java:125)
	at liquibase.database.core.DatabaseUtils.initializeDatabase(DatabaseUtils.java:63)
	at liquibase.integration.commandline.CommandLineUtils.createDatabaseObject(CommandLineUtils.java:134)
	... 33 more
Caused by: com.microsoft.sqlserver.jdbc.SQLServerException: Incorrect syntax near the keyword 'null'.
	at com.microsoft.sqlserver.jdbc.SQLServerException.makeFromDatabaseError(SQLServerException.java:265)
	at com.microsoft.sqlserver.jdbc.SQLServerStatement.getNextResult(SQLServerStatement.java:1676)
	at com.microsoft.sqlserver.jdbc.SQLServerStatement.doExecuteStatement(SQLServerStatement.java:907)
	at com.microsoft.sqlserver.jdbc.SQLServerStatement$StmtExecCmd.doExecute(SQLServerStatement.java:802)
	at com.microsoft.sqlserver.jdbc.TDSCommand.execute(IOBuffer.java:7730)
	at com.microsoft.sqlserver.jdbc.SQLServerConnection.executeCommand(SQLServerConnection.java:3786)
	at com.microsoft.sqlserver.jdbc.SQLServerStatement.executeCommand(SQLServerStatement.java:268)
	at com.microsoft.sqlserver.jdbc.SQLServerStatement.executeStatement(SQLServerStatement.java:242)
	at com.microsoft.sqlserver.jdbc.SQLServerStatement.execute(SQLServerStatement.java:775)
	at liquibase.executor.jvm.JdbcExecutor$ExecuteStatementCallback.doInStatement(JdbcExecutor.java:430)
	... 38 more

Our command looks something like this:

liquibase.bat --driver="com.microsoft.sqlserver.jdbc.SQLServerDriver" --url=jdbc:"sqlserver://host.docker.internal;databaseName=REDACTED;trustServerCertificate=true;" --defaultSchemaName="dbo" --username="REDACTED" --password="REDACTED" --changeLogFile="REDACTED" --logLevel="debug" --logFile="REDACTED" --defaultsFile="liquibase.properties" update

@chandlerkent
Copy link

chandlerkent commented Aug 17, 2022

We have determined that we can either leave off defaultSchemaName or add defaultCatalogName (but we need to properly escape it which seems wrong, like: --defaultCatologName "[MyDatabase-That-Has-Hyphens]").

Either way, I think the bug with this change is all the other database types use schema name and catalog name somewhat interchangeably, which is not true with SQL Server. The USE statement should not be run for SQL Server if defaultCatalogName is not specified.

@kataggart
Copy link
Contributor

@chandlerkent would you mind creating a new bug issue so this doesn't get lost since this PR is already merged? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants