Thread: Some changes
Hello list,
when I was seeking patch for server side prepare I found two
improvements and add some mylog output.
I send here 3 patch + one contains all of them to review.
1) psqlodbc-skip_cmd_fetch.diff - this skip QR_fetch_tuples when
backend returns PGRES_COMMAND_OK - there is no data to fetch
2) psqlodbc-rowcount.diff - this add support for returning rowcount
from select statements (with UseDeclareFetch disabled - as
non-libpq versions)
3) psqlodbc-mylog.diff - add some mylog output for better tracing
4) psqlodbc-all-skip_cmd_fetch-rowcount-mylog.diff
this one contains all (three) above in one becouse they are created
againist CVS version and they coincidents a little:
$ psqlodbc>patch -p1 < ..\psqlodbc-skip_command_fetch.diff
$ patching file `connection.c'
$ psqlodbc>patch -p1 < ..\psqlodbc-rowcount.diff
$ patching file `connection.c'
$ Hunk #1 succeeded at 1850 (offset 5 lines).
$ psqlodbc>patch -p1 < ..\psqlodbc-mylog.diff
$ patching file `connection.c'
$ Hunk #1 succeeded at 1800 (offset 5 lines).
$ Hunk #3 succeeded at 1887 (offset 13 lines).
$ Hunk #5 succeeded at 1911 (offset 13 lines).
$ Hunk #7 succeeded at 2007 (offset 13 lines).
Please feel free to send feedback here
Luf
Attachment
Hi Luf, Thanks, patches applied, though I have commented out the mylogs inside the inner loop of CC_mapping as they *really* churn out a lot of additional detail. I left them there though so if needed they can just be uncommented. Interestingly, this doesn't seem to fix the VB6 rowcount issue which I still see as -1. Not sure why though - ADO uses SQLRowcount and now gets the correct result, and SQLGetDiagField with SQL_HANDLE_STMT and SQL_DIAG_CURSOR_ROW_COUNT which also appears to return the correct value. Needs some further thought I think.... Regards, Dave > -----Original Message----- > From: pgsql-odbc-owner@postgresql.org > [mailto:pgsql-odbc-owner@postgresql.org] On Behalf Of Ludek Finstrle > Sent: 01 December 2005 20:21 > To: pgsql-odbc@postgresql.org > Subject: [ODBC] Some changes > > Hello list, > > when I was seeking patch for server side prepare I found two > improvements and add some mylog output. > > I send here 3 patch + one contains all of them to review. > > 1) psqlodbc-skip_cmd_fetch.diff - this skip QR_fetch_tuples when > backend returns PGRES_COMMAND_OK - there is no data to fetch > 2) psqlodbc-rowcount.diff - this add support for returning rowcount > from select statements (with UseDeclareFetch disabled - as > non-libpq versions) > 3) psqlodbc-mylog.diff - add some mylog output for better tracing > > 4) psqlodbc-all-skip_cmd_fetch-rowcount-mylog.diff > this one contains all (three) above in one becouse they > are created > againist CVS version and they coincidents a little: > > $ psqlodbc>patch -p1 < ..\psqlodbc-skip_command_fetch.diff > $ patching file `connection.c' > > $ psqlodbc>patch -p1 < ..\psqlodbc-rowcount.diff > $ patching file `connection.c' > $ Hunk #1 succeeded at 1850 (offset 5 lines). > > $ psqlodbc>patch -p1 < ..\psqlodbc-mylog.diff > $ patching file `connection.c' > $ Hunk #1 succeeded at 1800 (offset 5 lines). > $ Hunk #3 succeeded at 1887 (offset 13 lines). > $ Hunk #5 succeeded at 1911 (offset 13 lines). > $ Hunk #7 succeeded at 2007 (offset 13 lines). > > Please feel free to send feedback here > > Luf >
> -----Original Message----- > From: Ludek Finstrle [mailto:luf@pzkagis.cz] > Sent: 02 December 2005 12:29 > To: Dave Page > Subject: Re: [ODBC] Some changes > > I'll investigate it more in afternoon or tomorrow. > It seems to me that after succesful run app calls FreeStmt. > But after SQLCancel it doesn't. Have to we call it in SQLCancel? > Not only FreeStmt with SQL_Close but with SQL_DROP? > It sounds me to be too much brutal. It's the point I don't know > how describe it ODBC spec. > > I'm at work now and I have no access to source code. OK, here's what seems to be happening: 1) The app allocates a statement 2) The app binds 3 parameters using SQLBindParameter. 2 are given values, and the third (the text column) is set to data_at_exec. 3) The app calls SQLExecDirectW, with a statement containing 3 parameters. The driver returns SQL_NEED_DATA (because it has no data for param 3 yet). 4) SQLParamData is called to supply the required data and the query is executed. 5) The server returns SQL_ERROR because of a unique index violation. 6) The app calls SQLCancel. 7) The app calls SQLExecDirectW with a query containing no parameters *on the same statement handle*. The driver returns SQL_NEED_DATA, because it still has a bound parameter set to data_at_exec. 8) Foxpro barfs, not expecting SQL_NEED_DATA. The attached patch clears out any parameters during SQLCancel. This works for the test case, and seems to not hurt any of my apps that I've quickly tested. I definitely want some feedback before I apply this though please :-) Regards, Dave.
Attachment
> The attached patch clears out any parameters during SQLCancel. This > works for the test case, and seems to not hurt any of my apps that I've > quickly tested. I definitely want some feedback before I apply this > though please :-) I see (endly) ODBC specification and it's ok to free all parameters when SQLCancel is called during awaiting data (SQL_NEED_DATA). Only comment in patch is little bit strenge. I have to more study ODBC specification. It's mine bottleneck. Luf
> -----Original Message----- > From: Ludek Finstrle [mailto:luf@pzkagis.cz] > Sent: 02 December 2005 14:38 > To: Dave Page > Cc: pgsql-odbc@postgresql.org > Subject: Re: [ODBC] Some changes > > > The attached patch clears out any parameters during SQLCancel. This > > works for the test case, and seems to not hurt any of my > apps that I've > > quickly tested. I definitely want some feedback before I apply this > > though please :-) > > I see (endly) ODBC specification and it's ok to free all parameters > when SQLCancel is called during awaiting data (SQL_NEED_DATA). > Only comment in patch is little bit strenge. > > I have to more study ODBC specification. It's mine bottleneck. And mine :-(. I'm pretty sure the patch is within the spec though (and frankly, I'm more concerned that the driver works as most popular applications expect than 100% to the spec which is pretty much the line taken by previous maintainers). Anyhoo, please let me know if you think it's good to apply when you've taken a look at it - I'd appreciate the feedback on this one! Regards, Dave
> > Only comment in patch is little bit strenge. I'm sorry. I post only second part of my reviewing the patch. > Anyhoo, please let me know if you think it's good to apply when you've > taken a look at it - I'd appreciate the feedback on this one! It looks good to me except the comment. SQLFreeStmt do the freeing bind params same way as you implement in SQLCancel. This is better way. I try create first development snapshot yesterday. I didn't announce it. Do you agree I add this patch to the snapshot and re-release it with announcement here? So more people can test it. Thanks Luf
> -----Original Message----- > From: Ludek Finstrle [mailto:luf@pzkagis.cz] > Sent: 02 December 2005 17:14 > To: Dave Page > Cc: pgsql-odbc@postgresql.org > Subject: Re: [ODBC] Some changes > > > > Only comment in patch is little bit strenge. > > I'm sorry. I post only second part of my reviewing the patch. > > > Anyhoo, please let me know if you think it's good to apply > when you've > > taken a look at it - I'd appreciate the feedback on this one! > > It looks good to me except the comment. SQLFreeStmt do the freeing > bind params same way as you implement in SQLCancel. This is better > way. If you look above my extra couple of lines, the code goes out of it's way /not/ to use SQLFreeStmt when there are data_at_exec parameters. I'm not entirely sure why, but someone obviously was! As it happens, I did try modifying the code (by removing the data_at_exec check), however it actually didn't clear the params; I continued to see the same crash. > I try create first development snapshot yesterday. I didn't > announce it. > Do you agree I add this patch to the snapshot and re-release it > with announcement here? So more people can test it. By all means, please do. Regards, Dave.
> > It looks good to me except the comment. SQLFreeStmt do the freeing > > bind params same way as you implement in SQLCancel. This is better > > way. > > If you look above my extra couple of lines, the code goes out of it's > way /not/ to use SQLFreeStmt when there are data_at_exec parameters. I'm It's my terrible english and quicker hands then head :-) I think your way is better. > actually didn't clear the params; I continued to see the same crash. Andrus's problem is solved. > By all means, please do. Done. BTW people are so quick. They've downloaded 8.01.0102 (with bad version.h) 189 times. And I didn't announce it. I replaced 8.01.0102 with 8.01.0103 and announced it here. Luf