Re: Fix setArray() when using the v3 protocol (was Re: Postgres

Hello,

I have an application running under JBoss.
Up to today, I was using Postgres 7.3 and the appropriate version of the
jdbc driver.
In my application, I have to call a user-defined function which accept
in parameters 2 arrays. Here is the header of my function :
CREATE OR REPLACE FUNCTION getmembers(int8, int8, _text,
_float8)

So I called it using a prepared statement with setArray() :
double[] weights = {0.5};
String[] names = {"foo1", "foo2"};
java.sql.Array a_names = PostgresArray.create(names);
java.sql.Array a_weights = PostgresArray.create(weights);
ps = conn.prepareStatement("SELECT * FROM
getmembers(?,?,?::_text,?::_float8);");
ps.setLong(1, 1);
ps.setLong(2, 2);
ps.setArray(3, a_names);
ps.setArray(4, a_weights);
ps.executeQuery();

PostgresArray is a class which I found on the archives.postgresql.org.
The code is given is attached.

All worked fine.

But today, I decided to upgrade to Postgres 8.0 beta 3.

No problem with the definition of my function.
I downloaded the appropriate JDBC driver : pgdev.306.jdbc3.jar.

Now running the same code as before, I get the error while executing the
query :
java.sql.SQLException: ERROR: cannot cast type text to
text[]

So, what am I doing wrong?
Is it a beta bug or is my code incorrect?
What is the correct way to use SetArray()?

Thanks

JR

Johann Robette wrote:

> java.sql.SQLException: ERROR: cannot cast type text to
> text[]

setArray has never worked particularly well; you're seeing another
manifestation here.

The underlying problem here is that:

- setArray() is claiming that the type it is setting is a VARCHAR
- the parameter is being passed as a typed parameter via the V3
protocol, with type = 'text'
- apparently you can't cast directly from text -> text[] (although you
can interpret an untyped literal as text[], which is why it worked before).

It seems possible to fix the driver to handle this case by making
setArray() derive a proper array type name i.e. ("_" +
Array.getBaseType()), and using that rather than 'text' as the parameter
type.

I'll try to do this soon, but I'm a bit busy so it may be a few days.

A longer term fix is to properly implement array support in setArray()..
I've submitted patches in the past to do this but they've never made it
into the official driver.

-O

On Wed, 6 Oct 2004, Oliver Jowett wrote:

> It seems possible to fix the driver to handle this case by making
> setArray() derive a proper array type name i.e. ("_" +
> Array.getBaseType()), and using that rather than 'text' as the parameter
> type.
>

Wouldn't it be simpler to change setArray to call setString with 0 as the
type oid allowing the backend to figure out what to do with it? Perhaps
it would have trouble determining the type if the underlying function was
overloaded, but other than that I don't see a problem.

Kris Jurka

Kris Jurka wrote:
>
> On Wed, 6 Oct 2004, Oliver Jowett wrote:
>
>
>>It seems possible to fix the driver to handle this case by making
>>setArray() derive a proper array type name i.e. ("_" +
>>Array.getBaseType()), and using that rather than 'text' as the parameter
>>type.
>>
>
>
> Wouldn't it be simpler to change setArray to call setString with 0 as the
> type oid allowing the backend to figure out what to do with it? Perhaps
> it would have trouble determining the type if the underlying function was
> overloaded, but other than that I don't see a problem.

Using the unknown oid seems like it'd introduce more problems:
overloading, and the possibility you actually pass your array as some
other type unexpectedly.

I am wondering if the right way to find the array OID is to prepend "_"
and search on pg_type.typname, or to look for pg_type.typinput = (oid of
array_in) and pg_type.typelem = (oid of underlying type). Is there a
'standard' way of finding an array type OID?

-O

Oliver Jowett <oliver(at)opencloud(dot)com> writes:
> I am wondering if the right way to find the array OID is to prepend "_"
> and search on pg_type.typname, or to look for pg_type.typinput = (oid of
> array_in) and pg_type.typelem = (oid of underlying type). Is there a
> 'standard' way of finding an array type OID?

The backend does it by name, ie prepend '_' and lookup by typname (and
typnamespace). This is a mite unclean but it hasn't seemed worth
fixing. If you like you can use the typelem as an additional check that
you found the right thing.

regards, tom lane

OK Thanks a lot for your answer.
But as I'm a newbie in this field, could you give me an example of how
to do that.

I've already checked the array typename and it is "_text".

You said :
"It seems possible to fix the driver to handle this case by making
setArray() derive a proper array type name i.e. ("_" +
Array.getBaseType()), and using that rather than 'text' as the parameter

type."

How to do that?

Thanks

JR

-----Message d'origine-----
De : Oliver Jowett [mailto:oliver(at)opencloud(dot)com]
Envoyé : mardi 5 octobre 2004 22:11
À : Johann Robette
Cc : pgsql-jdbc(at)postgresql(dot)org
Objet : Re: [JDBC] Postgres 8.0 + JDBC

Johann Robette wrote:

> java.sql.SQLException: ERROR: cannot cast type text to

> text[]

setArray has never worked particularly well; you're seeing another
manifestation here.

The underlying problem here is that:

- setArray() is claiming that the type it is setting is a VARCHAR
- the parameter is being passed as a typed parameter via the V3
protocol, with type = 'text'
- apparently you can't cast directly from text -> text[] (although you
can interpret an untyped literal as text[], which is why it worked
before).

It seems possible to fix the driver to handle this case by making
setArray() derive a proper array type name i.e. ("_" +
Array.getBaseType()), and using that rather than 'text' as the parameter

type.

I'll try to do this soon, but I'm a bit busy so it may be a few days.

A longer term fix is to properly implement array support in setArray()..

I've submitted patches in the past to do this but they've never made it
into the official driver.

-O

On Wed, 6 Oct 2004, Johann Robette wrote:

> But as I'm a newbie in this field, could you give me an example of how
> to do that.

It's not something you can do in your code. It will require changes to
the driver itself. Oliver or I will do this at some point, but I couldn't
say when. If you're desperate to get a fix immediately I've attached a
patch implementing the inferior solution I suggested that made a simple
test work for me.

Kris Jurka

Oliver Jowett wrote:

> It seems possible to fix the driver to handle this case by making
> setArray() derive a proper array type name i.e. ("_" +
> Array.getBaseType()), and using that rather than 'text' as the parameter
> type.

I've done this (plus tests) and it seems to work OK. I can't generate a
patch right now as gborg seems to be having some problems (I get 70%
packet loss on the last hop, which pretty much hoses CVS access)

-O

Oliver Jowett wrote:
> Oliver Jowett wrote:
>
>> It seems possible to fix the driver to handle this case by making
>> setArray() derive a proper array type name i.e. ("_" +
>> Array.getBaseType()), and using that rather than 'text' as the
>> parameter type.
>
>
> I've done this (plus tests) and it seems to work OK. I can't generate a
> patch right now as gborg seems to be having some problems (I get 70%
> packet loss on the last hop, which pretty much hoses CVS access)

Here is the patch.

I made it fail if the array type can't be found, rather than risking the
backend interpreting the value in an unpredictable way.

-O

Oliver Jowett <oliver(at)opencloud(dot)com> writes:
> ! // Use a typename that is "_" plus the base type; this matches how the
> ! // backend looks for array types.
> ! String typename = "_" + x.getBaseTypeName();
> ! int oid = connection.getPGType(typename);
> ! if (oid == Oid.INVALID)
> ! throw new PSQLException("postgresql.prep.typenotfound", PSQLState.INVALID_PARAMETER_TYPE, typename);

Er, what about schema-qualified type names? Also, are you sure that
getBaseTypeName will return the correct internal type name, eg "int8"
not "bigint"?

regards, tom lane

Tom Lane wrote:
> Oliver Jowett <oliver(at)opencloud(dot)com> writes:
>
>>! // Use a typename that is "_" plus the base type; this matches how the
>>! // backend looks for array types.
>>! String typename = "_" + x.getBaseTypeName();
>>! int oid = connection.getPGType(typename);
>>! if (oid == Oid.INVALID)
>>! throw new PSQLException("postgresql.prep.typenotfound", PSQLState.INVALID_PARAMETER_TYPE, typename);
>
>
> Er, what about schema-qualified type names?

We don't really support those anywhere else in the driver yet.
getPGType() certainly doesn't know how to handle them.

> Also, are you sure that
> getBaseTypeName will return the correct internal type name, eg "int8"
> not "bigint"?

It'll be correct for Array implementations generated by the driver. I
didn't see an easy way to get at the list of type aliases -- they appear
to only exist in the backend's query parser. A type alias view or
something similar would be nice to have..

If you're using your own Array implementation, you're playing with fire
anyway, as the current setArray() implementation is limited and fragile.
This patch is really just to get things back to the same level of
support as before the V3 protocol rewrite, not to fix all the other
problems.

-O

On Fri, 8 Oct 2004, Oliver Jowett wrote:

> Oliver Jowett wrote:
> > Oliver Jowett wrote:
> >
> >> It seems possible to fix the driver to handle this case by making
> >> setArray() derive a proper array type name i.e. ("_" +
> >> Array.getBaseType()), and using that rather than 'text' as the
> >> parameter type.
> >
> >

I've applied this. It's not a great solution, but it's low impact and
does fix the problem. An idea I had when looking at it is that if the
lookup by getBaseTypeName fails we could convert getBaseType to a pg type
name by the first match in the connection's jdbcXTypes and retry looking
for an array type with that. Although considering our existing dependency
on Array.toString() returning a properly formatted array this seems like a
waste of effort.

Kris Jurka

On Wed, 6 Oct 2004, Kris Jurka wrote:

> On Wed, 6 Oct 2004, Johann Robette wrote:
>
> > But as I'm a newbie in this field, could you give me an example of how
> > to do that.
>
> It's not something you can do in your code. It will require changes to
> the driver itself. Oliver or I will do this at some point, but I couldn't
> say when. If you're desperate to get a fix immediately I've attached a
> patch implementing the inferior solution I suggested that made a simple
> test work for me.
>

Just wanted to follow up with you here in case you weren't carefully
following the mailing list. Oliver has provided a patch to make this
happen and it is now in the cvs repository.

It does require some changes to the PostgresArray class you sent. Notably
the datatype name needs to match up exactly with the pg data type (aliases
aren't allowed). So for example the create(long[] array) method currently
creates does this at the end:

return new PostgresArray(sb.toString(), Types.BIT, "bigint");

this now needs to use "int8" as the type name. The Types.BIT is also
bogus, but that currently isn't a problem.

Kris Jurka