Обсуждение: setObject(...) with native Java arrays like String[] ?

Hi all

I was recently surprised to find that PgJDBC doesn't accept Java arrays as parameters to prepared statements.

For example:

    PreparedStatement.setObject(1, new String[]{"a"}).

will fail with:

[ERROR] Internal Exception: org.postgresql.util.PSQLException: Can't infer a SQL
type to use for an instance of [Ljava.lang.String;.
Use setObject () with an explicit Types value to specify the type to use.

... which I've verified with the 902 driver in git.

After reading the spec I can't see anywhere the JDBC driver is required to accept native Java arrays, but at least EclipseLink seems to want to do it anyway. Is this reasonable to support? There's one par in the JDBC spec that suggests it might be supposed to work, but I'm not convinced it's more than bad phrasing. In §16.5.4 the JDBC4.2 draft spec reads:

    A Java array may be passed as an input parameter by calling the method
    PreparedSatement.setObject.


Thoughts? Opinions? Any idea what other DBs drivers do when they get passed a raw Java array to setObject() ?

Source:

http://stackoverflow.com/questions/12042181/how-can-i-set-a-string-parameter-to-a-native-query/12065537#12065537

--
Craig Ringer

Here's a proposed patch to the tests to make the current behaviour explicit:




Add tests for String[] arrays

Shows that Connection.createArrayOf works, and that passing a raw
Java String[] to PreparedStatement.setObject() isn't accepted.
---
  org/postgresql/test/jdbc2/ArrayTest.java | 31
+++++++++++++++++++++++++++++++
  1 file changed, 31 insertions(+)

diff --git a/org/postgresql/test/jdbc2/ArrayTest.java
b/org/postgresql/test/jdbc2/ArrayTest.java
index 16b0823..7b796be 100644
--- a/org/postgresql/test/jdbc2/ArrayTest.java
+++ b/org/postgresql/test/jdbc2/ArrayTest.java
@@ -186,6 +186,37 @@ public class ArrayTest extends TestCase
          assertEquals(3, resultCount);
      }

+    public void testSetObjectFromJavaArray() throws SQLException {
+        String[] strArray = new String[]{"a","b","c"};
+
+        PreparedStatement pstmt = conn.prepareStatement("INSERT INTO
arrtest(strarr) VALUES (?)");
+
+        // Incorrect, but commonly attempted by many ORMs:
+        try {
+            pstmt.setObject(1, strArray, Types.ARRAY);
+            pstmt.executeUpdate();
+            fail("setObject() with a Java array parameter and
Types.ARRAY shouldn't succeed");
+        } catch (org.postgresql.util.PSQLException ex) {
+            // Expected failure.
+        }
+
+        // Also incorrect, but commonly attempted by many ORMs:
+        try {
+            pstmt.setObject(1, strArray);
+            pstmt.executeUpdate();
+            fail("setObject() with a Java array parameter and no Types
argument shouldn't succeed");
+        } catch (org.postgresql.util.PSQLException ex) {
+            // Expected failure.
+        }
+
+        // Correct way, though the use of "text" as a type is non-portable.
+        Array sqlArray = conn.createArrayOf("text", strArray);
+        pstmt.setArray(1, sqlArray);
+        pstmt.executeUpdate();
+
+        pstmt.close();
+    }
+
      /**
       * Starting with 8.0 non-standard (beginning index isn't 1) bounds
       * the dimensions are returned in the data.  The following should
--
1.7.11.2

Sorry, mail client mangled the line wrapping. Attached patch that makes
the current behaviour explicit.

Also, maybe something this should go in the README?



---------------------------------------------------------------------------

TESTING

If you want to run the unit tests, create a user with username "test"
and password "password" in a Pg instance that listens on 127.0.0.1:5432
(the default). Create a database "test" owned by user "test".

     psql -U postgres
     postgres=# CREATE USER test WITH PASSWORD 'test';
     postgres=# CREATE DATABASE test OWNER test;

Now run "ant test" to execute the tests against your new DB. You should
drop the test user and DB after testing as the well-known password could
lead to a security issue.

---------------------------------------------------------------------------

On Tue, Aug 21, 2012 at 8:34 PM, Craig Ringer <ringerc@ringerc.id.au> wrote:
> Also, maybe something this should go in the README?

org/postgresql/test actually has its own testing-specific README,
although perhaps there should be a note of it somewhere more obvious.

I was recently surprised to find that PgJDBC doesn't accept Java arrays as parameters to prepared statements.

As was I, I posted a patch a couple of months ago that did the same thing but also accepted java List types as well (I'm often using these especially with JPA and it would be nice to pass them into the driver), although it seems to have been silently ignored. Maybe I'll repost it.

John

> On 22/08/2012 04:09, Craig Ringer wrote:
>>
>> I was recently surprised to find that PgJDBC doesn't accept Java
>> arrays as parameters to prepared statements.

I'm not surprised at this behavior since I needed to support all data
types with the MyJSQLView application for PostgreSQL. The app uses
prepare statements for inserts/updates and I found out very early
that all data types needed to be explicitly stated for prepare statements.
This is not the JDBC from my understanding, but rather the PostgreSQL
server. You can get away with this in other databases, but not so for
PostgreSQL.

One way around this I found is send the data with the initial prepare
statement creation. This of course precludes recursive use then of the
statement. Kinda defeats the purpose, but for singluar inserts/update
works fine.

It looks like from your code the arrays you speak of are not java.sql.Array?


 > John Lister wrote:
> As was I, I posted a patch a couple of months ago that did the same
> thing but also accepted java List types as well (I'm often using these
> especially with JPA and it would be nice to pass them into the driver),
> although it seems to have been silently ignored. Maybe I'll repost it.

John, the Silence I think is not purposeful. With the port to GIT does
this not allow contributors to more easily process patches independently?
I'm not that familiar with GIT so I apologize up front for my ignorance.

danap.

On 08/22/2012 11:18 PM, dmp wrote:
> One way around this I found is send the data with the initial prepare
> statement creation.

... which is OK if you know what SQL injection is and you're really
careful, but not something that any user should ever be advised to do or
need to do.

> It looks like from your code the arrays you speak of are not
> java.sql.Array?

Correct, I'm asking whether PgJDBC should also accept native Java arrays
like String[] and convert them to java.sql.Array with appropriate type
inference its self.

It's correct to use java.sql.Array and Connection.createArrayFrom(...)
but it's evident that some in-the-wild code, including EclipseLink,
doesn't do that and expects the JDBC driver to accept raw arrays.

> John, the Silence I think is not purposeful.

I don't remember seeing the patch, actually.

http://www.postgresql.org/search/?m=1&q=John+Lister&l=19&d=365&s=d

The Pg mailing list manager is irritatingly good at eating messages with
attached patches, so it's possible nobody ever saw it.

> With the port to GIT does
> this not allow contributors to more easily process patches independently?
> I'm not that familiar with GIT so I apologize up front for my ignorance.

Pg is using a traditional patch-based workflow not a push-pull workflow,
so even if you do your work in public git branches you need to send
patches to get things merged, you can't just post a git ref or use a
GitHub pull request.

That doesn't mean it's useless to keep your work in git. I keep the few
patches I write for Pg in feature branches on my public GitHub account:

https://github.com/ringerc/pgjdbc
https://github.com/ringerc/postgres

primarily so I don't lose it, and I refer to the feature branch when I
post a patch, eg:

https://github.com/ringerc/postgres/tree/sequence_documentation_fixes

However, there'll never be a merge from my branch in the history, either
via a git merge commit or a fast-forward pull, because the changes enter
Pg via patches.

Of course, even if John did have the work in a git repo, the next
question would be "where and in which branch" ? You still need a way to
find the work. Is it on GitHub somewhere? In a branch of a user repo on
git.postgresql.org ? On some personal server that allows public
read-only git access? Without a URL git isn't any more help than a patch.

--
Craig Ringer

On 23/08/2012 02:32, Craig Ringer wrote:
> I don't remember seeing the patch, actually.
>
> http://www.postgresql.org/search/?m=1&q=John+Lister&l=19&d=365&s=d
>
> The Pg mailing list manager is irritatingly good at eating messages
> with attached patches, so it's possible nobody ever saw it.
Odd, I remember getting the copy from the list, but not to worry, I've
added a couple of tests and waiting while they run, will then repost it.

John

To be candid, I need help managing the patches.

Many times the patches come in without test cases or without
documentation. All of these things require me to spend time I don't
have.

Also many patches or requests are very specific "itches" which solve a
particular problem for the submitter. Every new feature adds
complexity and possibly negatively effects performance.  It is useful
to keep in mind that the driver is used by lots of other people who
will not benefit from this particular feature request.


So I am open to suggestions as to how to manage this better. I know
that the main postgresql project has a system where other people
review patches. This seems like it is a good idea.

Notionally someone other than the submitter would review the patch.

Thoughts ?

Dave Cramer

dave.cramer(at)credativ(dot)ca
http://www.credativ.ca


On Thu, Aug 23, 2012 at 3:47 AM, John Lister <john.lister@kickstone.com> wrote:
> On 23/08/2012 02:32, Craig Ringer wrote:
>>
>> I don't remember seeing the patch, actually.
>>
>> http://www.postgresql.org/search/?m=1&q=John+Lister&l=19&d=365&s=d
>>
>> The Pg mailing list manager is irritatingly good at eating messages with
>> attached patches, so it's possible nobody ever saw it.
>
> Odd, I remember getting the copy from the list, but not to worry, I've added
> a couple of tests and waiting while they run, will then repost it.
>
> John
>
>
>
> --
> Sent via pgsql-jdbc mailing list (pgsql-jdbc@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-jdbc

On 23/08/2012 16:00, Dave Cramer wrote:
> To be candid, I need help managing the patches.
>
> Many times the patches come in without test cases or without
> documentation. All of these things require me to spend time I don't
> have.
>
> Also many patches or requests are very specific "itches" which solve a
> particular problem for the submitter. Every new feature adds
> complexity and possibly negatively effects performance.  It is useful
> to keep in mind that the driver is used by lots of other people who
> will not benefit from this particular feature request.
>
>
> So I am open to suggestions as to how to manage this better. I know
> that the main postgresql project has a system where other people
> review patches. This seems like it is a good idea.
>
> Notionally someone other than the submitter would review the patch.
>
Sounds like a good idea, I'm happy to review patches if required. Is
there a set of guidelines that need to be followed
in determining if a patch should be accepted?

John

Well my personal guidelines would be as follows

1) Has to come with a test case
2) If it requires docs it needs those as well
3) Don't change the format of the code
4) Performance is important ( not sure exactly what that means )

Lastly how to determine if it is a good idea to accept it? I'd like to
see more discussion from the group, voting would be one way.
However as I said some very large companies use this as I said. Just
because it satisfies your itch doesn't make it a good idea for
everyone else.

Dave Cramer

dave.cramer(at)credativ(dot)ca
http://www.credativ.ca


On Thu, Aug 23, 2012 at 2:09 PM, John Lister <john.lister@kickstone.com> wrote:
> On 23/08/2012 16:00, Dave Cramer wrote:
>>
>> To be candid, I need help managing the patches.
>>
>> Many times the patches come in without test cases or without
>> documentation. All of these things require me to spend time I don't
>> have.
>>
>> Also many patches or requests are very specific "itches" which solve a
>> particular problem for the submitter. Every new feature adds
>> complexity and possibly negatively effects performance.  It is useful
>> to keep in mind that the driver is used by lots of other people who
>> will not benefit from this particular feature request.
>>
>>
>> So I am open to suggestions as to how to manage this better. I know
>> that the main postgresql project has a system where other people
>> review patches. This seems like it is a good idea.
>>
>> Notionally someone other than the submitter would review the patch.
>>
> Sounds like a good idea, I'm happy to review patches if required. Is there a
> set of guidelines that need to be followed
> in determining if a patch should be accepted?
>
> John

On 08/23/2012 11:00 PM, Dave Cramer wrote:
> To be candid, I need help managing the patches.
>
> Many times the patches come in without test cases or without
> documentation. All of these things require me to spend time I don't
> have.

[snip]

> Notionally someone other than the submitter would review the patch.
>
> Thoughts ?

I've done enough Java and JDBC work now that I can do a half-decent job
with PgJDBC patch review. I'm happy to help.

It'd make sense to be able to manage patch review so it's easy to keep
track of. I see two options for that:

- Use the commitfest app, which fits the main server workflow but
requires wrangling message-IDs and manual updates of the tracking
that're sure to be forgotten; or

- Use GitHub pull requests, which are much nicer to work with and more
familiar to more people, but inconsistent with the main server's workflow.


I don't see any sign that PgJDBC development has used the commitfest app
recently, and there's no "PgJDBC" topic, only "Clients".

Do you have a preference? Given the chance I'd *love* to work via GitHub.

--
Craig Ringer

On 08/24/2012 02:16 AM, Dave Cramer wrote:
> Well my personal guidelines would be as follows
>
> 1) Has to come with a test case
> 2) If it requires docs it needs those as well
> 3) Don't change the format of the code
> 4) Performance is important ( not sure exactly what that means )
>
> Lastly how to determine if it is a good idea to accept it? I'd like to
> see more discussion from the group, voting would be one way.
> However as I said some very large companies use this as I said. Just
> because it satisfies your itch doesn't make it a good idea for
> everyone else.

I'd want to add:

- Doesn't break major existing users. A test suite that uses JPA and
Hibernate and/or EclipseLink would make sense, and if I'm lucky I'll
have time to work on that sometime in the next 100 years or so...

- Improves or preserves the existing level of JDBC compliance. If it's
nice but non-compliant, it IMO isn't acceptable. Same policy as the main
server re SQL standards, really.

- Fits within the existing JDBC interfaces and specs where possible;
extensions should have to jump a higher bar. If it is an extension, it
should mirror extensions from other drivers/vendors if possible.

- Builds with Java 1.5 (ugh, but it'll be time for 1.6 soon).

--
Craig Ringer