Thread: use of OID.Unknown

Before we cut an 8.1 stable release, I'd like to revisit the use of
unknown for strings at a minimum.

Currently the driver exhibits some non-intuitive behaviour.

1) Not using indexes unless the correct type is bound to the
parameter ( this is intuitive as I write it but many users get
tripped up ). I'm not sure using unknown for strings would help here
but the other option is using unknown for everything and let the
backend decide what type to use. This may be an option if we only do
this when prepareThreshold is 0 since we assume that we won't be
caching plans anyway.

2) We have broken backward compatibility by changing the behaviour of
setString.

At a minimum I'd propose we bind strings to unknown.

For or against ?

Dave

Dave Cramer <pg@fastcrypt.com> writes:
> Before we cut an 8.1 stable release, I'd like to revisit the use of
> unknown for strings at a minimum.

Isn't it *way* too late to be revisiting such questions for 8.1?

I'd be fairly worried about breaking cases that work conveniently
today.  It sounds like something to experiment with during a devel
cycle, not to cram in at the last moment with no field testing.

            regards, tom lane

It's my impression that we don't get near the level of testing that
the server does. Most people only download whatever we tag as stable.
In other words our real testing cycle occurs once a release has been
made.

This is really a one line change in the code, now how it interacts
with the backend is certainly more complex.

Dave

On 8-Nov-05, at 11:17 AM, Tom Lane wrote:

> Dave Cramer <pg@fastcrypt.com> writes:
>> Before we cut an 8.1 stable release, I'd like to revisit the use of
>> unknown for strings at a minimum.
>
> Isn't it *way* too late to be revisiting such questions for 8.1?
>
> I'd be fairly worried about breaking cases that work conveniently
> today.  It sounds like something to experiment with during a devel
> cycle, not to cram in at the last moment with no field testing.
>
>             regards, tom lane
>

Dave Cramer <pg@fastcrypt.com> writes:
> This is really a one line change in the code, now how it interacts
> with the backend is certainly more complex.

Right, that's the tricky part.  I have some recollection that something
close to this was tried awhile back and didn't work very well, which is
why I'm feeling nervous ...

            regards, tom lane

On Tue, 8 Nov 2005, Dave Cramer wrote:

> Before we cut an 8.1 stable release, I'd like to revisit the use of unknown
> for strings at a minimum.

I've already put out a stable 8.1 driver.  Our release plan has always
been to stay in line with the server.

Kris Jurka

The question is still valid. This is a usability issue which will
affect many users who switch from 7.4 to 8.x

I strongly suspect that the server will release a minor revision very
soon, which would allow us the opportunity to make this change.

I think Tom's objection is certainly of more concern.

Dave


On 8-Nov-05, at 12:10 PM, Kris Jurka wrote:

>
>
> On Tue, 8 Nov 2005, Dave Cramer wrote:
>
>> Before we cut an 8.1 stable release, I'd like to revisit the use
>> of unknown for strings at a minimum.
>
> I've already put out a stable 8.1 driver.  Our release plan has
> always been to stay in line with the server.
>
> Kris Jurka
>
> ---------------------------(end of
> broadcast)---------------------------
> TIP 3: Have you checked our extensive FAQ?
>
>               http://www.postgresql.org/docs/faq
>

On Tue, 8 Nov 2005, Dave Cramer wrote:

> The question is still valid. This is a usability issue which will affect
> many users who switch from 7.4 to 8.x
>
> I strongly suspect that the server will release a minor revision very
> soon, which would allow us the opportunity to make this change.
>

I don't think this is a simple switch we should throw in a minor revision
either.  The real source of our problems is that for performance reasons
we never issue a Describe Statement protocol message.

Right now we Parse/Bind/Describe/Execute/Sync never worrying about what
types the
server actually wants because we've already resolved and specified them
from the setXXX methods (except for timestamp recently).  As you note this
does generate some issues, especially with pg types that don't have a Java
equivalent, for example inet.  We've also seen issues where Ken Geis was
assuming that he could get binary results for any type.  He couldn't and
couldn't even know that was going to happen and request binary instead
because the s no Describe issued.

To correctly infer types we should Parse/Describe/Sync then
Bind/Execute/Sync.  This would allow us to use the unknown oid, but still
correctly convert provided Java types to the desired server types and in
general would give us a lot more flexibility.  The drawback is of course
that it'll be slower because we're doing two network trips instead of one.

Finally there are a few cases where the server simply cannot resolve the
provided type and bails out with an error message.  If using the JDBC
setXXX provided types, these work.  Consider "SELECT ? IS NULL" or an
overloaded function that cannot be resolved.

Kris Jurka

Kris Jurka <books@ejurka.com> writes:
> I don't think this is a simple switch we should throw in a minor revision
> either.  The real source of our problems is that for performance reasons
> we never issue a Describe Statement protocol message.

No, I don't think that's the issue; the part that I'm worried about is
this one:

> Finally there are a few cases where the server simply cannot resolve the
> provided type and bails out with an error message.  If using the JDBC
> setXXX provided types, these work.  Consider "SELECT ? IS NULL" or an
> overloaded function that cannot be resolved.

Sending UNKNOWN creates a risk that the server will barf because it
can't guess the right data type.

Now, if you are proposing to send a parameter marked UNKNOWN in exactly
the same cases where the previous driver release would have sent an
undecorated string literal, then I'm mostly OK with it --- the behavior
at least won't be a regression, because an undecorated string literal
is also taken as UNKNOWN to start with.

I remembered what it was that was nagging me about having seen this
before --- didn't we try using UNKNOWN to avoid having to choose between
timestamp with/without time zone, and didn't that idea crash and burn?
It'd be a good idea to go back and look at the details before thinking
of adopting UNKNOWN in a more general context.

            regards, tom lane

On Tue, 8 Nov 2005, Tom Lane wrote:

> I remembered what it was that was nagging me about having seen this
> before --- didn't we try using UNKNOWN to avoid having to choose between
> timestamp with/without time zone, and didn't that idea crash and burn?
> It'd be a good idea to go back and look at the details before thinking
> of adopting UNKNOWN in a more general context.
>

No, actually we went with this solution simply because we had no other
options.  This may work fine for text data, but how could we possibly
implement sending in binary format?

Kris Jurka

[ resending from proper address ]

Kris Jurka <books@ejurka.com> writes:
> On Tue, 8 Nov 2005, Tom Lane wrote:
>> It'd be a good idea to go back and look at the details before thinking
>> of adopting UNKNOWN in a more general context.

> No, actually we went with this solution simply because we had no other
> options.  This may work fine for text data, but how could we possibly
> implement sending in binary format?

That one's simple --- you don't.  Anything you have down as UNKNOWN,
send as text.  You have the option per-parameter in Bind, remember.

            regards, tom lane

> -----Original Message-----
> From: pgsql-jdbc-owner@postgresql.org
> [mailto:pgsql-jdbc-owner@postgresql.org]On Behalf Of Kris Jurka
> Sent: Tuesday, November 08, 2005 12:36 PM
> To: Dave Cramer
> Cc: List; Tom Lane
> Subject: Re: [JDBC] use of OID.Unknown
>

[SNIP]

> As you note this does generate some issues, especially with
> pg types that don't have a Java equivalent, for example inet.

I've wondered about this and the fact that Java does have an InetAddress type that should convert nicely into pg inet.
Hasthere been any discussion about whether a setInetAddress might exist in the implementation at some point, even if we
haveto downcast out of the specification / interface to get it? 

Cheers,

Nicolaus Bauman
Developer: TIR - SPS
* 612.667.2170
* 612.232.7120
This message may contain confidential and/or privileged information. If you are not the addressee or authorized to
receivethis for the addressee, you must not use, copy, disclose, or take any action based on this message or any
informationherein. If you have received this message in error, please advise the sender immediately by reply e-mail and
deletethis message. Thank you for your cooperation.  

On 8-Nov-05, at 1:52 PM, Tom Lane wrote:

> Kris Jurka <books@ejurka.com> writes:
>> I don't think this is a simple switch we should throw in a minor
>> revision
>> either.  The real source of our problems is that for performance
>> reasons
>> we never issue a Describe Statement protocol message.
>
> No, I don't think that's the issue; the part that I'm worried about is
> this one:
>
>> Finally there are a few cases where the server simply cannot
>> resolve the
>> provided type and bails out with an error message.  If using the JDBC
>> setXXX provided types, these work.  Consider "SELECT ? IS NULL" or an
>> overloaded function that cannot be resolved.
>
> Sending UNKNOWN creates a risk that the server will barf because it
> can't guess the right data type.
>
> Now, if you are proposing to send a parameter marked UNKNOWN in
> exactly
> the same cases where the previous driver release would have sent an
> undecorated string literal, then I'm mostly OK with it --- the
> behavior
> at least won't be a regression, because an undecorated string literal
> is also taken as UNKNOWN to start with.

At this point that is what I would be advocating. It appears that
using UNKNOWN for the general
case is fraught with problems. Previously we had advocated using
setString for postgres types which do not have
jdbc counterparts such as inet, or interval. The current driver
breaks this behaviour.
>
> I remembered what it was that was nagging me about having seen this
> before --- didn't we try using UNKNOWN to avoid having to choose
> between
> timestamp with/without time zone, and didn't that idea crash and burn?
> It'd be a good idea to go back and look at the details before thinking
> of adopting UNKNOWN in a more general context.

Agreed.
>
>             regards, tom lane
>
> ---------------------------(end of
> broadcast)---------------------------
> TIP 9: In versions below 8.0, the planner will ignore your desire to
>        choose an index scan if your joining column's datatypes do not
>        match
>

Dave Cramer wrote:

> 2) We have broken backward compatibility by changing the behaviour of
> setString.

My experience with the driver has been that we don't care much about
backwards compatibility of nonstandard behaviour between major revisions
-- if it's postgresql-specific code, the app can just change as the
driver does.

That's why the protocol changes went into the 8.0 driver, but weren't
backported.. If your app depends on driver-specific behaviour and you
don't want to change it, you can keep using the 7.4.x driver.

> At a minimum I'd propose we bind strings to unknown.

If we do this, what is the path to eventually moving away from unknown?
Using unknown does not feel like the right long-term solution to me .. I
don't want to just dig a deeper hole here.

I also agree with Tom and Kris that it seems too late to do this for the
"8.1" driver. Changing it in a later subrelease is the wrong time to do
it too -- then you'd have 7.4.x with one behaviour, 8.0.x and some 8.1.x
with a different behaviour, and then other 8.1.x with yet another.. urgh.

(I am on leave until February, but will be infrequently checking email)

-O

On 9-Nov-05, at 6:30 AM, Oliver Jowett wrote:

> Dave Cramer wrote:
>
>> 2) We have broken backward compatibility by changing the behaviour
>> of  setString.
>
> My experience with the driver has been that we don't care much
> about backwards compatibility of nonstandard behaviour between
> major revisions -- if it's postgresql-specific code, the app can
> just change as the driver does.
>
> That's why the protocol changes went into the 8.0 driver, but
> weren't backported.. If your app depends on driver-specific
> behaviour and you don't want to change it, you can keep using the
> 7.4.x driver.

The issue here is usability, and perception. Someone on the irc
channel was seeing a query run slower through the JDBC driver than
through psql. I figured it had to do with using the wrong setXXX().
It's my opinion that the driver should do the right thing from the
users perspective, not from the specifications perspective if
possible. If users think the driver is slower they will not use it,
and may not use postgres. I think the option of having this behaviour
dependent on prepareThreshold makes sense. If someone really wants to
cache statements then I will assume they know what they are doing.
>
>> At a minimum I'd propose we bind strings to unknown.
>
> If we do this, what is the path to eventually moving away from
> unknown? Using unknown does not feel like the right long-term
> solution to me .. I don't want to just dig a deeper hole here.
Well, we could use the describe statement as Kris pointed out.
>
> I also agree with Tom and Kris that it seems too late to do this
> for the "8.1" driver. Changing it in a later subrelease is the
> wrong time to do it too -- then you'd have 7.4.x with one
> behaviour, 8.0.x and some 8.1.x with a different behaviour, and
> then other 8.1.x with yet another.. urgh.

I don't believe that we can leave the behaviour the way it is, so the
sooner we can undo this the better in my opinion. There are cases
where minor revisions of the server are quite buggy and to be avoided.
>
> (I am on leave until February, but will be infrequently checking
> email)
Sweet, I'm jealous.
>
> -O
>
> ---------------------------(end of
> broadcast)---------------------------
> TIP 9: In versions below 8.0, the planner will ignore your desire to
>       choose an index scan if your joining column's datatypes do not
>       match
>