postgresql-interfaces / psqlodbc

Other
16 stars 13 forks source link

syntax error at or near "ROWS" #43

Open KristianIvarsson opened 1 month ago

KristianIvarsson commented 1 month ago

PostgreSQL version: 16.4 (SQLGetInfo with SQL_DBMS_NAME says version 16.0.4 though) Microsoft Windows 10 Enterprise Version 10.0.19044 PSQLODBC35W.DLL version 16.00.0005

When executing a statement SELECT "Id","Begin","End","Logfile" FROM "ServerSession" ORDER BY "Begin" ASC OFFSET ? ROWS FETCH FIRST ? ROW ONLY with binding parameters, an error syntax error at or near "ROWS" is issued

Changing to SELECT "Id","Begin","End","Logfile" FROM "ServerSession" ORDER BY "Begin" ASC OFFSET ? LIMIT ? executes successfully, but that is not standard SQL and thus not wanted

Extending the test a bit reveals some fishy stuff though

Executing SELECT "SELECT "Id","Begin","End","Logfile" FROM "ServerSession" WHERE "Logfile" = ? ORDER BY "Begin" ASC OFFSET ? ROWS FETCH FIRST ? ROW ONLY and having a string "abcde" as first parameter works, but not with the string "abc"

Looking at the postmaster log looks even more confusing

With FETCH FIRST-statement

With "abc" (failure)

2024-09-19 12:20:26.092 CEST [17828] STATEMENT:  SELECT "Id","Begin","End","Logfile" FROM "ServerSession" WHERE "Logfile" = $1 ORDER BY "Begin" ASC OFFSET $2::int4 ROWS FETCH FIRST $3::int4 ROW ONLY
2024-09-19 12:20:26.092 CEST [17828] ERROR:  syntax error at or near "ROWS" at character 116

With "abcde" (success)

2024-09-19 12:20:26.092 CEST [17828] LOG:  execute <unnamed>: SELECT "Id","Begin","End","Logfile" FROM "ServerSession" WHERE "Logfile" = $1 ORDER BY "Begin" ASC OFFSET $2 ROWS FETCH FIRST $3 ROW ONLY
2024-09-19 12:20:26.092 CEST [17828] DETAIL:  parameters: $1 = 'abcde', $2 = '0', $3 = '1'

With LIMIT-statement

With "abc" (success)

2024-09-19 12:23:40.386 CEST [32684] LOG:  execute <unnamed>: SELECT "Id","Begin","End","Logfile" FROM "ServerSession" WHERE "Logfile" = $1 ORDER BY "Begin" ASC OFFSET $2::int4 LIMIT $3::int4
2024-09-19 12:23:40.386 CEST [32684] DETAIL:  parameters: $1 = 'abc', $2 = '0', $3 = '1'

With "abcde" (success)

2024-09-19 12:23:40.386 CEST [32684] LOG:  execute <unnamed>: SELECT "Id","Begin","End","Logfile" FROM "ServerSession" WHERE "Logfile" = $1 ORDER BY "Begin" ASC OFFSET $2 LIMIT $3
2024-09-19 12:23:40.386 CEST [32684] DETAIL:  parameters: $1 = 'abcde', $2 = '0', $3 = '1'

In all the "abc" test-cases cbColDef and cbValueMax are 3 to SQLBindParameter() In all the "abcde" test-cases cbColDef and cbValueMax are 5 SQLBindParameter()

Why the integer parameters are serialized as $2int4 and $3int4 when $1 is "abc" and as $2 and $3 when $1 is "abcde" is a bit weird but maybe irrelevant

Maybe needles to say, but the top SQL-statement works for numerous other ODBC drivers to other DBMS

davecramer commented 1 month ago

Thanks for the report.

davecramer commented 1 month ago

It would be useful to provide test code and the table definition?

Dave

KristianIvarsson commented 1 month ago

Table definition

CREATE TABLE "ServerSession" (
    "Id" uuid NOT NULL,
    "Begin" timestamp without time zone,
    "End" timestamp without time zone,
    "Logfile" character varying(255)
);

ALTER TABLE ONLY "ServerSession" ADD CONSTRAINT "ServerSession_pkey" PRIMARY KEY ("Id");

CREATE INDEX "Indx_ServerSession_Begin" ON "ServerSession" USING btree ("Begin") WITH (fillfactor='90', deduplicate_items='true');

CREATE INDEX "Indx_ServerSession_End" ON "ServerSession" USING btree ("End") WITH (fillfactor='90', deduplicate_items='true');
davecramer commented 1 month ago

So I wrote some code and test this. Slightly different table works fine.

create table serversession("Id" serial , "Begin" int4, "End" int4, logfile text);
rc = SQLExecDirect(hstmt, (SQLCHAR *) "SET intervalstyle=postgres_verbose", SQL_NTS);
    /* Prepare a statement */
    rc = SQLPrepare(hstmt, (SQLCHAR *) "SELECT \"Id\",\"Begin\",\"End\",\"Logfile\" FROM ServerSession ORDER BY \"Begin\" ASC OFFSET ? ROWS FETCH FIRST ? ROW ONLY", SQL_NTS);
    CHECK_STMT_RESULT(rc, "SQLPrepare failed", hstmt);

    /* bind param  */
    longparam1 = 0;
    cbParam1 = sizeof(longparam1);
    rc = SQLBindParameter(hstmt, 1, SQL_PARAM_INPUT,
                          SQL_C_SLONG,  /* value type */
                          SQL_INTEGER,  /* param type */
                          0,            /* column size (ignored for SQL_INTEGER) */
                          0,            /* dec digits */
                          &longparam1,  /* param value ptr */
                          sizeof(longparam1), /* buffer len (ignored for SQL_INTEGER) */
                          &cbParam1     /* StrLen_or_IndPtr (ignored for SQL_INTEGER) */);
    CHECK_STMT_RESULT(rc, "SQLBindParameter failed", hstmt);

    /* bind param  */
    longparam2 = 3;
    cbParam2 = sizeof(longparam2);
    rc = SQLBindParameter(hstmt, 2, SQL_PARAM_INPUT,
                          SQL_C_SLONG,  /* value type */
                          SQL_INTEGER,  /* param type */
                          0,            /* column size (ignored for SQL_INTEGER) */
                          0,            /* dec digits */
                          &longparam2,  /* param value ptr */
                          sizeof(longparam2), /* buffer len (ignored for SQL_INTEGER) */
                          &cbParam2     /* StrLen_or_IndPtr (ignored for SQL_INTEGER) */);
    CHECK_STMT_RESULT(rc, "SQLBindParameter failed", hstmt);

    /* Test SQLNumResultCols, called before SQLExecute() */
    rc = SQLNumResultCols(hstmt, &colcount);
    CHECK_STMT_RESULT(rc, "SQLNumResultCols failed", hstmt);
    printf("# of result cols: %d\n", colcount);

    /* Execute */
    rc = SQLExecute(hstmt);
    CHECK_STMT_RESULT(rc, "SQLExecute failed", hstmt);

    /* Fetch result */
    print_result(hstmt);

    rc = SQLFreeStmt(hstmt, SQL_CLOSE);
    CHECK_STMT_RESULT(rc, "SQLFreeStmt failed", hstmt);

    /* Clean up */
    test_disconnect();
KristianIvarsson commented 1 month ago

Cool, the best option is if there’s a defect on our side

I’ll try to narrow down our code and reproduce the error whenever I’ll have a computer (using phone now)

KristianIvarsson commented 1 month ago

Okey, I now read your comment more thoroughly @davecramer and the "Slightly different table works fine"

Do you mean that you can reproduce the error with our table design but not with your table design ? If so, that is not a proper hotfix for us and this error occurs occationally for other tables with other layout as well. It ends up with this error even if you're doing a SELECT with a non existing table.

However, here's some code that reproduces the error. Compiled with Visual Studio 17.12.0 Preview 2.0 and /std:c++latest (in case the std::println() function doesn't work)

#ifdef UNICODE
#undef UNICODE
#endif

#include <windows.h>

#include <sqltypes.h>
#include <sqlext.h>
#include <sql.h>

#include <cassert>

#include <print>

void evaluate(const SQLUSMALLINT type, const SQLHANDLE handle, const SQLRETURN result)
{
   switch(result)
   {
   case SQL_SUCCESS:
   case SQL_SUCCESS_WITH_INFO:
      break;
   default:
      SQLCHAR state[5 + 1] = {0};
      SQLCHAR message[SQL_MAX_MESSAGE_LENGTH + 1] = {0};
      SQLSMALLINT length = sizeof(message);

      SQLGetDiagRec(type, handle, 1, state, nullptr, message, length, &length);

      std::println("{} [state={}] [result={}]", (const char*)message, (const char*)state, result);

      assert(false);
   }
}

int main()
{
   SQLHANDLE environment;
   evaluate(SQL_HANDLE_ENV, environment, SQLAllocHandle(SQL_HANDLE_ENV, SQL_NULL_HANDLE, &environment));
   evaluate(SQL_HANDLE_ENV, environment, SQLSetEnvAttr(environment, SQL_ATTR_ODBC_VERSION, reinterpret_cast<void*>(SQL_OV_ODBC3), 0));
   SQLHANDLE connection;
   evaluate(SQL_HANDLE_DBC, connection, SQLAllocHandle(SQL_HANDLE_DBC, environment, &connection));
   evaluate(SQL_HANDLE_DBC, connection, SQLDriverConnect(connection, NULL, (SQLCHAR*)"DSN=TestPG", SQL_NTS, NULL, 0, NULL, SQL_DRIVER_NOPROMPT));
   SQLHANDLE statement;
   evaluate(SQL_HANDLE_STMT, statement, SQLAllocHandle(SQL_HANDLE_STMT, connection, &statement));

   SQLSMALLINT number = 0;
   long page = 25;
   evaluate(SQL_HANDLE_STMT, statement, SQLBindParameter(statement, ++number, SQL_PARAM_INPUT, SQL_C_SLONG, SQL_INTEGER, 0, 0, &page, sizeof(page), NULL));
   long size = 25;
   evaluate(SQL_HANDLE_STMT, statement, SQLBindParameter(statement, ++number, SQL_PARAM_INPUT, SQL_C_SLONG, SQL_INTEGER, 0, 0, &size, sizeof(size), NULL));

   SQLCHAR sql[] = R"(SELECT "Id" FROM "ServerSession" ORDER BY "Begin" ASC OFFSET ? ROWS FETCH FIRST ? ROWS ONLY)";
   evaluate(SQL_HANDLE_STMT, statement, SQLExecDirect(statement, sql, sizeof(sql)));

   return 0;
}
davecramer commented 4 weeks ago

No, what I meant to say was that my code runs fine, the only difference is that start and end are integers whereas your code has timestamps.

I'll look at your code shortly

davecramer commented 4 weeks ago

I changed my table definition to match yours and have no issues with the code I put up above

KristianIvarsson commented 4 weeks ago

Yeah, but have you tried my code @davecramer ? You’re using prepared statement, but we’re using SQLExecDirect() (and maybe some other differences) and we’re getting the error despite addressing a bogus table even, so I think the table design is irrelevant in order to reproduce the error .

davecramer commented 4 weeks ago

I'll try your code today

davecramer commented 3 weeks ago

OK, I was able to replicate your problem.

Seems the issue is that only prepared statements will allow you to bind parameters. If I run your code the backend log looks like SELECT "Id" FROM ServerSession ORDER BY "Begin" ASC OFFSET $1::int4 ROWS FETCH FIRST $2::int4 ROWS ONLY

You can see the parameters were not bound.

Dave

KristianIvarsson commented 3 weeks ago

Possibly and I didn't know that it meant they weren't bound

It seems like the parameters are bound if using OFFSET ? LIMIT ?-statement, and SQLExecuteDirect() so it' doesn't seem to be just about that

As I wrote in the mailing list, OFFSET ? ROWS FETCH FIRST ? ROWS ONLY statement seems to work if you add a 3rd string binding parameter in some circumstances (it'd better to read the mailing list thread)

https://www.postgresql.org/message-id/DB9PR01MB9512A7C9601A507899D1D465F1632%40DB9PR01MB9512.eurprd01.prod.exchangelabs.com

davecramer commented 3 weeks ago

Well that seems like a bug. I'll see if I can replicate it.