Обсуждение: Here's a fix to AbstractJdbc3Statement.getGeneratedKeys
Frustrations over a 4 hour database script with MySQL forced me into this :-) Porting a project from MySQL to PostgreSQL, I discovered that the jdbc3 facility of returning the generated keys from an insert statement does not work. On a fresh cvs checkout, there is partial support, however, all fields are returned, not only the field that are autogenerated. Here is a patch that fixes this. I hope someone will take a look, and consider whether it can be adopted into the project. Or improved. It works for me :-) The strategy of the patch is simply to inspect the metadata of the returned (initially full) resultset, and then strip out any fields that are not marked as autoincrement. BTW the 4 hour db script finishes after 2m 19s on PostgreSQL. Best Regards, Jeppe
Sorry, I forgot to attach the patch :-(
Here it is.
Jeppe Sommer skrev:
> Frustrations over a 4 hour database script with MySQL forced me into
> this :-)
>
> Porting a project from MySQL to PostgreSQL, I discovered that the jdbc3
> facility of returning the generated keys from an insert statement does
> not work. On a fresh cvs checkout, there is partial support, however,
> all fields are returned, not only the field that are autogenerated.
>
> Here is a patch that fixes this. I hope someone will take a look, and
> consider whether it can be adopted into the project. Or improved. It
> works for me :-)
>
> The strategy of the patch is simply to inspect the metadata of the
> returned (initially full) resultset, and then strip out any fields that
> are not marked as autoincrement.
>
> BTW the 4 hour db script finishes after 2m 19s on PostgreSQL.
>
>
> Best Regards,
> Jeppe
>
>
>
Index: org/postgresql/jdbc2/AbstractJdbc2ResultSet.java
===================================================================
RCS file: /cvsroot/jdbc/pgjdbc/org/postgresql/jdbc2/AbstractJdbc2ResultSet.java,v
retrieving revision 1.105
diff -u -r1.105 AbstractJdbc2ResultSet.java
--- org/postgresql/jdbc2/AbstractJdbc2ResultSet.java 30 Sep 2008 04:34:51 -0000 1.105
+++ org/postgresql/jdbc2/AbstractJdbc2ResultSet.java 24 Apr 2009 07:54:23 -0000
@@ -55,7 +55,7 @@
private int fetchdirection = ResultSet.FETCH_UNKNOWN;
protected final BaseConnection connection; // the connection we belong to
protected final BaseStatement statement; // the statement we belong to
- protected final Field fields[]; // Field metadata for this resultset.
+ protected Field fields[]; // Field metadata for this resultset.
protected final Query originalQuery; // Query we originated from
protected final int maxRows; // Maximum rows in this resultset (might be 0).
@@ -2909,6 +2909,18 @@
public String getValue() {
return null;
}
- };
+ }
+
+ public Vector getRows() {
+ return rows;
+ }
+
+ public Field[] getFields() {
+ return fields;
+ }
+
+ public void setFields(Field[] fields) {
+ this.fields = fields;
+ };
}
Index: org/postgresql/jdbc3/AbstractJdbc3Statement.java
===================================================================
RCS file: /cvsroot/jdbc/pgjdbc/org/postgresql/jdbc3/AbstractJdbc3Statement.java,v
retrieving revision 1.24
diff -u -r1.24 AbstractJdbc3Statement.java
--- org/postgresql/jdbc3/AbstractJdbc3Statement.java 28 Jan 2009 09:50:21 -0000 1.24
+++ org/postgresql/jdbc3/AbstractJdbc3Statement.java 24 Apr 2009 07:54:23 -0000
@@ -11,16 +11,19 @@
import java.math.BigDecimal;
import java.sql.*;
+import java.util.ArrayList;
import java.util.Calendar;
+import java.util.HashSet;
import java.util.Vector;
-import org.postgresql.util.PSQLException;
-import org.postgresql.util.PSQLState;
-import org.postgresql.core.Utils;
-import org.postgresql.core.QueryExecutor;
-import org.postgresql.core.Field;
import org.postgresql.core.BaseConnection;
+import org.postgresql.core.Field;
+import org.postgresql.core.QueryExecutor;
+import org.postgresql.core.Utils;
+import org.postgresql.jdbc2.AbstractJdbc2ResultSet;
import org.postgresql.util.GT;
+import org.postgresql.util.PSQLException;
+import org.postgresql.util.PSQLState;
/**
* This class defines methods of the jdbc3 specification. This class extends
@@ -30,6 +33,7 @@
public abstract class AbstractJdbc3Statement extends org.postgresql.jdbc2.AbstractJdbc2Statement
{
private final int rsHoldability;
+ public HashSet<String> specifiedColumns;
public AbstractJdbc3Statement (AbstractJdbc3Connection c, int rsType, int rsConcurrency, int rsHoldability) throws
SQLException
{
@@ -111,7 +115,34 @@
if (generatedKeys == null || generatedKeys.getResultSet() == null)
return createDriverResultSet(new Field[0], new Vector());
- return generatedKeys.getResultSet();
+ AbstractJdbc2ResultSet rs = (AbstractJdbc2ResultSet) generatedKeys.getResultSet();
+
+ ResultSetMetaData metaData = rs.getMetaData();
+
+ final ArrayList<Field> fields = new ArrayList<Field>();
+ ArrayList<byte[]> tuples = new ArrayList<byte[]>();
+ int columnCount = metaData.getColumnCount();
+
+ byte[][] origTuples = (byte[][]) rs.getRows().get(0);
+ Field[] origFields = rs.getFields();
+
+ for (int c = 0; c < columnCount; c++) {
+ if (metaData.isAutoIncrement(c + 1)) {
+ tuples.add(origTuples[c]);
+ fields.add(origFields[c]);
+ }
+ }
+
+ Field[] f = new Field[fields.size()];
+ fields.toArray(f);
+
+ byte[][] t = new byte[tuples.size()][];
+ tuples.toArray(t);
+
+ rs.getRows().set(0, t);
+ rs.setFields(f);
+
+ return rs;
}
/**
On Fri, 24 Apr 2009, Jeppe Sommer wrote: > Porting a project from MySQL to PostgreSQL, I discovered that the jdbc3 > facility of returning the generated keys from an insert statement does not > work. On a fresh cvs checkout, there is partial support, however, all fields > are returned, not only the field that are autogenerated. > The problem is knowing what fields are autogenerated. A field that simply has a default value may in fact be autogenerated. Also, what about a before trigger which sets that value, that's also autogenerated. > Here is a patch that fixes this. I hope someone will take a look, and > consider whether it can be adopted into the project. Or improved. It works > for me :-) > > The strategy of the patch is simply to inspect the metadata of the returned > (initially full) resultset, and then strip out any fields that are not marked > as autoincrement. > I don't see how this helps. If it was able to avoid returning all fields to the client, you could see how it would be a performance improvement, but returning them all and then filtering is just adding more time isn't it? Kris Jurka
Kris Jurka wrote on 24.04.2009 19:20: > The problem is knowing what fields are autogenerated. Isn't that supposed to be passed when preparing the statement? <http://java.sun.com/j2se/1.5.0/docs/api/java/sql/Connection.html#prepareStatement(java.lang.String,%20int[])> Thomas
On Fri, 24 Apr 2009, Thomas Kellerer wrote: > Kris Jurka wrote on 24.04.2009 19:20: >> The problem is knowing what fields are autogenerated. > > Isn't that supposed to be passed when preparing the statement? > > <http://java.sun.com/j2se/1.5.0/docs/api/java/sql/Connection.html#prepareStatement(java.lang.String,%20int[])> > Yes, but not always. If it's provided as above, we use it. If it's not as below, we can't. http://java.sun.com/j2se/1.5.0/docs/api/java/sql/Connection.html#prepareStatement(java.lang.String,%20int) Kris Jurka
Kris Jurka skrev: > On Fri, 24 Apr 2009, Jeppe Sommer wrote: > > >> Porting a project from MySQL to PostgreSQL, I discovered that the jdbc3 >> facility of returning the generated keys from an insert statement does not >> work. On a fresh cvs checkout, there is partial support, however, all fields >> are returned, not only the field that are autogenerated. >> >> > > The problem is knowing what fields are autogenerated. A field that simply > has a default value may in fact be autogenerated. Also, what about a > before trigger which sets that value, that's also autogenerated. > > So what you are saying is that ResultSetMetaData.isAutoIncrement will only return true for a subset of the fields that are actually autogenerated? But perhaps it could serve as a starting point.. The jdbc apidoc is not very precise on what constitutes an autogenerated key, maybe autogenerated=autoincrement is even a valid interpretation? >> Here is a patch that fixes this. I hope someone will take a look, and >> consider whether it can be adopted into the project. Or improved. It works >> for me :-) >> >> The strategy of the patch is simply to inspect the metadata of the returned >> (initially full) resultset, and then strip out any fields that are not marked >> as autoincrement. >> >> > > I don't see how this helps. If it was able to avoid returning all fields > to the client, you could see how it would be a performance improvement, > but returning them all and then filtering is just adding more time isn't > it? > I'm not trying to improve performance, only to get the correct behaviour. I introduced the filtering because the application (actually the Spring framework) failed on receiving all fields when it was expecting only the generated field(s). > Kris Jurka > Jeppe
On Sat, 25 Apr 2009, Jeppe Sommer wrote: > So what you are saying is that ResultSetMetaData.isAutoIncrement will only > return true for a subset of the fields that are actually autogenerated? But > perhaps it could serve as a starting point.. The jdbc apidoc is not very > precise on what constitutes an autogenerated key, maybe > autogenerated=autoincrement is even a valid interpretation? Yeah, it doesn't really say anything about what autogeneratedkeys are. I would take auto-generated to mean, "any field that gets a value that wasn't directly provided." That would include defaults and things set by triggers. I would take key to mean "any field that's part of an index." But putting these together doesn't seem to make a whole lot of sense. Would you not return a value generated by a serial column just because it didn't have an index on it? Given that it's poorly defined and because I wanted to avoid having to parse the provided query, what's been done is to just return everything possible and let the caller deal with it. If the data isn't there, the caller can't do anything about it. If there's too much, the caller can just ignore it. > I'm not trying to improve performance, only to get the correct behaviour. I > introduced the filtering because the application (actually the Spring > framework) failed on receiving all fields when it was expecting only the > generated field(s). > Failed how (a stacktrace for example), perhaps it would be better to fix Spring? Also, I thought of an issue with your patch. It will only work when using the V3 protocol. When connected to a server via V2, we don't get the base table information, ResultSetMetaData.isAutoIncrement will always return false. Kris Jurka
Kris Jurka skrev: > On Sat, 25 Apr 2009, Jeppe Sommer wrote: > > >> So what you are saying is that ResultSetMetaData.isAutoIncrement will only >> return true for a subset of the fields that are actually autogenerated? But >> perhaps it could serve as a starting point.. The jdbc apidoc is not very >> precise on what constitutes an autogenerated key, maybe >> autogenerated=autoincrement is even a valid interpretation? >> > > Yeah, it doesn't really say anything about what autogeneratedkeys are. I > would take auto-generated to mean, "any field that gets a value that > wasn't directly provided." That would include defaults and things set by > triggers. I would take key to mean "any field that's part of an index." > But putting these together doesn't seem to make a whole lot of sense. > Would you not return a value generated by a serial column just because it > didn't have an index on it? Given that it's poorly defined and because I > wanted to avoid having to parse the provided query, what's been done is to > just return everything possible and let the caller deal with it. If the > data isn't there, the caller can't do anything about it. If there's too > much, the caller can just ignore it. > However, this strategy is only helpful with applications/frameworks that can be customized to work with PostgreSQL. You have to build key filtering into the application. > >> I'm not trying to improve performance, only to get the correct behaviour. I >> introduced the filtering because the application (actually the Spring >> framework) failed on receiving all fields when it was expecting only the >> generated field(s). >> >> > > Failed how (a stacktrace for example), perhaps it would be better to fix > Spring? > I'm sorry, I don't have a stacktrace any more. From memory, what failed was GeneratedKeyHolder.getKey(), throwing an exception on receiving more than one key (code can be viewed here: http://www.devdaily.com/java/jwarehouse/spring-framework-2.5.3/src/org/springframework/jdbc/support/GeneratedKeyHolder.java.shtml). Again, if you are writing for PostgreSQL you could use the getKeys method instead of getKey and do the filtering yourself. But for an application/framework that tries to stay portable, thats not a good option. > Also, I thought of an issue with your patch. It will only work when using > the V3 protocol. When connected to a server via V2, we don't get the base > table information, ResultSetMetaData.isAutoIncrement will always return > false. > From my point of view, it seems ok that improvements are only supported in newer versions of PostgreSQL. If someone prefers to have all keys returned when this metadata info is not available, some graceful degradation could be built into the driver. > Kris Jurka > Jeppe