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

Update Derby reserved words #1971

Merged
merged 2 commits into from Jul 26, 2022

Conversation

andrewhj
Copy link
Contributor

@andrewhj andrewhj commented Jul 8, 2021

Environment

Liquibase Version: master
Liquibase Integration & Version: spring boot

Liquibase Extension(s) & Version:

Database Vendor & Version: Derby

Operating System Type & Version: Windows 10

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

Updated DerbyDatabase definition to use the set of keywords defined in
https://db.apache.org/derby/docs/10.2/ref/rrefkeywords29722.html

Fixes #1031

Steps To Reproduce

define a changeset using a reserved word from the SQL-92 spec (such as action or state)
generate sql or migrate
table name should be standard but in this case would be "quoted"

the VerifyChangeClassesTest and modified derby.sql (included in the PR) perfectly illustrates the change.

Actual Behavior

generates sql like the following:

CREATE TABLE "state" ...

Expected/Desired Behavior

it should generate sql like:

CREATE TABLE state

which should make sql statements less picky about case, which will also make it work better with tools like JPA or DBUnit.

Updated DerbyDatabase definition to use the set of keywords defined in
https://db.apache.org/derby/docs/10.2/ref/rrefkeywords29722.html

resolves liquibase#1031
@molivasdat
Copy link
Contributor

Hi @andrewhj Thanks for creating this PR.
This does a little more than just updating keywords. We will add this to the list of changes to process.

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 DBApacheDerby ImpactLow IntegrationSpringboot RiskMedium Changes that require more testing or that affect many different code paths. Severity4 TypeEnhancement labels Nov 23, 2021
@nvoxland nvoxland added the SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions label Jul 17, 2022
@github-actions
Copy link

github-actions bot commented Jul 17, 2022

Unit Test Results

  4 608 files  ±0    4 608 suites  ±0   31m 11s ⏱️ - 5m 53s
  4 589 tests ±0    4 375 ✔️ ±0     214 💤 ±0  0 ±0 
54 240 runs  ±0  49 232 ✔️ ±0  5 008 💤 ±0  0 ±0 

Results for commit 88fa5f2. ± Comparison against base commit 83faa21.

♻️ This comment has been updated with latest results.

@nvoxland
Copy link
Contributor

Code review and test results:

Things to be aware of:

  • Uses a better list of words for derby
  • Only impacts derby's list of what to quote

Things to worry about:

  • Nothing

@kataggart kataggart added this to To Do in Conditioning++ via automation Jul 18, 2022
Copy link
Contributor

@XDelphiGrl XDelphiGrl left a comment

Choose a reason for hiding this comment

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

  • PR introduces a revised list of Derby reserved keywords used by Liquibase to determine quoting of generated Derby SQL.
  • Existing tests updated to reflect changes in keyword quoting.
  • No additional testing required.

APPROVED

Passing Functional Tests
Passing Test Harness Execution

@nvoxland nvoxland merged commit 8434904 into liquibase:master Jul 26, 2022
Conditioning++ automation moved this from To Do to Done Jul 26, 2022
@kataggart kataggart added this to the NEXT milestone Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autocandidate DBApacheDerby ImpactLow IntegrationSpringboot RiskMedium Changes that require more testing or that affect many different code paths. SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions Severity4 sprint2022-29 sprint2022-30 TypeEnhancement
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Derby reserved keywords is incorrect
6 participants