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] PreparedStatement swallows exceptions from SQL batch #995

Closed
fhossfel opened this issue Mar 19, 2019 · 14 comments
Closed

[BUG] PreparedStatement swallows exceptions from SQL batch #995

fhossfel opened this issue Mar 19, 2019 · 14 comments

Comments

@fhossfel
Copy link

fhossfel commented Mar 19, 2019

Driver version

7.2.1.jre8

SQL Server version

Microsoft SQL Server 2017 (RTM-CU13) (KB4466404) - 14.0.3048.4 (X64)
Nov 30 2018 12:57:58
Copyright (C) 2017 Microsoft Corporation
Express Edition (64-bit) on Linux (Ubuntu 16.04.5 LTS)

Client Operating System

Windows 10

JAVA/JVM version

java version "1.8.0_152"
Java(TM) SE Runtime Environment (build 1.8.0_152-b16)
Java HotSpot(TM) 64-Bit Server VM (build 25.152-b16, mixed mode)

Table schema

master

Problem description

  1. Expected behaviour:
    If I run a batch of SQL statements through PreparedStatement I expect any exception encountered to be thrown. This is what happens if you use a Statment

  2. Actual behaviour:
    No exeption is thrown.

  3. Error message/stack trace:
    Nothing!

  4. Any other details that can be helpful:
    This might be related to Throw the initial batchException  #458

Reproduction code

pom.xml

<?xml version="1.0" encoding="UTF-8" ?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    <modelVersion>4.0.0</modelVersion>
    <groupId>de.thoughtgang.otto</groupId>
    <artifactId>sql-server-test</artifactId>
    <version>1.0-SNAPSHOT</version>
    <packaging>jar</packaging>

    <properties>
        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
        <maven.compiler.source>1.8</maven.compiler.source>
        <maven.compiler.target>1.8</maven.compiler.target>
        <maven.surefire.plugin.version>2.22.1</maven.surefire.plugin.version>
        <junit.jupiter.version>5.3.1</junit.jupiter.version>
        <junit.platform.version>1.4.0</junit.platform.version>	
    </properties>

    <dependencies>

        <dependency>
            <groupId>org.junit.jupiter</groupId>
            <artifactId>junit-jupiter-api</artifactId>
            <version>${junit.jupiter.version}</version>
            <scope>test</scope>
        </dependency>
        <dependency>
            <groupId>org.junit.jupiter</groupId>
            <artifactId>junit-jupiter-engine</artifactId>
            <version>${junit.jupiter.version}</version>
            <scope>test</scope>
        </dependency>
        <dependency>
            <groupId>com.microsoft.sqlserver</groupId>
            <artifactId>mssql-jdbc</artifactId>
            <version>7.2.1.jre8</version>
            <scope>test</scope>
        </dependency>
    </dependencies>

    <build>
        <plugins>
            <plugin>
                <artifactId>maven-surefire-plugin</artifactId>
                <version>${maven.surefire.plugin.version}</version>
            </plugin>
        </plugins>
    </build>
`
</project>

SQLServerExceptionTest:

/*
 * To change this license header, choose License Headers in Project Properties.
 * To change this template file, choose Tools | Templates
 * and open the template in the editor.
 */
package de.thoughtgang.microsoft.sqlserver;

import com.microsoft.sqlserver.jdbc.SQLServerException;
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import org.junit.jupiter.api.AfterAll;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.TestInstance.Lifecycle;

/**
 *
 * @author FEHOSSFE
 */
@TestInstance(Lifecycle.PER_CLASS)
public class SQLServerExceptionTest {

    private static final String URL = "jdbc:sqlserver://localhost;databaseName=master";
    private static final String USERNAME = "sa";
    private static final String PASSWORD = "MSSQLServer2017";
    private static final String SQL = "SELECT 1; INSERT INTO test (test) VALUES (23);";

    private Connection con;

    @BeforeAll
    public void beforeAll() throws ClassNotFoundException, SQLException {

        Class.forName("com.microsoft.sqlserver.jdbc.SQLServerDriver");
        con = DriverManager.getConnection(URL, USERNAME, PASSWORD);

    }

    @AfterAll
    public void afterAll() throws SQLException {

        if (con != null && !con.isClosed()) {

            con.close();

        }

    }

    /**
     * This unit test will execute the SQL string consisting of multiple
     * statments but using a Prepared Statement. It fails to detect the error
     * that occurs because there is not table 'test'.
     */
    @Test
    public void testPreparedStatement() {

        SQLServerException sqlex = assertThrows(SQLServerException.class, () -> {
            boolean hasMoreResultSets;
            try (PreparedStatement stmt = con.prepareStatement(SQL);) {

                hasMoreResultSets = stmt.execute();
                System.out.println(hasMoreResultSets);

                while (hasMoreResultSets) {

                    System.out.println("Resultset found");
                    ResultSet rs = stmt.getResultSet();
                    rs.next();
                    String test = rs.getString(1);
                    rs.close();

                    hasMoreResultSets = stmt.getMoreResults();

                }

                System.out.println(stmt.getMoreResults());

            }

        });

        assertEquals("Invalid object name 'test'.", sqlex.getMessage());

    }

    /**
     * This unit test will execute the SQL string consisting of multiple
     * statments and successfully detect that an error has occured.
     */
    @Test
    public void testStatement() {

        SQLServerException sqlex = assertThrows(SQLServerException.class, () -> {
            boolean hasMoreResultSets;
            try (Statement stmt = con.createStatement();) {

                hasMoreResultSets = stmt.execute(SQL);

                System.out.println(hasMoreResultSets);

                 while (hasMoreResultSets) {

                    System.out.println("Resultset found");
                    ResultSet rs = stmt.getResultSet();
                    rs.next();
                    rs.close();

                    hasMoreResultSets = stmt.getMoreResults();

                } 

                System.out.println(stmt.getMoreResults()); 
            }
        });

        assertEquals("Invalid object name 'test'.", sqlex.getMessage());

    }

}
@fhossfel fhossfel added the Bug A bug in the driver. A high priority item that one can expect to be addressed quickly. label Mar 19, 2019
@fhossfel
Copy link
Author

I have also reaised this issue on stackoverflow.com but have not received any replies.

https://stackoverflow.com/questions/55226638/getting-exceptions-from-a-preparedstatement

@ulvii
Copy link
Contributor

ulvii commented Mar 19, 2019

Hi @fhossfel,

The reason why you are seeing this behavior is because the driver does not parse the result sets on execute(), you would need to iterate through result sets to access the exception. Try adding rs.getString(1); after rs.next() in testPreparedStatement. Please take a look at the issues #826, #937 for detailed explanation.

@fhossfel
Copy link
Author

This does not work. Since the INSERT Statement does not return a ResultSet calling getMoreResults() returns false. Hence I do not have a RequltSet from the failing statement to call getString(1) on.

@ulvii
Copy link
Contributor

ulvii commented Mar 19, 2019

Please try:

@TestInstance(Lifecycle.PER_CLASS)
public class SQLServerExceptionTest {

    private static final String URL = "jdbc:sqlserver://localhost;databaseName=master";
    private static final String USERNAME = "sa";
    private static final String PASSWORD = "MSSQLServer2017";
    private static final String SQL = "SELECT 1; INSERT INTO test (test) VALUES (23);";

    private Connection con;

    @BeforeAll
    public void beforeAll() throws ClassNotFoundException, SQLException {

        Class.forName("com.microsoft.sqlserver.jdbc.SQLServerDriver");
        con = DriverManager.getConnection(URL, USERNAME, PASSWORD);

    }

    @AfterAll
    public void afterAll() throws SQLException {

        if (con != null && !con.isClosed()) {

            con.close();

        }

    }

    /**
     * This unit test will execute the SQL string consisting of multiple
     * statments but using a Prepared Statement. It fails to detect the error
     * that occurs because there is not table 'test'.
     */
    @Test
    public void testPreparedStatement() {

        SQLServerException sqlex = assertThrows(SQLServerException.class, () -> {
            boolean hasMoreResultSets;
            try (PreparedStatement stmt = con.prepareStatement(SQL);) {

                hasMoreResultSets = stmt.execute();
                System.out.println(hasMoreResultSets);

                while (hasMoreResultSets) {

                    System.out.println("Resultset found");
                    ResultSet rs = stmt.getResultSet();
                    while(rs.next()) {rs.getString(1);}
                    rs.close();

                    hasMoreResultSets = stmt.getMoreResults();

                }

                System.out.println(stmt.getMoreResults());

            }

        });

        assertEquals("Invalid object name 'test'.", sqlex.getMessage());

    }

    /**
     * This unit test will execute the SQL string consisting of multiple
     * statments and successfully detect that an error has occured.
     */
    @Test
    public void testStatement() {

        SQLServerException sqlex = assertThrows(SQLServerException.class, () -> {
            boolean hasMoreResultSets;
            try (Statement stmt = con.createStatement();) {

                hasMoreResultSets = stmt.execute(SQL);

                System.out.println(hasMoreResultSets);

                 while (hasMoreResultSets) {

                    System.out.println("Resultset found");
                    ResultSet rs = stmt.getResultSet();
                    while(rs.next()) {rs.getString(1);}
                    rs.close();

                    hasMoreResultSets = stmt.getMoreResults();

                } 

                System.out.println(stmt.getMoreResults()); 
            }
        });

        assertEquals("Invalid object name 'test'.", sqlex.getMessage());

    }

}

@fhossfel
Copy link
Author

This does not work. You will get the error message "Failed: Expected com.microsoft.sqlserver.jdbc.SQLServerException to be thrown, but nothing was thrown." from the unit test.

@ulvii ulvii removed the Bug A bug in the driver. A high priority item that one can expect to be addressed quickly. label Mar 19, 2019
@ulvii
Copy link
Contributor

ulvii commented Mar 19, 2019

I ran the test below and it seems to pass. Please make sure to update your project.

Please try:

@TestInstance(Lifecycle.PER_CLASS)
public class SQLServerExceptionTest {

    private static final String URL = "jdbc:sqlserver://localhost;databaseName=master";
    private static final String USERNAME = "sa";
    private static final String PASSWORD = "MSSQLServer2017";
    private static final String SQL = "SELECT 1; INSERT INTO test (test) VALUES (23);";

    private Connection con;

    @BeforeAll
    public void beforeAll() throws ClassNotFoundException, SQLException {

        Class.forName("com.microsoft.sqlserver.jdbc.SQLServerDriver");
        con = DriverManager.getConnection(URL, USERNAME, PASSWORD);

    }

    @AfterAll
    public void afterAll() throws SQLException {

        if (con != null && !con.isClosed()) {

            con.close();

        }

    }

    /**
     * This unit test will execute the SQL string consisting of multiple
     * statments but using a Prepared Statement. It fails to detect the error
     * that occurs because there is not table 'test'.
     */
    @Test
    public void testPreparedStatement() {

        SQLServerException sqlex = assertThrows(SQLServerException.class, () -> {
            boolean hasMoreResultSets;
            try (PreparedStatement stmt = con.prepareStatement(SQL);) {

                hasMoreResultSets = stmt.execute();
                System.out.println(hasMoreResultSets);

                while (hasMoreResultSets) {

                    System.out.println("Resultset found");
                    ResultSet rs = stmt.getResultSet();
                    while(rs.next()) {rs.getString(1);}
                    rs.close();

                    hasMoreResultSets = stmt.getMoreResults();

                }

                System.out.println(stmt.getMoreResults());

            }

        });

        assertEquals("Invalid object name 'test'.", sqlex.getMessage());

    }

    /**
     * This unit test will execute the SQL string consisting of multiple
     * statments and successfully detect that an error has occured.
     */
    @Test
    public void testStatement() {

        SQLServerException sqlex = assertThrows(SQLServerException.class, () -> {
            boolean hasMoreResultSets;
            try (Statement stmt = con.createStatement();) {

                hasMoreResultSets = stmt.execute(SQL);

                System.out.println(hasMoreResultSets);

                 while (hasMoreResultSets) {

                    System.out.println("Resultset found");
                    ResultSet rs = stmt.getResultSet();
                    while(rs.next()) {rs.getString(1);}
                    rs.close();

                    hasMoreResultSets = stmt.getMoreResults();

                } 

                System.out.println(stmt.getMoreResults()); 
            }
        });

        assertEquals("Invalid object name 'test'.", sqlex.getMessage());

    }

}

@fhossfel
Copy link
Author

Thanks for the reply. I can run your unit successfully: The error is thrown. I I have analyzed this correctly you need to retrieve the complete ResultSet of the previous statment to get to the error.

I think this is a bug nevertheless - or at least very surprsing. The JDBC spec says in section 14.1.4:

Error handling in the case of PreparedStatement objects is the same as error
handling in the case of Statement objects. Some drivers may stop processing as
soon as an error occurs, while others may continue processing the rest of the batch.

I admit, that this is in the section Batch Updates but I would argue that the general idea behind PreparedStatement/Statement is that they behave similar. As a matter of fact: I think that this is the whole idea behind inhertance in OOP.

Currently the PreparedStatement has a lot of nasty surprises. I did not expect:

a.) that the error is not thrown automatically upon exectuion
b.) the error does not occur when asking for the resultset of the error
c.) it does only occur when retrieving all rows of a resultset of a previous statement that is unrelated to the exception that has occured.

I don't think there is any other JDBC driver that behaves that way.

@fhossfel
Copy link
Author

BTW: What do you do if you have a combination of one INSERT/UPDATE that runs and successfully and a second one that fails? Do you get an exception straight away?

@ulvii
Copy link
Contributor

ulvii commented Mar 20, 2019

@fhossfel ,

I admit, that this is in the section Batch Updates but I would argue that the general idea behind PreparedStatement/Statement is that they behave similar.

They do indeed behave the same in this scenario. Please try to debug both testStatement() and testPreparedStatement(), both methods throw an exception when calling rs.getString(1) and parsing the result set, not on execute().

a.) that the error is not thrown automatically upon exectuion
b.) the error does not occur when asking for the resultset of the error
c.) it does only occur when retrieving all rows of a resultset of a previous statement that is unrelated to the exception that has occured.

In your scenario, the server returns the exception as a result set and there is not much the driver can do but let the application parse the result set.

BTW: What do you do if you have a combination of one INSERT/UPDATE that runs and successfully and a second one that fails? Do you get an exception straight away?

I would again suggest to take a look at the issues #826, #937 for the detailed explanation about the current behavior.

I don't think there is any other JDBC driver that behaves that way.

I just confirmed that jTDS - another SQL Server JDBC driver behaves exactly the same.

Please let me know if you have any other questions.

@fhossfel
Copy link
Author

They do indeed behave the same in this scenario. Please try to debug both testStatement() and testPreparedStatement(), both methods throw an exception when calling rs.getString(1) and parsing the result set, not on execute().

No, that's not true. The statement test case throws the exception without having to call getString(1) on the result set. Does the SQL Server handle Statement and PreparedStatement differently or is this a difference in the driver?

In your scenario, the server returns the exception as a result set and there is not much the driver can do but let the application parse the result set.

Sorry, if I did not understand this correctly: Does this mean I get the data from two different statements in one ResultSet? I mean, does the server return one ResultSet which may have completely different meta data per row. I would have expected that I get two different ResultSets for two different SELECTs and that I use stmt.getMoreResults() to access them.

IMHO the issues #826 and #937 are a strong indicator that I am not the only one who is, ummm, "surprised" by this behavior. I can confirm that Pentaho Data Integration is not usable with SQL Server. SO this problem seems to be fairly widespread.

@fhossfel
Copy link
Author

Hi @ulvii,

sorry to bother you again but this is really quite a pressing issue for us. I have just adapter the unit test to use jTDS and it does not behave the same. It will throw and java.sql.SQLException without having to loop through the rows of a ResultSet or even getting the first row of the ResultSet. It is sufficient to call
getMoreResults() to retrieve all ResultSets.

package de.thoughtgang.microsoft.sqlserver;

import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.PreparedStatement;
import java.sql.SQLException;
import org.junit.jupiter.api.AfterAll;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.TestInstance.Lifecycle;


@TestInstance(Lifecycle.PER_CLASS)
public class jTDSExceptionTest {

    private static final String URL = "jdbc:jtds:sqlserver://localhost;databaseName=master";
    private static final String USERNAME = "sa";
    private static final String PASSWORD = "MSSQLServer2017";
    private static final String SQL = "SELECT 1; INSERT INTO test (test) VALUES (23);";

    private Connection con;

    @BeforeAll
    public void beforeAll() throws ClassNotFoundException, SQLException {

        Class.forName("net.sourceforge.jtds.jdbc.Driver");
        con = DriverManager.getConnection(URL, USERNAME, PASSWORD);

    }

    @AfterAll
    public void afterAll() throws SQLException {

        if (con != null && !con.isClosed()) {

            con.close();

        }

    }

    /**
     * This unit test will execute the SQL string consisting of multiple
     * statments but using a Prepared Statement. It fails to detect the error
     * that occurs because there is not table 'test'.
     */
    @Test
    public void testPreparedStatement() throws SQLException {

        SQLException sqlex = assertThrows(SQLException.class, () -> {

            try (PreparedStatement stmt = con.prepareStatement(SQL);) {

                stmt.execute();
                while (stmt.getMoreResults()) {}

            }

        });

        assertEquals("Invalid object name 'test'.", sqlex.getMessage());

    }

}

        assertEquals("Invalid object name 'test'.", sqlex.getMessage());

    }

}

pom.xml:

    <dependency>
        <groupId>net.sourceforge.jtds</groupId>
        <artifactId>jtds</artifactId>
        <version>1.3.1</version>
    </dependency>

@ulvii
Copy link
Contributor

ulvii commented Mar 21, 2019

Hi @fhossfel ,

You are actually correct that PreparedStatement behaves differently and the difference seems to be introduced with this PR. Would you mind trying 6.4 version of the driver and let me know if it behaves the way you expect? In the meantime I will continue the investigations.

@fhossfel
Copy link
Author

Hi @ulvii,

I can confirm that the unit test runs successfully with 6.4.0.jre8. I think I should be able to downgrade the driver to that version. Still, it would be nice if this could be addressed in a future release.

Thanks

Felix

@ulvii
Copy link
Contributor

ulvii commented Mar 22, 2019

Hi @fhossfel,

I created a PR that reverts #664, The change will be reviewed by the team for one of the upcoming releases. Thank you for bringing the issue to our attention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants