Thread: SQLExecute returns 0 rows with BoolsAsChar=1
If you SQLPrepare a query, and bind a VARCHAR parameter with column_size of exactly 5, and you have BoolsAsChar=1 (which is the default), the subsequent SQLExecute will return 0 rows. With those conditions, HowToPrepareBeforeExec returns shouldExec, which means that the query is sent to the server to be parse, in a Parse message. PGAPI_Execute() calls HowToPrepareBeforeExec, and calls prepareParameters(), which sends the Parse message, and reads the resulting ParameterDescription messages from the server. It also creates a new QResultClass object, and stores it as the statement's result. Now, when PGAPI_Execute() proceeds to execute the statement for real, it doesn't clear the dummy result set created by prepareParameters(), but just appends the real result set to the statement object's chain of results. When SQLExecute() returns, the first result set (the one that the application cares about) is the dummy one, with 0 rows. As a quick fix, but which I believe is quite safe and correct anyway, is to always clear any existing result sets in PGAPI_Execute() before actually executing the query. Normally, there shouldn't be any result sets, so clearing is harmless. In this buggy corner, case it clears the dummy result set, fixing the problem. Attached is a patch to fix that. As usual, this is also present in my github repository at https://github.com/hlinnaka/psqlodbc/, as well as a test case in the master-with-testcases branch. - Heikki
Attachment
Hi, (2013/03/19 7:25), Heikki Linnakangas wrote: > If you SQLPrepare a query, and bind a VARCHAR parameter with column_size > of exactly 5, and you have BoolsAsChar=1 (which is the default), the > subsequent SQLExecute will return 0 rows. > > With those conditions, HowToPrepareBeforeExec returns shouldExec, which > means that the query is sent to the server to be parse, in a Parse > message. PGAPI_Execute() calls HowToPrepareBeforeExec, and calls > prepareParameters(), which sends the Parse message, and reads the > resulting ParameterDescription messages from the server. It also creates > a new QResultClass object, and stores it as the statement's result. Now, > when PGAPI_Execute() proceeds to execute the statement for real, it > doesn't clear the dummy result set created by prepareParameters(), but > just appends the real result set to the statement object's chain of > results. When SQLExecute() returns, the first result set (the one that > the application cares about) is the dummy one, with 0 rows. > > As a quick fix, but which I believe is quite safe and correct anyway, is I don't think it's safe. It's in the middle of handling an array of parameter set and may go back to next_param_row label. In addtion, apps may get out of PGAPI_Execute() in the middle of paramater handling when there are data at execution parameters. > to always clear any existing result sets in PGAPI_Execute() before > actually executing the query. Normally, there shouldn't be any result > sets, so clearing is harmless. In this buggy corner, case it clears the > dummy result set, fixing the problem. > > Attached is a patch to fix that. As usual, this is also present in my > github repository at https://github.com/hlinnaka/psqlodbc/, as well as a > test case in the master-with-testcases branch. > > - Heikki
On 20.03.2013 10:56, Hiroshi Inoue wrote: > Hi, > > (2013/03/19 7:25), Heikki Linnakangas wrote: >> If you SQLPrepare a query, and bind a VARCHAR parameter with column_size >> of exactly 5, and you have BoolsAsChar=1 (which is the default), the >> subsequent SQLExecute will return 0 rows. >> >> With those conditions, HowToPrepareBeforeExec returns shouldExec, which >> means that the query is sent to the server to be parse, in a Parse >> message. PGAPI_Execute() calls HowToPrepareBeforeExec, and calls >> prepareParameters(), which sends the Parse message, and reads the >> resulting ParameterDescription messages from the server. It also creates >> a new QResultClass object, and stores it as the statement's result. Now, >> when PGAPI_Execute() proceeds to execute the statement for real, it >> doesn't clear the dummy result set created by prepareParameters(), but >> just appends the real result set to the statement object's chain of >> results. When SQLExecute() returns, the first result set (the one that >> the application cares about) is the dummy one, with 0 rows. >> >> As a quick fix, but which I believe is quite safe and correct anyway, is > > I don't think it's safe. > It's in the middle of handling an array of parameter set and may > go back to next_param_row label. Ah, I see. > In addtion, apps may get out of PGAPI_Execute() in the middle of > paramater handling when there are data at execution parameters. Hmm, I don't think that's a problem. Even with data-at-execution parameters, the statement is only really going to be executed once. Until all data-at-execution parameters have filled in by the application, PGAPI_Execute() won't actually execute the statement, it will just return SQL_NEED_DATA. It's OK to reset the result set between those PGAPI_Execute() calls - there shouldn't be any results yet. The last PGAPI_Execute() call produces all the result sets. I think we're OK if we just move the SC_set_Result call to before the next_param_row label. Does the attached look right to you? - Heikki
Attachment
(2013/03/20 22:04), Heikki Linnakangas wrote: > On 20.03.2013 10:56, Hiroshi Inoue wrote: >> Hi, >> >> (2013/03/19 7:25), Heikki Linnakangas wrote: >>> If you SQLPrepare a query, and bind a VARCHAR parameter with column_size >>> of exactly 5, and you have BoolsAsChar=1 (which is the default), the >>> subsequent SQLExecute will return 0 rows. >>> >>> With those conditions, HowToPrepareBeforeExec returns shouldExec, which >>> means that the query is sent to the server to be parse, in a Parse >>> message. PGAPI_Execute() calls HowToPrepareBeforeExec, and calls >>> prepareParameters(), which sends the Parse message, and reads the >>> resulting ParameterDescription messages from the server. It also creates >>> a new QResultClass object, and stores it as the statement's result. Now, >>> when PGAPI_Execute() proceeds to execute the statement for real, it >>> doesn't clear the dummy result set created by prepareParameters(), but >>> just appends the real result set to the statement object's chain of >>> results. When SQLExecute() returns, the first result set (the one that >>> the application cares about) is the dummy one, with 0 rows. >>> >>> As a quick fix, but which I believe is quite safe and correct anyway, is >> >> I don't think it's safe. >> It's in the middle of handling an array of parameter set and may >> go back to next_param_row label. > > Ah, I see. > >> In addtion, apps may get out of PGAPI_Execute() in the middle of >> paramater handling when there are data at execution parameters. > > Hmm, I don't think that's a problem. Even with data-at-execution > parameters, the statement is only really going to be executed once. The statement may be executed many (at most parameter set array size) times in the current implementation. > Until all data-at-execution parameters have filled in by the > application, PGAPI_Execute() won't actually execute the statement, it > will just return SQL_NEED_DATA. It's OK to reset the result set between > those PGAPI_Execute() calls - there shouldn't be any results yet. The > last PGAPI_Execute() call produces all the result sets.
(2013/03/19 7:25), Heikki Linnakangas wrote: > If you SQLPrepare a query, and bind a VARCHAR parameter with column_size > of exactly 5, and you have BoolsAsChar=1 (which is the default), the > subsequent SQLExecute will return 0 rows. Hmm I can't reproduce it here. Could you send me the Mylog output of the case? regards, Hiroshi Inoue
On 21.03.2013 14:19, Hiroshi Inoue wrote: > (2013/03/19 7:25), Heikki Linnakangas wrote: >> If you SQLPrepare a query, and bind a VARCHAR parameter with column_size >> of exactly 5, and you have BoolsAsChar=1 (which is the default), the >> subsequent SQLExecute will return 0 rows. > > Hmm I can't reproduce it here. > Could you send me the Mylog output of the case? Sure. Here's a test program to reproduce it, and the mylog output from running it. It should print: got result: foobar But due to the bug, you get: SQLFetch returned SQL_NO_DATA - Heikki
Attachment
(2013/03/22 1:42), Heikki Linnakangas wrote: > On 21.03.2013 14:19, Hiroshi Inoue wrote: >> (2013/03/19 7:25), Heikki Linnakangas wrote: >>> If you SQLPrepare a query, and bind a VARCHAR parameter with column_size >>> of exactly 5, and you have BoolsAsChar=1 (which is the default), the >>> subsequent SQLExecute will return 0 rows. >> >> Hmm I can't reproduce it here. >> Could you send me the Mylog output of the case? > > Sure. Here's a test program to reproduce it, and the mylog output from > running it. Thanks. I can reproduce it now when I turn off the *Server side prepare* option or turn on the *Use Declare/Fetch* option. Attached is a patch to fix the bug. regards, Hiroshi Inoue
Attachment
On 24.03.2013 12:07, Hiroshi Inoue wrote: > I can reproduce it now when I turn off the *Server side prepare* option > or turn on the *Use Declare/Fetch* option. Ah yeah, UseServerSidePrepare hides it. I'm also seeing the problem without UseDeclareFetch, though. > Attached is a patch to fix the bug. Thanks. I must admit I don't understand how this works. Also, the 2nd test case in attached file now fails. It does a DELETE ... RETURNING, binding a parameter array with a VARCHAR(5) column, and fetches the results. With the patch, it can't fetch the result sets. - Heikki
Attachment
(2013/03/25 20:44), Heikki Linnakangas wrote: > On 24.03.2013 12:07, Hiroshi Inoue wrote: >> I can reproduce it now when I turn off the *Server side prepare* option >> or turn on the *Use Declare/Fetch* option. > > Ah yeah, UseServerSidePrepare hides it. I'm also seeing the problem > without UseDeclareFetch, though. > >> Attached is a patch to fix the bug. > > Thanks. I must admit I don't understand how this works. Also, the 2nd > test case in attached file now fails. It does a DELETE ... RETURNING, > binding a parameter array with a VARCHAR(5) column, and fetches the > results. With the patch, it can't fetch the result sets. Thanks for the test case. The attached patch instead of the previous one would fix the problem but still has a problem when the server side prepare option is turned on. regards, Hiroshi Inoue
Attachment
(2013/03/27 0:02), Hiroshi Inoue wrote: > (2013/03/25 20:44), Heikki Linnakangas wrote: >> On 24.03.2013 12:07, Hiroshi Inoue wrote: >>> I can reproduce it now when I turn off the *Server side prepare* option >>> or turn on the *Use Declare/Fetch* option. >> >> Ah yeah, UseServerSidePrepare hides it. I'm also seeing the problem >> without UseDeclareFetch, though. >> >>> Attached is a patch to fix the bug. >> >> Thanks. I must admit I don't understand how this works. Also, the 2nd >> test case in attached file now fails. It does a DELETE ... RETURNING, >> binding a parameter array with a VARCHAR(5) column, and fetches the >> results. With the patch, it can't fetch the result sets. > > Thanks for the test case. > The attached patch instead of the previous one would fix the problem > but still has a problem when the server side prepare option is turned > on. OK attached is a patch to fix the problem with the *Server side Prepare* option. regards, Hiroshi Inoue
Attachment
On 29.03.2013 15:51, Hiroshi Inoue wrote: > (2013/03/27 0:02), Hiroshi Inoue wrote: >> (2013/03/25 20:44), Heikki Linnakangas wrote: >>> On 24.03.2013 12:07, Hiroshi Inoue wrote: >>>> I can reproduce it now when I turn off the *Server side prepare* option >>>> or turn on the *Use Declare/Fetch* option. >>> >>> Ah yeah, UseServerSidePrepare hides it. I'm also seeing the problem >>> without UseDeclareFetch, though. >>> >>>> Attached is a patch to fix the bug. >>> >>> Thanks. I must admit I don't understand how this works. Also, the 2nd >>> test case in attached file now fails. It does a DELETE ... RETURNING, >>> binding a parameter array with a VARCHAR(5) column, and fetches the >>> results. With the patch, it can't fetch the result sets. >> >> Thanks for the test case. >> The attached patch instead of the previous one would fix the problem >> but still has a problem when the server side prepare option is turned >> on. > > OK attached is a patch to fix the problem with the *Server side > Prepare* option. Many thanks! Although I still don't fully understand the fix, I committed that with minor copy-editing. - Heikki
(2013/04/25 0:42), Heikki Linnakangas wrote: > On 29.03.2013 15:51, Hiroshi Inoue wrote: >> (2013/03/27 0:02), Hiroshi Inoue wrote: >>> (2013/03/25 20:44), Heikki Linnakangas wrote: >>>> On 24.03.2013 12:07, Hiroshi Inoue wrote: >>>>> I can reproduce it now when I turn off the *Server side prepare* >>>>> option >>>>> or turn on the *Use Declare/Fetch* option. >>>> >>>> Ah yeah, UseServerSidePrepare hides it. I'm also seeing the problem >>>> without UseDeclareFetch, though. >>>> >>>>> Attached is a patch to fix the bug. >>>> >>>> Thanks. I must admit I don't understand how this works. Also, the 2nd >>>> test case in attached file now fails. It does a DELETE ... RETURNING, >>>> binding a parameter array with a VARCHAR(5) column, and fetches the >>>> results. With the patch, it can't fetch the result sets. >>> >>> Thanks for the test case. >>> The attached patch instead of the previous one would fix the problem >>> but still has a problem when the server side prepare option is turned >>> on. >> >> OK attached is a patch to fix the problem with the *Server side >> Prepare* option. > > Many thanks! Although I still don't fully understand the fix, I > committed that with minor copy-editing. Thanks. I'm examing the report from Jack Wilson. Probably I can reproduce his case now. regards, Hiroshi Inoue