AWS MySQL script execution raises UnableToExecuteStatementException when includes begin..end; block under certain conditions

[IrinaTerlizhenko]

Script execution fails in case the following conditions are satisfied:

• the script contains at least 1 statement with begin..end block
• the script contains 5+ statements in total
• rewriteBatchedStatements=true parameter is set in the connection string

The issue is reproducible in 3.42.0 and doesn't appear in 3.30.0. Cannot easily check versions in between because of the previous bug #2535.

JDBI versions: 3.42.0
Java version: 11
MySQL version: 8.0.32
JDBC Driver: software.aws.rds:aws-mysql-jdbc:1.1.11

Test to reproduce:

plugins {
    id 'java'
}

repositories {
    mavenCentral()
}

dependencies {
    implementation 'org.jdbi:jdbi3-core:3.42.0'
    runtimeOnly 'software.aws.rds:aws-mysql-jdbc:1.1.11'
    testImplementation 'org.junit.jupiter:junit-jupiter-api:5.8.1'
    testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.8.1'
}

test {
    useJUnitPlatform()
}

import org.jdbi.v3.core.Jdbi;
import org.junit.jupiter.api.Test;

public class QweTest {
    private static final Jdbi jdbi = Jdbi.create("jdbc:mysql://localhost:3306/dbname?rewriteBatchedStatements=true");

    @Test
    void fails() {
        String sqlScript = "CREATE PROCEDURE QWE()\n" +
                "BEGIN\n" +
                "END;\n" +
                "SELECT 1 FROM DUAL;\n" +
                "SELECT 1 FROM DUAL;\n" +
                "SELECT 1 FROM DUAL;\n" +
                "SELECT 1 FROM DUAL;\n";
        jdbi.withHandle(h -> h.createScript(sqlScript).execute());
    }
}

Other 4 statements can be anything, used SELECT 1 FROM DUAL; here.

I managed to track this issue down to the place in aws-mysql-jdbc driver: https://github.com/awslabs/aws-mysql-jdbc/blob/820cab7e48dc7f2324ee2522f3d15d673adc3c6b/src/main/user-impl/java/com/mysql/cj/jdbc/StatementImpl.java#L841C25-L841C25
Checking the SQL statements in debug mode there, the difference that I see that after END; there is a trailing semicolon, while for other statements it is removed when preparing a batch. If I run the same test with 3.30.0 JDBI version, I also see no semicolon after END.

[hgschmie]

Hi,

so this does not seem to be specific to the AWS driver. I can make the following test pass:

@Test
    public void testIssue2554() {
        String sqlScript = "CREATE PROCEDURE QWE()\n" +
            "BEGIN\n" +
            "END;\n" +
            "SELECT 1 FROM DUAL;\n" +
            "SELECT 1 FROM DUAL;\n" +
            "SELECT 1 FROM DUAL;\n" +
            "SELECT 1 FROM DUAL;\n";

        int[] result = extension.getJdbi().withHandle(h -> h.createScript(sqlScript).execute());
        assertThat(result).isEqualTo(new int[] {0, -1, -1, -1, -1});
    }

by commenting out these lines: https://github.com/jdbi/jdbi/blob/master/core/src/main/java/org/jdbi/v3/core/internal/SqlScriptParser.java#L114-L116

otherwise I get:

org.jdbi.v3.core.statement.UnableToExecuteStatementException: java.sql.BatchUpdateException: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ';SELECT 1 FROM DUAL;SELECT 1 FROM DUAL;SELECT 1 FROM DUAL;SELECT 1 FROM DUAL' at line 1 [statement:"null", arguments:{positional:{}, named:{}, finder:[]}]

the difference is that the parser right now (with 3.42.0) creates

CREATE PROCEDURE QWE()
BEGIN
END;

SELECT 1 FROM DUAL

SELECT 1 FROM DUAL

SELECT 1 FROM DUAL

SELECT 1 FROM DUAL

as separate statements and with the lines removed, it does

CREATE PROCEDURE QWE()
BEGIN
END

SELECT 1 FROM DUAL

SELECT 1 FROM DUAL

SELECT 1 FROM DUAL

SELECT 1 FROM DUAL

note the missing semicolon at the end of the first statement.

However, removing the lines above fails this test in core/TestScript`

/**
     * <p>
     *   Test for correct handling of semicolons in sql containing begin/end blocks.
     * </p>
     * <p>
     *   Class {@link Script} splits sql scripts into lists of statements by semicolon ({@code ;}) and then batch-executes them.<br>
     *   Statements may contain {@code BEGIN...END} blocks containing subordinated statements (also ending in semicolons).<br>
     *   Only semicolons on the highest level (i.e. outside any block) actually signal the end of an sql statement.
     * </p>
     * @throws SQLException on failure to create the database handle
     */
    @Test
    public void testOracleScriptWithBeginEndBlock() throws SQLException {
        String sql = getClasspathSqlLocator().getResource("script/oracle-with-begin-end-blocks.sql");
        try (Script script = new Script(HandleAccess.createHandle(), sql)) {

            List<String> statements = script.getStatements();

            assertThat(statements).hasSize(3);

            // local variable of CharSequence not String because
            // CharSequence.chars() since Java 1.8 <=> String.chars() since Java 9
            CharSequence lastStmt = statements.get(2);
            assertThat(lastStmt)
                    .startsWith("CREATE OR REPLACE TRIGGER EXAMPLE_TRIGGER")
                    .endsWith("END;")
                    .hasLineCount(15)
                    .has(new Condition<>(s -> 7 == s.chars().filter(ch -> ch == ';').count(), "count semicolons"));
        }
    }

Note that this test exists to explicitly test that there is a semicolon at the end of an END statement.

I feel we are now in the "different SQL dialects" territory, something that we always tried to keep away from. I need someone else to look at this. @stevenschlansker?

[hgschmie]

This boils down to the fix for #2021 (#2051).

[stevenschlansker]

I think the oracle script test is more checking that the script inner semicolons are preserved, not that the end; particularly ends with a ;. If we think updating the test will fix a bug, I'm comfortable with us updating the tests to remove the trailing semicolon check.

[hgschmie]

No, there is specific code in the parser that ensures that the final semicolon stays there. Turns out that oracle does need it while MySQL calls it a syntax error.

The proposed PR (#2559) makes that configurable. I am afraid, @IrinaTerlizhenko as "keeping the semicolon" is the current behavior, you will need to add a minor modification to your code. Assuming that you are using MySQL uniformly, call .getConfig(SqlStatements.class).setScriptStatementsNeedSemicolon(false) for each of your Jdbi objects when they get created.

I actually think that the error message is a red herring / a bug in the driver as the driver behaves dramatically different if rewriteBatchedStatements is not set, but it is nevertheless user visible.

This will be 3.43.0, there are two new bug reports that I would like to glance at briefly before releasing; expect a release later this week.

[hgschmie]

This should be finally laid to rest with the next release.