Bug #25025 Connector/J getRewriteBatchedStatements() should be more forgiving of comments
Submitted: 13 Dec 2006 3:58 Modified: 16 May 2007 19:53
Reporter: Martin Ross Email Updates:
Status: Closed Impact on me:
Category:Connector / J Severity:S2 (Serious)
Version:5.0.x, 3.1.x OS:Any (any)
Assigned to: CPU Architecture:Any

[13 Dec 2006 3:58] Martin Ross
Current code inside executeBatch() scans the originalSql for the INSERT value in order to decide to enable batched performance.  See fragment below...

if (!this.batchHasPlainStatements
&& this.connection.getRewriteBatchedStatements()) {
if (StringUtils.startsWithIgnoreCaseAndWs(this.originalSql,
							"INSERT")) {
return executeBatchedInserts();
BUT executeBatchInserts() is not called the case that comments placed in the SQL string ahead of the INSERT.

/* insert com.medicaltelecom.hibernate.model.Services */
/* insert com.medicaltelecom.hibernate.model.Services */ INSERT into Services()

I classify this issue as serious because Hibernate has an option (use_sql_comments) that will prepend SQL comments to the SQL string.  Thus any batch inserting will be broken on Hibernate.

How to repeat:
See above.

Suggested fix:
Allow comments to be embedded before the insert.
[2 Jan 2007 18:56] Tonci Grgin
Hi Martin and thanks for your problem report. Same code is present in both 3.1 and 5.0 PreparedStatement.java sources preventing adding of comments.
[3 Jan 2007 20:37] Mark Matthews
rewriteBatchedStatements=true is a workaround for missing functionality in the server-side implementation of prepared statements (array binding). Short of putting a full SQL parser in the JDBC driver, we can't expand a lot of effort on improving the feature until either the server implements this functionality, or other more important features (JDBC-4.0 support for example) are completed.
[3 Jan 2007 20:55] Martin Ross
Fair enough.  But at a minimum you should update documentation to tell hibernate users to make sure that use_sql_comments is disabled.  Otherwise Hibernate users are going to be really screwed over by this issue since statement batching is a massive performance improvement.  

Either that or code a dirty regexp that just strips out any leading /* */...  

If you are okay with including it then I can whip up a few line fix for this...

[3 Jan 2007 23:57] Mark Matthews
Notice that batching won't be broken in this case, just that performance will suffer (to clarify the comment in the original bug report).
[4 Jan 2007 8:35] MC Brown
Set to documenting.
[4 Jan 2007 10:33] MC Brown
I've added a new section to the documentation on using Hibernate and C/J together, which currently only consists of a note on this configuration issue: 

Within Hibernate, do not enable the use_sql_comments option, as this can introduce comments
before the real SQL statements that may confuse the parsing mechanism in Connector/J.

Setting status back to 'To be fixed later'
[4 Jan 2007 18:22] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

[5 Jan 2007 5:55] Martin Ross
Given that Mark has committed the patch it seems that we don't need to add the documentation comment.
[5 Jan 2007 7:48] Tonci Grgin
Martin, please recheck with your test case and confirm your troubles have gone away.
[26 Jan 2007 20:27] Martin Ross
Work for me..  Will this make it into next release?


[29 Jan 2007 8:38] Tonci Grgin
Hi Martin. I can't be 100% sure but it's likely to appear in next release.