Re: Let's use libpq for everything - Mailing list pgsql-odbc

On 11/18/2014 09:01 AM, Craig Ringer wrote:
> On 11/04/2014 01:22 AM, Heikki Linnakangas wrote:
>> I went ahead and replaced all the backend-interactions with libpq calls.
>> I've pushed that to a development branch at:
>> git@github.com:hlinnaka/psqlodbc.git, branch "libpq-integration4". It's
>> not 100% complete - a few regression tests are failing - but it mostly
>> works. Not surprisingly, this makes a lot of code unnecessary:
>>
>>   46 files changed, 1357 insertions(+), 7545 deletions(-)
>
> That's what I like to see.

I spent some time adding regression tests, and making it easier to run
them with different combinations of configuration options. I've pushed
those changes to the master branch.

I fixed the remaining regression test failures, and some new ones
revealed by the new tests, and pushed a new version of this patch to the
above github branch. It now passes all regression tests, in all
combinations of UseDeclareFetch, UseServerSidePrepare, and Protocol
(i.e. rollback on error behavior - it doesn't really control the
protocol anymore)

>> 2. Using libpq functions requires more round-trips for some operations,
>> because when speaking the protocol directly, you can "pipeline" some
>> operations, and libpq doesn't expose functions to do that.
>
> That's something that needs addressing in libpq, IMO.

I spent some time analyzing the situation with Wireshark, and managed to
eliminate the worst cases of extra round-trips. It turns out that the
driver doesn't really do much more pipelining than libpq already allows.
So this isn't such a big deal.

To measure the round-trips, I wrote a little hack. I patched the server
to count the number of Sync messages and Query messages it receives;
those are the messages that tell the server to send a reply back, i.e.
the number of round-trips. When a backend exits, it prints out the
counter, together with the application_name, to the log.

I then hacked the driver to use an application_name that includes the
test program's name, and the current UseServerSidePrepare,
UseDeclareFetch, and Protocol settings. Finally, I ran the regression
suite, with all combinations of those, with current git master and this
libpq-integration4 branch, and tabulated the results. See attached.

There is little difference in the number of round-trips. Some test cases
need a few extra round-trips, while others need less. I'm not sure where
all those little differences come from, but it seems acceptable. There
is a quite significant increase in round-trips in the insertreturning
test case, 808 vs 608, but that's not very representative of real-world
usage. In a loop, it prepares a statement with SQLPrepare, and calls
SQLNumResultCols on it, before SQLExecute. It then runs SQLExecute, and
closes the prepared statement, after just one execution. So I think
that's acceptable too.

>> 3. I'm not sure if I got all the less common "modes" like
>> "DisallowPremature=1/0" and "Parse=1/0" to work the same they did
>> before. I think we need to have a discussion on which modes we still
>> need to support, and which are just legacy that we could remove.
>
> I'd like to see a lot of that complexity and variation simply removed.
>
> People who want the old stuff can use the old driver.

Agreed. I'm going to spend a little time testing those modes, and see if
there are issues. If not, or they're easy to fix, we might as well not
touch them right now. Otherwise, we'll need to discuss the fate of those
options.

If anyone has any test cases in mind that are not covered by the current
regression tests, please speak up. I'm relying heavily on the regression
suite, and if there's some major functionality or usage pattern that's
not covered, it's likely to get broken.

Overall, I think this looks *very* good. I'm actually in favor of
committing this fairly soon, in the next week or so, so that the
09.04.0000 version would use libpq-only code. Any objections?

- Heikki