[#1797] SQL syntax errors when plain SQL contains comments with question

marks and SQL is executed as StatementType.STATIC_STATEMENT
This commit is contained in:
Lukas Eder 2012-09-05 20:35:55 +02:00
parent 55e7449efa
commit a32cc252ef
4 changed files with 158 additions and 12 deletions

View File

@ -40,6 +40,8 @@ import static junit.framework.Assert.assertFalse;
import static junit.framework.Assert.assertNotNull;
import static junit.framework.Assert.assertTrue;
import static org.jooq.SQLDialect.FIREBIRD;
import static org.jooq.SQLDialect.H2;
import static org.jooq.conf.StatementType.STATIC_STATEMENT;
import static org.jooq.impl.Factory.field;
import static org.jooq.impl.Factory.fieldByName;
import static org.jooq.impl.Factory.function;
@ -68,6 +70,7 @@ import org.jooq.ResultQuery;
import org.jooq.Table;
import org.jooq.TableRecord;
import org.jooq.UpdatableRecord;
import org.jooq.conf.Settings;
import org.jooq.impl.CustomCondition;
import org.jooq.impl.CustomField;
import org.jooq.impl.Factory;
@ -249,6 +252,56 @@ extends BaseTest<A, AP, B, S, B2S, BS, L, X, DATE, BOOL, D, T, U, I, IPK, T658,
assertEquals(Integer.valueOf(1), books.getValue(1, TBook_AUTHOR_ID()));
}
@Test
public void testPlainSQLAndComments() throws Exception {
// Skip comments test for most dialects, as the behaviour w.r.t. comments
// may differ
if (getDialect() != H2) {
log.info("SKIPPING", "Skip comments tests");
return;
}
// [#1797] Plain SQL should be allowed to contain comments. Special care
// must be taken when comments contain ' or ? characters
// Single-line comments
// --------------------
// Render bind values
Record record1 = create()
.fetchOne("select 1 x -- what's this ?'? \n" +
", '-- no comment' y from t_book \n" +
" -- what's this ?'?\r" +
"where id = ?", 1);
assertEquals(1, record1.getValue(0));
assertEquals("-- no comment", record1.getValue(1));
// Inline bind values
Record record2 = create(new Settings().withStatementType(STATIC_STATEMENT))
.fetchOne("select 1 x -- what's this ?'? \n" +
", '-- no comment' y from t_book \n" +
" -- what's this ?'?\r" +
"where id = ?", 1);
assertEquals(1, record2.getValue(0));
assertEquals("-- no comment", record2.getValue(1));
// Multi-line comments
// -------------------
// Render bind values
Record record3 = create()
.fetchOne("select /* what's this ?'?\n\r?'? */ 1 x, '/* no comment */' y from t_book where id = ?", 1);
assertEquals(1, record3.getValue(0));
assertEquals("/* no comment */", record3.getValue(1));
// Inline bind values
Record record4 = create(new Settings().withStatementType(STATIC_STATEMENT))
.fetchOne("select /* what's this ?'?\n\r?'? */ 1 x, '/* no comment */' y from t_book where id = ?", 1);
assertEquals(1, record4.getValue(0));
assertEquals("/* no comment */", record4.getValue(1));
}
@Test
public void testPlainSQLCRUD() throws Exception {
jOOQAbstractTest.reset = false;

View File

@ -837,6 +837,11 @@ public abstract class jOOQAbstractTest<
new PlainSQLTests(this).testPlainSQL();
}
@Test
public void testPlainSQLAndComments() throws Exception {
new PlainSQLTests(this).testPlainSQLAndComments();
}
@Test
public void testPlainSQLCRUD() throws Exception {
new PlainSQLTests(this).testPlainSQLCRUD();

View File

@ -386,37 +386,73 @@ final class Util {
static final void toSQLReference(RenderContext context, String sql, List<QueryPart> substitutes) {
int bindIndex = 0;
char[] sqlChars = sql.toCharArray();
boolean stringLiteral = false;
for (int i = 0; i < sqlChars.length; i++) {
// [#1797] Skip content inside of single-line comments, e.g.
// select 1 x -- what's this ?'?
// from t_book -- what's that ?'?
// where id = ?
if (peek(sqlChars, i, "--")) {
// Consume the complete comment
for (; sqlChars[i] != '\r' && sqlChars[i] != '\n'; context.sql(sqlChars[i++]));
// Consume the newline character
context.sql(sqlChars[i]);
}
// [#1797] Skip content inside of multi-line comments, e.g.
// select 1 x /* what's this ?'?
// I don't know ?'? */
// from t_book where id = ?
else if (peek(sqlChars, i, "/*")) {
// Consume the complete comment
for (; !peek(sqlChars, i, "*/"); context.sql(sqlChars[i++]));
// Consume the comment delimiter
context.sql(sqlChars[i++]);
context.sql(sqlChars[i]);
}
// [#1031] [#1032] Skip ? inside of string literals, e.g.
// insert into x values ('Hello? Anybody out there?');
if (sqlChars[i] == '\'') {
else if (sqlChars[i] == '\'') {
// Delimiter is actually an escaping apostrophe
if (i + 1 < sqlChars.length && sqlChars[i + 1] == '\'') {
// Consume the initial string literal delimiter
context.sql(sqlChars[i++]);
// Skip subsequent character
// Consume the whole string literal
for (;;) {
// Consume an escaped apostrophe
if (peek(sqlChars, i, "''")) {
context.sql(sqlChars[i++]);
}
// Break on the terminal string literal delimiter
else if (peek(sqlChars, i, "'")) {
break;
}
// Consume string literal content
context.sql(sqlChars[i++]);
context.sql(sqlChars[i]);
}
else {
stringLiteral = !stringLiteral;
context.sql(sqlChars[i]);
}
// Consume the terminal string literal delimiter
context.sql(sqlChars[i]);
}
// TODO: This case should be mutually exclusive with the next one
// Inline bind variables only outside of string literals
else if (sqlChars[i] == '?' && context.inline() && !stringLiteral && bindIndex < substitutes.size()) {
else if (sqlChars[i] == '?' && context.inline() && bindIndex < substitutes.size()) {
context.sql(substitutes.get(bindIndex++));
}
// TODO: This case should be mutually exclusive with the previous one
// [#1432] Inline substitues for {numbered placeholders} outside of string literals
else if (sqlChars[i] == '{' && !stringLiteral && bindIndex < substitutes.size()) {
else if (sqlChars[i] == '{' && bindIndex < substitutes.size()) {
// [#1461] Be careful not to match any JDBC escape syntax
if (JDBC_ESCAPE_PATTERN.matcher(sql.substring(i)).matches()) {
@ -451,6 +487,28 @@ final class Util {
}
}
/**
* Peek for a string at a given <code>index</code> of a <code>char[]</code>
*
* @param sqlChars The char array to peek into
* @param index The index within the char array to peek for a string
* @param peek The string to peek for
*/
static final boolean peek(char[] sqlChars, int index, String peek) {
char[] peekArray = peek.toCharArray();
for (int i = 0; i < peekArray.length; i++) {
if (index + i >= sqlChars.length) {
return false;
}
if (sqlChars[index + i] != peekArray[i]) {
return false;
}
}
return true;
}
/**
* Create {@link QueryPart} objects from bind values or substitutes
*/

View File

@ -1059,6 +1059,36 @@ public class BasicTest {
context.assertIsSatisfied();
}
@Test
public void testPlainSQLComments() throws Exception {
Field<?> f = field(
"-- comment ? '\n" +
"/* another comment ? '\n" +
" continuing -- */" +
"a bind value : ? /* a comment : ? */ another bind value : ?", "a", "b");
assertEquals(
"-- comment ? '\n" +
"/* another comment ? '\n" +
" continuing -- */" +
"a bind value : 'a' /* a comment : ? */ another bind value : 'b'", r_refI().render(f));
assertEquals(
"-- comment ? '\n" +
"/* another comment ? '\n" +
" continuing -- */" +
"a bind value : ? /* a comment : ? */ another bind value : ?", r_ref().render(f));
context.checking(new Expectations() {{
oneOf(statement).setString(1, "a");
oneOf(statement).setString(2, "b");
}});
int i = b_ref().bind(f).peekIndex();
assertEquals(3, i);
context.assertIsSatisfied();
}
@Test
public void testPlainSQLField() throws Exception {
Field<?> f1 = field("DECODE(TABLE1.ID, 1, 'a', 'b')");