Marek Lewczuk wrote:
>
>> 3) When determing if NULL in an array string is a null value, you need
>> to check the server version. Prior to 8.2 an unadorned NULL is the
>> text "null", not an actual null value.
> See line 106 (of the attached AbstractJdbc2Array.java) - I'm checking,
> whether Object[] should be used instead of primitive arrays. It also
> used to check, whether NULL elements can be used. Now, see line 230/231
> - at this point, I'm checking, whether element is a text "NULL" or null
> element - it works just fine.

Perhaps that's OK. If you asked for primitive types on an int[] array
with a NULL value, you could want zero instead of an error trying to
convert the String "NULL" to an int, similar to rs.getInt() on a NULL
value. Let's leave this for now and work on fixing/writing new tests
and #6 and we can revisit this later.

>> 4) Shouldn't toString(PgArrayList) be PgArrayList.toString() ?
> It could be, but see line 598 (of the attached AbstractJdbc2Array.java)
> - I'm using escapeString, that throws SQLException and I cannot declare
> PgArrayList.toString() as throwing SQLException (super class declaration
> doesn't allow for that) - I could of course catch SQLException but it
> think it is better to stay with current implementation.

You're right, it's fine.

>> 6) I was unable to recursively retrieve multidimensional arrays as
>> ResultSets as I thought I should be able to do (in the attached test).
>> Shouldn't you retain an array as the base type for the top level of a
>> multi-dimensional array and expose a dimension at a time via each
>> ResultSet?
> You have made a mistake, please try attached ArrayTest.

Doh! OK, now it mostly works, but there's still an issue with setting
the basetype on a subarray to the base element type instead of an array
type, as attached. rs.getObject() (and metadata) are confused about
what the correct type is.



---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

Kris Jurka pisze:
> Doh! OK, now it mostly works, but there's still an issue with setting
> the basetype on a subarray to the base element type instead of an array
> type, as attached. rs.getObject() (and metadata) are confused about
> what the correct type is.
I see the problem. I assume that we need to add support for array types,
which means that org.postgresql.core.Oid must have oid for every base
type array, e.g. _INT2 = 1005. It will be also required to add
appropriate data within org.postgresql.jdbc2.TypeInfoCache#types. Should
I do it ?

Regards,
Marek Lewczuk





---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

On Fri, 26 Oct 2007, Marek Lewczuk wrote:

> I see the problem. I assume that we need to add support for array types,
> which means that org.postgresql.core.Oid must have oid for every base type
> array, e.g. _INT2 = 1005. It will be also required to add appropriate data
> within org.postgresql.jdbc2.TypeInfoCache#types. Should I do it ?
>

That doesn't sound right to me because we won't be able to put every
possible type (think about user defined) into the Oid class. Perhaps
getResultSet should convert getBaseTypeName() to oid instead of
getBaseType? Then you just need to know if your output is an array or not
(by checking isMultiDimensional) to know whether you want the oid for type
or _type.

Kris Jurka

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

Kris Jurka pisze:
>
>
> That doesn't sound right to me because we won't be able to put every
> possible type (think about user defined) into the Oid class. Perhaps
I was thinking only about base types (they are already in Oid class),
because it will work much faster if they will be statically written in
Oid class.

> getResultSet should convert getBaseTypeName() to oid instead of
> getBaseType? Then you just need to know if your output is an array or
> not (by checking isMultiDimensional) to know whether you want the oid
> for type or _type.
I still think that we should add base type arrays into Oid class - it
will work much faster and will be appropriate cause Oid class already
contains base types, so it is logical to put there base type arrays too.
For user defined types I would provide a way to fetch oid from pg_type -
but for now user defined types are not supported. However, if you really
think that we should fetch oid for every array type then I'm able to do
it but in my opinion we should stick with Oid class for now (only for
base types).

Regards,
ML




---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

On Fri, 26 Oct 2007, Marek Lewczuk wrote:

> I still think that we should add base type arrays into Oid class - it will
> work much faster and will be appropriate cause Oid class already contains
> base types, so it is logical to put there base type arrays too. For user
> defined types I would provide a way to fetch oid from pg_type - but for now
> user defined types are not supported. However, if you really think that we
> should fetch oid for every array type then I'm able to do it but in my
> opinion we should stick with Oid class for now (only for base types).
>

This makes sense to me, static data for known types, dynamic for unknown.

Kris Jurka

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

Kris Jurka pisze:
>
> This makes sense to me, static data for known types, dynamic for unknown.
Great, I will prepare appropriate patches and send them tomorrow.



---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

On Wed, Nov 21, 2007 at 05:49:28PM -0500, Kris Jurka wrote:
>
>
> On Tue, 30 Oct 2007, Marek Lewczuk wrote:
>
> >in the attachment you can find jdbc.patch (that includes patches for a few
> >classes). You will see that I've added base types array types and right
> >now Array implementation works as should (your last test case is working,
> >which means that when using Array.getResultSet() returns appropriate
> >Array.getBaseType() and Array.getBaseTypeName()). However those changes
> >are not enough, there has to be more to be done in order to provide
> >support for user defined types, but I think that should be done later. I
> >also think that class TypeInfoCache must be absolutely rewritten, cause it
> >is not prepared for user defined types.
> >
>
> Attached is a revised version of your patch after I worked on it a bit.
> Things I've changed:
>
> 1) Looking up the array type for a base type by using typelem doesn't work
> because typelem is not unique. Since no code was calling getPGTypeArray,
> I just removed this code.
>
> 2) Fixed driver code that was relying on the array return type in
> AbstractJdbc2DatabaseMetaData.
>
> 3) Fixed the regression tests and added new ones to verify that array
> handling works.
>
> 4) Changed Array.getResultSet to use generic coding rather than hardcoding
> type oids for each java.sql.Types value. This allows getResultSet to be
> used on types that aren't known to the driver, like aclitem[]. You still
> can't call getArray on this type, but we're getting closer.
>
> 5) Array.getResultSet didn't respect index or offset parameters.
>
> 6) For TypeInfoCache I've added a new column to link the base and array
> types instead of duplicating the whole rows.
>
> 7) Code did not correctly handle a string value of NULL in arrays for 8.1
> and earlier server versions.
>
> So with these changes, I think the code is pretty much ready to go, with
> the exception of how multidimensional results are returned. The way you
> wrap the arrays in Object[] makes it impossible to cast to things like
> Integer[][]. Also I think we should be able to return multidimensional
> arrays of primitive type if the "compatible" option is set to 8.2. I
> think you should be using java.lang.reflect.Array#newInstance to create
> the arrays you are returning to get the correct type instead of building
> them up incrementally.
>
> Kris Jurka

Kris/Marek: I've attached a very small followup patch which addresses a
need I had: I was using java.sql.DatabaseMetaData's getColumns method to
attempt to get the COLUMN_SIZE attribute for a column of type "character
varying(255) []". Without my patch (applied after Kris's patches) the
COLUMN_SIZE would be 2147483647, with the patch it would be 255. I am
by no means a JDBC or Postgresql expert, so maybe I am way off base.
What do you think?

Eric.


---------------------------(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

On Thu, 22 Nov 2007, Eric Lenio wrote:

> Kris/Marek: I've attached a very small followup patch which addresses a
> need I had: I was using java.sql.DatabaseMetaData's getColumns method to
> attempt to get the COLUMN_SIZE attribute for a column of type "character
> varying(255) []". Without my patch (applied after Kris's patches) the
> COLUMN_SIZE would be 2147483647, with the patch it would be 255. I am
> by no means a JDBC or Postgresql expert, so maybe I am way off base.
> What do you think?
>

Looks reasonable, but way too specific. We don't want to do that for each
array type. At the top of each of the methods that have the switch
statements we should have something that checks to see if it is an array
type and converts the oid to the oid of the base type.

Kris Jurka


---------------------------(end of broadcast)---------------------------
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate

Marek Lewczuk pisze:
> Kris Jurka pisze:
Argh... sorry for English mistakes. I type to fast and sometimes I don't
pay attention to typos and grammar mistakes. Sorry...

> I was concentrate on make it working for >= 8.2 - again my mistake, sorry.
I was concentrated on doing it for...

> Well, I aware that java.lang.reflect.Array#newInstance can be used
I'm aware...

> I one to discuss one more thing - see below lines in the AbstractJdb2Array:
I want to discuss one...

Best wishes,
Marek



---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

On Tue, 27 Nov 2007, Marek Lewczuk wrote:

> [one more patch for multi-dimensional arrays]
>

Applied with one last minor fix. Array elements are escaped differently
than literals or identifiers, so I've added a new escapeArrayElement
method instead of trying to use connection.escapeString.

> Btw, you didn't answer about ResultSetMetaData.getColumnTypeName() - does it
> mean, that we won't do anything with that in near future ?
>

I don't have any immediate plans to work on this issue. The 8.3 release
is coming fast and there are two more large patches that I need to have
another look at (copy + binary transfer). Schema qualifying all types has
been a problem since the 7.3 release and we haven't heard a whole lot of
complaints about it, so I'm not hugely concerned. That's not to say I
don't think we should fix it, just that it's low on my priority list. If
you want to work on it, I'd be happy to look at whatever you come up with.

Kris Jurka

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org