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

BUG: Quoting not always identified properly, causes issues in end delimiter identification #5674

Closed
2 tasks done
jasonlyle88 opened this issue Mar 8, 2024 · 9 comments · Fixed by #5700
Closed
2 tasks done

Comments

@jasonlyle88
Copy link
Contributor

jasonlyle88 commented Mar 8, 2024

Search first

  • I searched and no similar issues were found

Description

When liquibase is looking for end delimiters, it is not supposed to consider end delimiter characters that are character literals (aka quoted strings). However, I have discovered certain use cases where this is not the case. This functionality was working (at least for my test cases) in 4.25.0 and broke in 4.25.1.

In looking at the version change log for 4.25.1, I noticed PR #5108 (titled Improve SQL parsing of character literals (quoted strings)) which directly relates to what is going on here.

I am unfamiliar with the .jj extension of the file changed in this PR or the grammar syntax being used here, but I am assuming this is the culprit.

Steps To Reproduce

Run an update command with liquibase version 4.25.1 or 4.26.0 (4.26.0 is the latest release at time of writing) and run the following changelogs on an oracle database:

changelog.xml:

<?xml version="1.0" encoding="UTF-8"?>
<databaseChangeLog
    xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xmlns:ora="http://www.oracle.com/xml/ns/dbchangelog-ext"
    xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.17.xsd"
>
    <include relativeToChangelogFile="true" file="test1.sql"/>
    <include relativeToChangelogFile="true" file="test2.sql"/>
</databaseChangeLog>

test1.sql:

--liquibase formatted sql

--changeset jlyle:test1 stripComments:false runOnChange:true runAlways:true endDelimiter:; rollbackEndDelimiter:;
create or replace view jmltest1 as
select '[;]' jml
from sys.dual;
--rollback not required

test2.sql:

--liquibase formatted sql

--changeset jlyle:test2 stripComments:false runOnChange:true runAlways:true endDelimiter:;
create or replace view jmltest as
select '~'';[\s]*~' jml
from sys.dual;
--rollback not required

The test1 changeset passes, proving this is not all quoted strings with end delimiters in them.

The test2 changeset fails, even though the end delimiter character (; in this case) is inside a quoted string and should not be considered an end delimiter.

Expected/Desired Behavior

Character literal end delimiters should not be considered for end delimiters

Liquibase Version

4.25.1, 4.26.0

Database Vendor & Version

Oracle

Liquibase Integration

No response

Liquibase Extensions

No response

OS and/or Infrastructure Type/Provider

No response

Additional Context

In the example, I used the string q'~'';[\s]*~' to cause the issue, but the following list all cause the issue. I have not fully determined what exactly about these strings cause the issue

End delimiter: ;

q'~'';[\s]*~'
q'~';[\s]*~'
q'~;[\s]*~'
q'~;[\s]~'
q'~;\~'
q'!;\!'
q'{;\}'
q'{
;
\}'
q'{
;\}'
q'(;\)'
q'<;\>'
  • WORKS: q'[;\]'

Are you willing to submit a PR?

  • I'm willing to submit a PR (Thank you!)
@jasonlyle88 jasonlyle88 changed the title BUG: Oracle Q quoting (q'{...}') not always handled properly in end delimiter processing BUG: Quoting not always handled properly in end delimiter processing Mar 11, 2024
@jasonlyle88
Copy link
Contributor Author

All of my previous examples show the use case where an end delimiter is part of a quoted string, but still considered as an end delimiter.

In further explorations, I have found another facet of this issue: end delimiters that are not a part of a quoted string, but liquibase thinks they are and does not consider them an end delimiter.

For an example of this, see the following changelog:

--liquibase formatted sql

--changeset jlyle:test_run_1 stripComments:false runOnChange:true runAlways:true endDelimiter:/
begin
    dbms_output.put_line('foo\_bar');
end;
/

begin
    dbms_output.put_line('bar');
end;
/
--rollback not required

The parser does not recongnize the first / character as an end delimiter and tries to send both anonymous blocks together as a single statement, causing an error.

With my continued testing, I have found that this has something to do with the \ character being in a string. It appears that this behavior occurs only when certain characters follow the \ character. Based on my limited understanding of the SimpleSqlGrammar.jj file, I believe any character except for n, t, b, r, f, \, 0, ', " would case an issue.

As such... this seems like a VERY large issue.

Tagging people I've worked with for visibility as I think this is important: @filipelautert @MalloD12

@jasonlyle88
Copy link
Contributor Author

jasonlyle88 commented Mar 14, 2024

Hey @kevin-atx I see this was flagged as DBOracle, but this actually affects every database (its an issue with the SQL grammar used to parse strings before a database is involved). The original issue was created as if it was just Oracle, because I though it was something else. But once I dug into it, I discovered this is the grammar and will affect all databases.

I verified this by creating a test in the liquibase-standard/src/test/groovy/liquibase/util/grammar/SimpleSqlGrammarTest.groovy file as described in my second post above, and it is failing there.

The test I added was the following input:

"select 'foo\\_bar' from sys.dual;"

should equal the following expected value:

["select", " ", "'foo\\_bar'", " ", "from", " ", "sys.dual", ";"]

With the version of the SQL grammar before the problematic MR I mentioned, the output is as expected. But with the current implementation, the output does not match and the quoted string is incorrectly identified.

@jasonlyle88 jasonlyle88 changed the title BUG: Quoting not always handled properly in end delimiter processing BUG: Quoting not always identified properly, causes issues in end delimiter identification Mar 18, 2024
@doakd
Copy link

doakd commented Mar 19, 2024

Is there an ETA for releasing the fix for this? It is causing problems for us since we upgraded to 4.26.

Here is an example of a string that causes this issue for us:

'ENV[PAMDRV]PFS\PFI\Scripts'

@MalloD12
Copy link
Contributor

Is there an ETA for releasing the fix for this? It is causing problems for us since we upgraded to 4.26.

Here is an example of a string that causes this issue for us:

'ENV[PAMDRV]PFS\PFI\Scripts'

We aim to have this change in the next release which could be around the end of March/beginning of April.

Thanks,
Daniel.

@doakd
Copy link

doakd commented Mar 28, 2024

This issue was not resolved by 4.27, or I've found another new issue.

--changeset BOB:insert_into_test_country stripComments:false

INSERT INTO test_country (country_number, country_name)
VALUES ('1',   --county_number
        'USA') --country_name
;
INSERT INTO test_country (country_number, country_name)
VALUES ('2',   --county_number
        '@E:\PAMCFA\Report\JobOutput') --country_name
;
INSERT INTO test_country (country_number, country_name)
VALUES ('3',   --county_number
        'ENV[PAMDRV]PFS\PFI\Scripts') --country_name
;

This results in:

[2024-03-28 18:42:20] INFO [liquibase.ui] Starting Liquibase at 18:42:20 (version 4.27.0 #1525 built at 2024-03-25 17:08+0000)

[2024-03-28 18:42:38] SEVERE [liquibase.changelog] ChangeSet db-changelog-2.sql::insert_into_test_country1::doaksec encountered an exception.
liquibase.exception.DatabaseException: ORA-00900: invalid SQL statement

These statements run fine in sql-developer.

@MalloD12
Copy link
Contributor

Hi @jasonlyle88,

Could you please look into the issue @doakd mentioned when you have some time?

Thanks,
Daniel.

@jasonlyle88
Copy link
Contributor Author

@doakd , I was unable to replicate this issue in 4.27.0. Please see below:

Example changelog:

--liquibase formatted sql

--changeset jlyle:test_country_create stripComments:false
--preconditions onFail:MARK_RAN onError:HALT
--precondition-sql-check expectedResult:0 select count(1) from all_tables where owner = 'LB' and table_name = 'TEST_COUNTRY';
create table lb.test_country(
    country_number  number,
    country_name    varchar2(4000 char)
)
logging;
--rollback drop table lb.test_country;

--changeset BOB:insert_into_test_country stripComments:false

INSERT INTO lb.test_country (country_number, country_name)
VALUES ('1',   --county_number
        'USA') --country_name
;
INSERT INTO lb.test_country (country_number, country_name)
VALUES ('2',   --county_number
        '@E:\PAMCFA\Report\JobOutput') --country_name
;
INSERT INTO lb.test_country (country_number, country_name)
VALUES ('3',   --county_number
        'ENV[PAMDRV]PFS\PFI\Scripts') --country_name
;
--rollback truncate table lb.test_country

Results on 4.27.0:

####################################################
##   _     _             _ _                      ##
##  | |   (_)           (_) |                     ##
##  | |    _  __ _ _   _ _| |__   __ _ ___  ___   ##
##  | |   | |/ _` | | | | | '_ \ / _` / __|/ _ \  ##
##  | |___| | (_| | |_| | | |_) | (_| \__ \  __/  ##
##  \_____/_|\__, |\__,_|_|_.__/ \__,_|___/\___|  ##
##              | |                               ##
##              |_|                               ##
##                                                ##
##  Get documentation at docs.liquibase.com       ##
##  Get certified courses at learn.liquibase.com  ##
##                                                ##
####################################################
Starting Liquibase at 09:45:35 (version 4.27.0 #1525 built at 2024-03-25 17:08+0000)
Liquibase Version: 4.27.0
Liquibase Open Source 4.27.0 by Liquibase
Running Changeset: changelog.sql::test_country_create::jlyle
Running Changeset: changelog.sql::insert_into_test_country::BOB

UPDATE SUMMARY
Run:                          2
Previously run:               0
Filtered out:                 0
-------------------------------
Total change sets:            2

Liquibase: Update has been successful. Rows affected: 5
Liquibase command 'update' was executed successfully.

However, I am seeing the issue you describe in 4.26.0:

####################################################
##   _     _             _ _                      ##
##  | |   (_)           (_) |                     ##
##  | |    _  __ _ _   _ _| |__   __ _ ___  ___   ##
##  | |   | |/ _` | | | | | '_ \ / _` / __|/ _ \  ##
##  | |___| | (_| | |_| | | |_) | (_| \__ \  __/  ##
##  \_____/_|\__, |\__,_|_|_.__/ \__,_|___/\___|  ##
##              | |                               ##
##              |_|                               ##
##                                                ##
##  Get documentation at docs.liquibase.com       ##
##  Get certified courses at learn.liquibase.com  ##
##                                                ##
####################################################
Starting Liquibase at 09:47:29 (version 4.26.0 #1141 built at 2024-02-06 21:31+0000)
Liquibase Version: 4.26.0
Liquibase Open Source 4.26.0 by Liquibase
Running Changeset: changelog.sql::test_country_create::jlyle
Running Changeset: changelog.sql::insert_into_test_country::BOB

UPDATE SUMMARY
Run:                          2
Previously run:               0
Filtered out:                 0
-------------------------------
Total change sets:            2

ERROR: Exception Details
ERROR: Exception Primary Class:  OracleDatabaseException
ERROR: Exception Primary Reason: ORA-00933: SQL command not properly ended

ERROR: Exception Primary Source: Oracle Oracle Database 19c Enterprise Edition Release 19.0.0.0.0 - Production
Version 19.23.0.1.0

Unexpected error running Liquibase: Migration failed for changeset changelog.sql::insert_into_test_country::BOB:
     Reason: liquibase.exception.DatabaseException: ORA-00933: SQL command not properly ended
 [Failed SQL: (933) INSERT INTO lb.test_country (country_number, country_name)
VALUES ('2',   --county_number
        '@E:\PAMCFA\Report\JobOutput') --country_name
;
INSERT INTO lb.test_country (country_number, country_name)
VALUES ('3',   --county_number
        'ENV[PAMDRV]PFS\PFI\Scripts') --country_name]

For more information, please use the --log-level flag

Can you please confirm you are actually testing wit hthe correct version?

@doakd
Copy link

doakd commented Mar 29, 2024

OK @jasonlyle88 , you are correct, I discovered the issue. I had a commented changeset after the insert changset, and it contained a nested comment block:

/*
--changeset BOB:insert_into_test_country2 stripComments:false
INSERT INTO test_country (country_number, country_name)
VALUES ('3',   --county_number
        'ENV[PAMDRV]PFS\PFI\Scripts') --country_name
;

/* liquibase rollback
DELETE FROM test_country where country_number = '3';
*/
*/

When that is removed it runs as expected in 4.27. Even though the comments are balanced, this doesn't run in sql-developer either. Sorry for the firedrill.

@jasonlyle88
Copy link
Contributor Author

@doakd No worries! I've sent the liquibase team down a couple rabbit holes myself. It happens! I'm just glad the issue isn't with the code 🤣

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants