nanodbc icon indicating copy to clipboard operation
nanodbc copied to clipboard

Cannot execute() a prepared statement more than once

Open lexicalunit opened this issue 6 years ago • 11 comments

From @yktoo on October 28, 2014 12:59

At first glance this is similar to #24, but in my case I'm not selecting but issuing DML statements.

The code is something like:

stmt.prepare(connection, "delete from ABC where key=?");

// some other stuff

void func(int key){
  stmt.bind(0, &key);
  stmt.execute();
}

So in the example above the first execution of func() succeeds, whereas any subsequent one fails at the bind() call. Digging deeper showed prepare_bind() is failing the call to SQLDescribeParam, which returns the HY010 - Function sequence error code.

At the same time, changing the code to

void func(int key){
  stmt.prepare(connection, "delete from ABC where key=?");
  stmt.bind(0, &key);
  stmt.execute();
  stmt.close();
}

eliminates the error, but then of we lose all the benefit of doing statement parsing only once.

The DB is Oracle, just in case that matters.

Copied from original issue: lexicalunit/nanodbc#29

lexicalunit avatar Oct 14 '17 00:10 lexicalunit

I definitely want to make this work properly! Hopefully I will have some free time soon to look into it, but right now my workplace is really focused on ramping up for Black Friday and things are hectic.

lexicalunit avatar Oct 14 '17 00:10 lexicalunit

From @weitjong on July 28, 2015 5:38

I have exactly the opposite problem. After connecting to a database, all the initial DML statements of type create table ... always returning [iODBC][Driver Manager]Function sequence error. I am using nanodbc::execute() method. Only after a first successful execution of DML statement of type insert table ... then the subsequent execution of create table ... would be successful.

Replacing the call with nanodbc::just_execute() solves the problem, however, this is not a solution because the app may not know in advance whether the SQL statement is DML or not.

The exception was thrown by columns() called by auto_bind(). But why it only throws exception initially and not after a first successful execution of insert statement? The insert statement can be any statement inserting data to any existing tables and probably other DML statement type that I haven't tried may have the same side effect to cure this initial problem.

I am using a SQLite-ODBC driver compiled directly from source and installed to iODBC driver manager on a Linux host system. At this moment, I have no idea whether this is a driver-specific bug or something to do with nanodbc.

Thanks in advance for any help and thanks for providing such a great library.

lexicalunit avatar Oct 14 '17 00:10 lexicalunit

From @weitjong on July 31, 2015 12:47

Sorry for the noise, I can confirm that my issue above is due a sqliteodbc driver bug when it was built against iODBC driver manager. Some how this issue does not come up again after I rebuilt the driver against unixODBC driver manager this time. Again, sorry for the noise.

EDIT: In case someone else finding my comment post here. The bug is caused by my program trying to fetch a row when columns() is greater than zero. The sqliteodbc/iODBC somehow erroneously returns columns > 0 after the create table ... statement is executed when it shouldn't.

lexicalunit avatar Oct 14 '17 00:10 lexicalunit

@weitjong Don't worry about it! This is good information to have here because it may help someone else in the future :)

lexicalunit avatar Oct 14 '17 00:10 lexicalunit

@yktoo I have been looking at this, I just haven't made comments here to that effect. I'm not seeing a super easy way to resolve this issue. The problem is that nanodbc ultimately translates into straight C API calls, and the order of those calls is important and also, sadly, particularly strict.

We could make this case work by automatically re-preparing the statement after execution because ODBC (I am pretty sure but correct me if I'm wrong) requires that to have happened to be able to call binding statements without error. However we don't want to make it so nanodbc makes those prepare statements unless the user wants them because that could easily lead to inefficient code paths.

A more elegant solution for your use case might be to encapsulate the data "delete from ABC where key=?" somehow, for example via a functor or lambda or other means (it just depends which would be best for you...).

lexicalunit avatar Oct 14 '17 00:10 lexicalunit

Would it be possible to reopen this issue? I can confirm that it's still present.

From observing nanodbc's behavior here, it looks like this sequence of calls still breaks:

prepare(statement, "sql")
...
statement.bind(0, "data")
statement.bind(1, "data")
just_execute(statement)
...
statement.bind(0, "data") // <-- error on this call
statement.bind(1, "data")
just_execute(statement)

It looks like a has_been_executed flag on statement could fix this; set it on execute and check it within bind calls. This would make the first additional bind call re-prepare, fixing this issue as well at only re-preparing when needed.

Edit: using postgres 11

bgemmill avatar Jun 27 '19 14:06 bgemmill

Feel free to reopen it, but it likely is a driver-specific bug. I heavily use the following pattern with SQL Server and {ODBC Driver 17 for SQL Server} without any issues:

nanodbc::statement s(c);
nanodbc::prepare(s, sql_insert);
for (auto const& r : data)
{
    s.bind(0, r.a);
    s.bind(1, r.b);
    s.bind_strings(0, r.c.data(), r.c.size(), 1);
    ...
    nanodbc::result result = nanodbc::execute(s);
    assert(result.affected_rows() == 1);
}

mloskot avatar Jun 27 '19 16:06 mloskot

@mloskot thanks for the update. Reopening issues seems limited to nanodbc members, so I may have to ask you for that :-)

I'm curious about the driver-specific nature of the bug given the last comment by @lexicalunit; that comment made it sound like nanodbc was missing re-prepare logic due to concerns about doing it twice.

Looking over the code as of commit 77ec87e, it doesn't appear that statement_impl::prepare is called from execute. I don't have any SQL Servers to test on at the moment, but I'm really curious why this works for you. Is it possible that the SQL Server driver is doing the right thing anyway?

It may be worth mentioning that the error seems to come from unixodbc rather than postgres: HY01: [unixODBC][Driver Manager]Function sequence error

bgemmill avatar Jun 27 '19 18:06 bgemmill

@bgemmill I've reopened the issue.

I'm unable to give any meaningful response to your question about why some drivers do work while others don't. I only can confirm, based on experience, that there are differences, some are trivial, some are annoying and some are just bugs. If you read code of the nanodbc tests, you will notice numerous conditional checks depending on database/driver.

Looking over the code as of commit 77ec87e

The 77ec87eba6e75b53fac63189082cf609c4e4280c is "Define NOMINMAX only if not defined". Is this the commit you mean?

It may be worth mentioning that the error seems to come from unixodbc rather than postgres: HY01: [unixODBC][Driver Manager]Function sequence error

Either nanodbc (or its client) calls ODBC functions out of order w.r.t. ODBC specification or the driver does it or the driver manager does something unexpected.

In order to move this issue forward, I'd suggest to add a minimal test case reproducing your problem, e.g. at the end of test/postgresql_test.cpp and submit it as a pull request "Add test for prepared statement issue #56". Then, I will be able to have a look myself.

mloskot avatar Jun 28 '19 06:06 mloskot

@mloskot thanks for your patience, the PR is in.

bgemmill avatar Jul 16 '19 20:07 bgemmill

For anyone else still experiencing this issue, the SOCI library can reuse prepared statements via it's ODBC plugin when connecting to postgres: http://soci.sourceforge.net/doc/release/4.0/statements/#statement-caching

bgemmill avatar Mar 31 '20 15:03 bgemmill