Thread: Fw: PATCH (Re: Startup message issues)
Sorry. Again forgot to copy the list. (I'll get the hang of this eventually...). Here's what I mistakenly sent only to Kris, but meant to send to the list. Chris Smith wrote: > Kris Jurka wrote: > > This would be a good thing to do. I believe the V3 protocol was put > > in with the goal of getting it working which meant a copy, paste, > > and minimal edit. So yes V3 implies 7.4 and it would be nice if it > > took full advantage of what is offered. > > Okay, great! Could someone review the attached patch? It makes the > following changes: > > 1. Remove backward-compatibility for pre-7.4 versions from > the v3 protocol connection code. > 2. Move client_encoding and DateStyle setup to startup message. > 3. Parse ParameterStatus for server_version. > > The result of #2 and #3 is that v3 protocol connections are > established with only one round-trip to the server (in the trust auth > case; two when authentication is required). I've done some ad-hoc > testing... is there a test suite that should be run against this? -- www.designacourse.com The Easiest Way to Train Anyone... Anywhere. Chris Smith - Lead Software Developer/Technical Trainer MindIQ Corporation
Attachment
> > Okay, great! Could someone review the attached patch? It makes the > > following changes: > > > > 1. Remove backward-compatibility for pre-7.4 versions from > > the v3 protocol connection code. > > 2. Move client_encoding and DateStyle setup to startup message. > > 3. Parse ParameterStatus for server_version. > > > > The result of #2 and #3 is that v3 protocol connections are > > established with only one round-trip to the server (in the trust auth > > case; two when authentication is required). I've done some ad-hoc > > testing... is there a test suite that should be run against this? > Patch applied. In the future please submit patches based on cvs instead of the 7.4 code. Driver development is now being done here: http://gborg.postgresql.org/project/pgjdbc/projdisplay.php Kris Jurka
Kris Jurka wrote: > Patch applied. In the future please submit patches based on cvs > instead of the 7.4 code. Driver development is now being done here: > > http://gborg.postgresql.org/project/pgjdbc/projdisplay.php Thanks. I had actually submitted that patch for review, though. I'm not entirely confident that it's correct at this point. All the unit tests pass, but I'm still thinking about when I said the encoding field to UNICODE, and learning a lot more about the source as time goes on. I have just grabbed a version of the driver from CVS. I have a bit of a conflict here because I'm being paid to get stuff working under 7.4.1 for our production application; but if there's not a huge amount of effort involved, I'll try to submit patches for CVS instead. (By the way, where should I send JDBC driver patches in the future that I intend to be committed. I've realized just now that I'm the only person sending them here.) -- www.designacourse.com The Easiest Way to Train Anyone... Anywhere. Chris Smith - Lead Software Developer/Technical Trainer MindIQ Corporation
Chris, You can send patches here Dave On Sat, 2004-02-14 at 09:39, Chris Smith wrote: > Kris Jurka wrote: > > Patch applied. In the future please submit patches based on cvs > > instead of the 7.4 code. Driver development is now being done here: > > > > http://gborg.postgresql.org/project/pgjdbc/projdisplay.php > > Thanks. I had actually submitted that patch for review, though. I'm not > entirely confident that it's correct at this point. All the unit tests pass, > but I'm still thinking about when I said the encoding field to UNICODE, and > learning a lot more about the source as time goes on. > > I have just grabbed a version of the driver from CVS. I have a bit of a > conflict here because I'm being paid to get stuff working under 7.4.1 for our > production application; but if there's not a huge amount of effort involved, > I'll try to submit patches for CVS instead. > > (By the way, where should I send JDBC driver patches in the future that I > intend to be committed. I've realized just now that I'm the only person > sending them here.) -- Dave Cramer 519 939 0336 ICQ # 14675561
On Sat, 14 Feb 2004, Chris Smith wrote: > Kris Jurka wrote: > > Patch applied. In the future please submit patches based on cvs > > instead of the 7.4 code. Driver development is now being done here: > > > > http://gborg.postgresql.org/project/pgjdbc/projdisplay.php > > Thanks. I had actually submitted that patch for review, though. I'm not > entirely confident that it's correct at this point. All the unit tests pass, > but I'm still thinking about when I said the encoding field to UNICODE, and > learning a lot more about the source as time goes on. Well, I didn't just blindly apply it. I read it, did some testing, and actually modified it slightly. You had a comment saying V2 when you meant V3 protocol. In StartupPacket I put the user and database variables into the params Hashtable. (I'm not convinced this is much better, but I was playing with this idea and thats how things ended up when I was ready to commit.) My only concern about the code is that String.getBytes is called in StartupPacket without specifying an encoding. I think this is potentially a problem depending on the user's encoding and if the values for username and database name. The code has been this way since it was originally written, so that's no reason not to commit your patch. > I have just grabbed a version of the driver from CVS. I have a bit of a > conflict here because I'm being paid to get stuff working under 7.4.1 for our > production application; but if there's not a huge amount of effort involved, > I'll try to submit patches for CVS instead. Working for a 7.4.1 database or working for the 7.4.1 jar file? As you have noticed the driver has a bunch of backwards compatibility stuff, so you know the cvs driver will happing run against older versions of the database. > (By the way, where should I send JDBC driver patches in the future that I > intend to be committed. I've realized just now that I'm the only person > sending them here.) > People send them here, just rather infrequently. Keep them coming. Kris Jurka
Kris Jurka wrote: > Working for a 7.4.1 database or working for the 7.4.1 jar file? As > you have noticed the driver has a bunch of backwards compatibility > stuff, so you know the cvs driver will happing run against older > versions of the database. Okay, I'm convinced. Since the changes I'm planning on making are fairly major, I suppose it's senseless to apply normal logic about not using development code for a production application. So I'm now working with CVS and will plan to deploy from there. -- www.designacourse.com The Easiest Way to Train Anyone... Anywhere. Chris Smith - Lead Software Developer/Technical Trainer MindIQ Corporation