Thread: jdbc bug/feature?
Hello,
There is a bug (I say) or a lack of proper functionality in JDBC driver regarding batch updates.
The following code is expected to work by some library that I use and it does work with inet tds driver for M$ SQL server. It however does not work with the latest jdbc from cvs or anything older than that either.
PreparedStatements st = c.prepareStatement("insert into my_table (field1, field2) values (?, ?)");
// we have say 30 inserts to do
Iterator it = inserts.iterator();
int i=0;
while(it.hasNext()) {
// there is a maximum batch size of 15, so we must periodically execute it
if(i==batchMax) {
st.executeBatch();
i=0;
// the problem - after this call a PreparedStatement is empty and consequtive binds fail
}
Object [] binds = (Object [])it.next();
st.setString(binds[0]);
st.setString(binds[1]);
st.addBatch();
i++;
}
Upon some research I discovered the problem and found a very simple solution:
In org.postgresql.jdbc2.AbstractJdbc2Statement:
public int[] executeBatch() throws SQLException
{
System.out.println("### executeBatch");
if (batch == null)
batch = new Vector();
int size = batch.size();
int[] result = new int[size];
int i = 0;
// >>> added
// save m_binds and m_sqlFragments because executeUpdate(String) will destroy them
String [] oldFrags = m_sqlFragments;
Object [] oldMBinds = m_binds;
// <<
try
{
for (i = 0;i < size;i++)
result[i] = this.executeUpdate((String)batch.elementAt(i));
}
catch (SQLException e)
{
int[] resultSucceeded = new int[i];
System.arraycopy(result, 0, resultSucceeded, 0, i);
PBatchUpdateException updex =
new PBatchUpdateException("postgresql.stat.batch.error",
new Integer(i), batch.elementAt(i), resultSucceeded);
updex.setNextException(e);
throw updex;
}
finally
{
batch.removeAllElements();
// >> added
// restore m_binds and m_sqlFragments
m_sqlFragments = oldFrags;
m_binds = oldMBinds;
// <<
}
return result;
}
Please consider this as a bug, and patch it in cvs. I did not test this very well, so the described patch may possibly cause some incosistencies somewhere - the people who wrote that class will know best. It does work for my case though.
And thanks for PostgreSQL, I've just started using it and I love it.
- Marko
Marko,
I checked in a fix for this problem. It should appear in 7.3RC2 this
weekend.
thanks,
--Barry
Marko Štrukelj wrote:
>
> Hello,
>
> There is a bug (I say) or a lack of proper functionality in JDBC driver
> regarding batch updates.
>
> The following code is expected to work by some library that I use and it
> does work with inet tds driver for M$ SQL server. It however does not
> work with the latest jdbc from cvs or anything older than that either.
>
>
> PreparedStatements st = c.prepareStatement("insert into my_table
> (field1, field2) values (?, ?)");
>
> // we have say 30 inserts to do
> Iterator it = inserts.iterator();
> int i=0;
> while(it.hasNext()) {
>
> // there is a maximum batch size of 15, so we must periodically
> execute it
> if(i==batchMax) {
> st.executeBatch();
> i=0;
>
> // the problem - after this call a PreparedStatement is
> empty and consequtive binds fail
> }
>
> Object [] binds = (Object [])it.next();
> st.setString(binds[0]);
> st.setString(binds[1]);
> st.addBatch();
> i++;
> }
>
>
>
> Upon some research I discovered the problem and found a very simple
> solution:
>
> In org.postgresql.jdbc2.AbstractJdbc2Statement:
>
> public int[] executeBatch() throws SQLException
> {
> System.out.println("### executeBatch");
> if (batch == null)
> batch = new Vector();
> int size = batch.size();
> int[] result = new int[size];
> int i = 0;
>
> // >>> added
> // save m_binds and m_sqlFragments because
> executeUpdate(String) will destroy them
> String [] oldFrags = m_sqlFragments;
> Object [] oldMBinds = m_binds;
> // <<
>
> try
> {
> for (i = 0;i < size;i++)
> result[i] =
> this.executeUpdate((String)batch.elementAt(i));
>
> }
> catch (SQLException e)
> {
> int[] resultSucceeded = new int[i];
> System.arraycopy(result, 0, resultSucceeded, 0, i);
>
> PBatchUpdateException updex =
> new
> PBatchUpdateException("postgresql.stat.batch.error",
>
> new Integer(i), batch.elementAt(i), resultSucceeded);
>
> updex.setNextException(e);
>
> throw updex;
> }
> finally
> {
> batch.removeAllElements();
> // >> added
> // restore m_binds and m_sqlFragments
> m_sqlFragments = oldFrags;
> m_binds = oldMBinds;
> // <<
> }
> return result;
> }
>
>
>
>
> Please consider this as a bug, and patch it in cvs. I did not test this
> very well, so the described patch may possibly cause some incosistencies
> somewhere - the people who wrote that class will know best. It does work
> for my case though.
>
>
> And thanks for PostgreSQL, I've just started using it and I love it.
>
>
>
> - Marko
>
I think the code is still buggy. Specifically:
The following lines do not restore state:
//restore state of statement
String[] m_sqlFragments = l_origSqlFragments;
Object[] m_binds = l_origBinds;
String[] m_bindTypes = l_origBindTypes;
You are allocating new local variables here and you are setting them instead of inherited variables. I'm sure it's a copy-paste typo :) Should be like this:
this.m_sqlFragments = l_origSqlFragments;
this.m_binds = l_origBinds;
this.m_bindTypes = l_origBindTypes;
regards,
-Marko
> -----Original Message-----
> From: Barry Lind [mailto:blind@xythos.com]
> Sent: Wednesday, November 20, 2002 9:08 AM
> To: Marko Štrukelj
> Cc: 'pgsql-jdbc@postgresql.org'
> Subject: Re: [JDBC] jdbc bug/feature?
>
>
> Marko,
>
> I checked in a fix for this problem. It should appear in 7.3RC2 this
> weekend.
>
> thanks,
> --Barry
>
>
> Marko Štrukelj wrote:
> >
> > Hello,
> >
> > There is a bug (I say) or a lack of proper functionality in
> JDBC driver
> > regarding batch updates.
> >
> > The following code is expected to work by some library that
> I use and it
> > does work with inet tds driver for M$ SQL server. It
> however does not
> > work with the latest jdbc from cvs or anything older than
> that either.
> >
> >
> > PreparedStatements st = c.prepareStatement("insert into my_table
> > (field1, field2) values (?, ?)");
> >
> > // we have say 30 inserts to do
> > Iterator it = inserts.iterator();
> > int i=0;
> > while(it.hasNext()) {
> >
> > // there is a maximum batch size of 15, so we must
> periodically
> > execute it
> > if(i==batchMax) {
> > st.executeBatch();
> > i=0;
> >
> > // the problem - after this call a
> PreparedStatement is
> > empty and consequtive binds fail
> > }
> >
> > Object [] binds = (Object [])it.next();
> > st.setString(binds[0]);
> > st.setString(binds[1]);
> > st.addBatch();
> > i++;
> > }
> >
> >
> >
> > Upon some research I discovered the problem and found a very simple
> > solution:
> >
> > In org.postgresql.jdbc2.AbstractJdbc2Statement:
> >
> > public int[] executeBatch() throws SQLException
> > {
> > System.out.println("### executeBatch");
> > if (batch == null)
> > batch = new Vector();
> > int size = batch.size();
> > int[] result = new int[size];
> > int i = 0;
> >
> > // >>> added
> > // save m_binds and m_sqlFragments because
> > executeUpdate(String) will destroy them
> > String [] oldFrags = m_sqlFragments;
> > Object [] oldMBinds = m_binds;
> > // <<
> >
> > try
> > {
> > for (i = 0;i < size;i++)
> > result[i] =
> > this.executeUpdate((String)batch.elementAt(i));
> >
> > }
> > catch (SQLException e)
> > {
> > int[] resultSucceeded = new int[i];
> > System.arraycopy(result, 0,
> resultSucceeded, 0, i);
> >
> > PBatchUpdateException updex =
> > new
> > PBatchUpdateException("postgresql.stat.batch.error",
> >
>
> > new Integer(i), batch.elementAt(i), resultSucceeded);
> >
> > updex.setNextException(e);
> >
> > throw updex;
> > }
> > finally
> > {
> > batch.removeAllElements();
> > // >> added
> > // restore m_binds and m_sqlFragments
> > m_sqlFragments = oldFrags;
> > m_binds = oldMBinds;
> > // <<
> > }
> > return result;
> > }
> >
> >
> >
> >
> > Please consider this as a bug, and patch it in cvs. I did
> not test this
> > very well, so the described patch may possibly cause some
> incosistencies
> > somewhere - the people who wrote that class will know best.
> It does work
> > for my case though.
> >
> >
> > And thanks for PostgreSQL, I've just started using it and I love it.
> >
> >
> >
> > - Marko
> >
>
>
>
> ---------------------------(end of
> broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster
>
Marko,
You are correct. It was a copy-paste error. I will fix it up today.
Thanks for the quick code review.
--Barry
Marko Štrukelj wrote:
>
> I think the code is still buggy. Specifically:
>
>
> The following lines do not restore state:
>
> //restore state of statement
> String[] m_sqlFragments = l_origSqlFragments;
> Object[] m_binds = l_origBinds;
> String[] m_bindTypes = l_origBindTypes;
>
>
> You are allocating new local variables here and you are setting them
> instead of inherited variables. I'm sure it's a copy-paste typo :)
> Should be like this:
>
> this.m_sqlFragments = l_origSqlFragments;
> this.m_binds = l_origBinds;
> this.m_bindTypes = l_origBindTypes;
>
>
>
> regards,
>
> -Marko
>
>
> > -----Original Message-----
> > From: Barry Lind [mailto:blind@xythos.com]
> > Sent: Wednesday, November 20, 2002 9:08 AM
> > To: Marko Štrukelj
> > Cc: 'pgsql-jdbc@postgresql.org'
> > Subject: Re: [JDBC] jdbc bug/feature?
> >
> >
> > Marko,
> >
> > I checked in a fix for this problem. It should appear in 7.3RC2 this
> > weekend.
> >
> > thanks,
> > --Barry
> >
> >
> > Marko Štrukelj wrote:
> > >
> > > Hello,
> > >
> > > There is a bug (I say) or a lack of proper functionality in
> > JDBC driver
> > > regarding batch updates.
> > >
> > > The following code is expected to work by some library that
> > I use and it
> > > does work with inet tds driver for M$ SQL server. It
> > however does not
> > > work with the latest jdbc from cvs or anything older than
> > that either.
> > >
> > >
> > > PreparedStatements st = c.prepareStatement("insert into my_table
> > > (field1, field2) values (?, ?)");
> > >
> > > // we have say 30 inserts to do
> > > Iterator it = inserts.iterator();
> > > int i=0;
> > > while(it.hasNext()) {
> > >
> > > // there is a maximum batch size of 15, so we must
> > periodically
> > > execute it
> > > if(i==batchMax) {
> > > st.executeBatch();
> > > i=0;
> > >
> > > // the problem - after this call a
> > PreparedStatement is
> > > empty and consequtive binds fail
> > > }
> > >
> > > Object [] binds = (Object [])it.next();
> > > st.setString(binds[0]);
> > > st.setString(binds[1]);
> > > st.addBatch();
> > > i++;
> > > }
> > >
> > >
> > >
> > > Upon some research I discovered the problem and found a very simple
> > > solution:
> > >
> > > In org.postgresql.jdbc2.AbstractJdbc2Statement:
> > >
> > > public int[] executeBatch() throws SQLException
> > > {
> > > System.out.println("### executeBatch");
> > > if (batch == null)
> > > batch = new Vector();
> > > int size = batch.size();
> > > int[] result = new int[size];
> > > int i = 0;
> > >
> > > // >>> added
> > > // save m_binds and m_sqlFragments because
> > > executeUpdate(String) will destroy them
> > > String [] oldFrags = m_sqlFragments;
> > > Object [] oldMBinds = m_binds;
> > > // <<
> > >
> > > try
> > > {
> > > for (i = 0;i < size;i++)
> > > result[i] =
> > > this.executeUpdate((String)batch.elementAt(i));
> > >
> > > }
> > > catch (SQLException e)
> > > {
> > > int[] resultSucceeded = new int[i];
> > > System.arraycopy(result, 0,
> > resultSucceeded, 0, i);
> > >
> > > PBatchUpdateException updex =
> > > new
> > > PBatchUpdateException("postgresql.stat.batch.error",
> > >
> >
> > > new Integer(i), batch.elementAt(i), resultSucceeded);
> > >
> > > updex.setNextException(e);
> > >
> > > throw updex;
> > > }
> > > finally
> > > {
> > > batch.removeAllElements();
> > > // >> added
> > > // restore m_binds and m_sqlFragments
> > > m_sqlFragments = oldFrags;
> > > m_binds = oldMBinds;
> > > // <<
> > > }
> > > return result;
> > > }
> > >
> > >
> > >
> > >
> > > Please consider this as a bug, and patch it in cvs. I did
> > not test this
> > > very well, so the described patch may possibly cause some
> > incosistencies
> > > somewhere - the people who wrote that class will know best.
> > It does work
> > > for my case though.
> > >
> > >
> > > And thanks for PostgreSQL, I've just started using it and I love it.
> > >
> > >
> > >
> > > - Marko
> > >
> >
> >
> >
> > ---------------------------(end of
> > broadcast)---------------------------
> > TIP 4: Don't 'kill -9' the postmaster
> >
>