xerial / sqlite-jdbc

SQLite JDBC Driver
Apache License 2.0
2.79k stars 611 forks source link

SQLException: The prepared statement has been finalized #731

Open bajinsheng opened 2 years ago

bajinsheng commented 2 years ago

Hi, when I execute the query ";" in the JDBC and get the "The prepared statement has been finalized" error, but there is no error when I execute the query ";" in the cli.

Reproducible test case:

...
Connection connection = connection = DriverManager.getConnection("jdbc:sqlite:sample.db");
Statement statement = connection.createStatement();
statement.executeUpdate(";");
...

The stack trace:

java.lang.AssertionError: ;
        at Sample.Main(Sample.java:17)
Caused by: java.sql.SQLException: The prepared statement has been finalized
        at org.sqlite.core.NativeDB.throwex(NativeDB.java:406)
        at org.sqlite.core.NativeDB.step(Native Method)
        at org.sqlite.core.DB.execute(DB.java:962)
        at org.sqlite.core.CoreStatement.exec(CoreStatement.java:74)
        at org.sqlite.jdbc3.JDBC3Statement.execute(JDBC3Statement.java:41)

SQLite3 JDBC version: 3.36.0.3

Is it expected?

gotson commented 2 years ago

I'm wondering why you would execute a single command containing only ; though.

bajinsheng commented 2 years ago

It may be randomly generated by my application when converting external data to SQL query.

gotson commented 2 years ago

I just tried the following:

@Test
    public void issue731() throws Exception {
        Connection conn = getConnection();
        Statement stmt = conn.createStatement();
        stmt.executeUpdate(";");
        stmt.close();
        conn.close();
    }

And i don't get any exception.

Can you try with the latest snapshot version to confirm on your side?

pyckle commented 2 years ago

@bajinsheng - I think you made a mistake in your test case that fails - I think you intended to call executeQuery(), not executeUpdate(). executeQuery() is a noop. See: https://www.sqlite.org/c3ref/exec.html

I think the bug here is that we throw the wrong error from the wrong place.

Internally, sqlite3 gives us a null pointer from sqlite3_prepare_v2 (https://www.sqlite.org/draft/c3ref/prepare.html) when we create this query.

Oddly enough, I don't get an error from the return code from this function. I don't see that the sqlite3 documentation defines the return code for this scenario, but it's definitely possible that I am missing something.

From the docs:

ppStmt is left pointing to a compiled prepared statement that can be executed using sqlite3_step(). If there is an error, ppStmt is set to NULL. If the input text contains no SQL (if the input is an empty string or a comment) then *ppStmt is set to NULL. The calling procedure is responsible for deleting the compiled SQL statement using sqlite3_finalize() after it has finished with it. ppStmt may not be NULL.

Anyways, the exception is thrown because the pointer is set to null with a successful return code. When we try to use it, we check whether it's null first, and throw the already closed exception if it is.

@gotson - I think the issue here is that we should be checking for null in the prepared statement made with sqlite3_prepare_v2. If it is null, we should throw an SQLException that ";" is an invalid query, rather than bombing later. Alternatively we could return a empty ResultSet. What do you think? I haven't seen anything within the JDBC spec for what to do when a noop query is run.

gotson commented 2 years ago

@pyckle i was thinking that in doubt we should check what's the behaviour with another JDBC driver, maybe the PostgreSQL one. What do you think?

I know SQLite itself is usually following the PG implementation.