Thread: 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
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.
Attachment
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.
On 22/08/2012 04:09, Craig Ringer 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.
I was recently surprised to find that PgJDBC doesn't accept Java arrays as parameters to prepared statements.
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