Permalink
Browse files

fix: better support for RETURNING for WITH queries and queries with c…

…omments (#1138)

1) pgjdbc ignored to add RETURNING to WITH queries even though it makes sense.
2) `insert/*comment*/` was not treated as `insert` statement, thus the driver did not add `returning`.

fixes #1104
  • Loading branch information...
vlsi committed Mar 14, 2018
1 parent bcdd427 commit 04e76661586b54157a1220552c005569231388a9
@@ -13,6 +13,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
- Wrong data from Blob/Clob when mark/reset is used [PR 971](https://github.com/pgjdbc/pgjdbc/pull/971)
- Adjust XAException return codes for better compatibility with XA specification [PR 782](https://github.com/pgjdbc/pgjdbc/pull/782)
- Wrong results when single statement is used with different bind types[PR 1137](https://github.com/pgjdbc/pgjdbc/pull/1137)
- Support generated keys for WITH queries that miss RETURNING [PR 1138](https://github.com/pgjdbc/pgjdbc/pull/1138)
- Support generated keys when INSERT/UPDATE/DELETE keyword is followed by a comment [PR 1138](https://github.com/pgjdbc/pgjdbc/pull/1138)
## [42.2.1] (2018-01-25)
### Known issues
@@ -74,11 +74,13 @@
boolean whitespaceOnly = true;
int keyWordCount = 0;
int keywordStart = -1;
int keywordEnd = -1;
for (int i = 0; i < aChars.length; ++i) {
char aChar = aChars[i];
boolean isKeyWordChar = false;
// ';' is ignored as it splits the queries
whitespaceOnly &= aChar == ';' || Character.isWhitespace(aChar);
keywordEnd = i; // parseSingleQuotes, parseDoubleQuotes, etc move index so we keep old value
switch (aChar) {
case '\'': // single-quotes
i = Parser.parseSingleQuotes(aChars, i, standardConformingStrings);
@@ -202,7 +204,7 @@
break;
}
if (keywordStart >= 0 && (i == aChars.length - 1 || !isKeyWordChar)) {
int wordLength = (isKeyWordChar ? i + 1 : i) - keywordStart;
int wordLength = (isKeyWordChar ? i + 1 : keywordEnd) - keywordStart;
if (currentCommandType == SqlCommandType.BLANK) {
if (wordLength == 6 && parseUpdateKeyword(aChars, keywordStart)) {
currentCommandType = SqlCommandType.UPDATE;
@@ -225,6 +227,12 @@
isCurrentReWriteCompatible = false;
}
}
} else if (currentCommandType == SqlCommandType.WITH
&& inParen == 0) {
SqlCommandType command = parseWithCommandType(aChars, i, keywordStart, wordLength);
if (command != null) {
currentCommandType = command;
}
}
if (inParen != 0 || aChar == ')') {
// RETURNING and VALUES cannot be present in braces
@@ -287,14 +295,56 @@
return nativeQueries;
}
private static SqlCommandType parseWithCommandType(char[] aChars, int i, int keywordStart,
int wordLength) {
// This parses `with x as (...) ...`
// Corner case is `with select as (insert ..) select * from select
SqlCommandType command;
if (wordLength == 6 && parseUpdateKeyword(aChars, keywordStart)) {
command = SqlCommandType.UPDATE;
} else if (wordLength == 6 && parseDeleteKeyword(aChars, keywordStart)) {
command = SqlCommandType.DELETE;
} else if (wordLength == 6 && parseInsertKeyword(aChars, keywordStart)) {
command = SqlCommandType.INSERT;
} else if (wordLength == 6 && parseSelectKeyword(aChars, keywordStart)) {
command = SqlCommandType.SELECT;
} else {
return null;
}
// update/delete/insert/select keyword detected
// Check if `AS` follows
int nextInd = i;
// The loop should skip whitespace and comments
for (; nextInd < aChars.length; nextInd++) {
char nextChar = aChars[nextInd];
if (nextChar == '-') {
nextInd = Parser.parseLineComment(aChars, nextInd);
} else if (nextChar == '/') {
nextInd = Parser.parseBlockComment(aChars, nextInd);
} else if (Character.isWhitespace(nextChar)) {
// Skip whitespace
continue;
} else {
break;
}
}
if (nextInd + 2 >= aChars.length
|| (!parseAsKeyword(aChars, nextInd)
|| isIdentifierContChar(aChars[nextInd + 2]))) {
return command;
}
return null;
}
private static boolean addReturning(StringBuilder nativeSql, SqlCommandType currentCommandType,
String[] returningColumnNames, boolean isReturningPresent) throws SQLException {
if (isReturningPresent || returningColumnNames.length == 0) {
return false;
}
if (currentCommandType != SqlCommandType.INSERT
&& currentCommandType != SqlCommandType.UPDATE
&& currentCommandType != SqlCommandType.DELETE) {
&& currentCommandType != SqlCommandType.DELETE
&& currentCommandType != SqlCommandType.WITH) {
return false;
}
@@ -659,6 +709,22 @@ public static boolean parseWithKeyword(final char[] query, int offset) {
&& (query[offset + 3] | 32) == 'h';
}
/**
* Parse string to check presence of AS keyword regardless of case.
*
* @param query char[] of the query statement
* @param offset position of query to start checking
* @return boolean indicates presence of word
*/
public static boolean parseAsKeyword(final char[] query, int offset) {
if (query.length < (offset + 2)) {
return false;
}
return (query[offset] | 32) == 'a'
&& (query[offset + 1] | 32) == 's';
}
/**
* @param c character
* @return true if the character is a whitespace character as defined in the backend's parser
@@ -6,6 +6,8 @@
package org.postgresql.core;
import static org.postgresql.core.SqlCommandType.INSERT;
import static org.postgresql.core.SqlCommandType.SELECT;
import static org.postgresql.core.SqlCommandType.WITH;
/**
* Data Modification Language inspection support.
@@ -37,6 +39,10 @@ public boolean isReturningKeywordPresent() {
return parsedSQLhasRETURNINGKeyword;
}
public boolean returnsRows() {
return parsedSQLhasRETURNINGKeyword || commandType == SELECT || commandType == WITH;
}
public static SqlCommand createStatementTypeInfo(SqlCommandType type,
boolean isBatchedReWritePropertyConfigured,
int valuesBraceOpenPosition, int valuesBraceClosePosition, boolean isRETURNINGkeywordPresent,
@@ -162,6 +162,17 @@ public void insertSelectReturning() throws SQLException {
Assert.assertTrue("Query has a returning clause " + query, returningKeywordPresent);
}
@Test
public void insertReturningInWith() throws SQLException {
String query =
"with x as (insert into mytab(x) values(1) returning x) insert test(id, name) select 1, 'value' from test2";
List<NativeQuery> qry =
Parser.parseJdbcSql(
query, true, true, true, true);
boolean returningKeywordPresent = qry.get(0).command.isReturningKeywordPresent();
Assert.assertFalse("There's no top-level <<returning>> clause " + query, returningKeywordPresent);
}
@Test
public void insertBatchedReWriteOnConflict() throws SQLException {
String query = "insert into test(id, name) values (:id,:name) ON CONFLICT (id) DO NOTHING";
@@ -156,11 +156,19 @@ public void testInsert() throws SQLException {
}
@Test
public void testWith() throws SQLException {
public void testWithSelect() throws SQLException {
List<NativeQuery> queries;
queries = Parser.parseJdbcSql("with update as insert (update foo set (a=?,b=?,c=?)) select * from update", true, true, false, true);
queries = Parser.parseJdbcSql("with update as (update foo set (a=?,b=?,c=?)) select * from update", true, true, false, true);
NativeQuery query = queries.get(0);
assertEquals("This is a WITH command", SqlCommandType.WITH, query.command.getType());
assertEquals("with ... () select", SqlCommandType.SELECT, query.command.getType());
}
@Test
public void testWithInsert() throws SQLException {
List<NativeQuery> queries;
queries = Parser.parseJdbcSql("with update as (update foo set (a=?,b=?,c=?)) insert into table(select) values(1)", true, true, false, true);
NativeQuery query = queries.get(0);
assertEquals("with ... () insert", SqlCommandType.INSERT, query.command.getType());
}
@Test
@@ -13,10 +13,12 @@
import static org.junit.Assert.fail;
import org.postgresql.PGStatement;
import org.postgresql.core.ServerVersion;
import org.postgresql.test.TestUtil;
import org.postgresql.test.jdbc2.BaseTest4;
import org.postgresql.util.PSQLState;
import org.junit.Assume;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -198,7 +200,7 @@ public void testMultipleRows() throws SQLException {
public void testSerialWorks() throws SQLException {
Statement stmt = con.createStatement();
int count = stmt.executeUpdate(
"INSERT INTO genkeys (b,c) VALUES ('a', 2), ('b', 4)" + returningClause + "; ",
"INSERT/*fool parser*/ INTO genkeys (b,c) VALUES ('a', 2), ('b', 4)" + returningClause + "; ",
new String[]{"a"});
assertEquals(2, count);
ResultSet rs = stmt.getGeneratedKeys();
@@ -222,6 +224,37 @@ public void testUpdate() throws SQLException {
assertTrue(!rs.next());
}
@Test
public void testWithInsertInsert() throws SQLException {
assumeMinimumServerVersion(ServerVersion.v9_1);
Statement stmt = con.createStatement();
int count = stmt.executeUpdate(
"WITH x as (INSERT INTO genkeys (b,c) VALUES ('a', 2) returning c) insert into genkeys(a,b,c) VALUES (1, 'a', 2)" + returningClause + "",
new String[]{"c", "b"});
assertEquals(1, count);
ResultSet rs = stmt.getGeneratedKeys();
assertTrue(rs.next());
assertCB1(rs);
assertTrue(!rs.next());
}
@Test
public void testWithInsertSelect() throws SQLException {
assumeMinimumServerVersion(ServerVersion.v9_1);
Assume.assumeTrue(returningInQuery != ReturningInQuery.NO);
Statement stmt = con.createStatement();
int count = stmt.executeUpdate(
"WITH x as (INSERT INTO genkeys(a,b,c) VALUES (1, 'a', 2) " + returningClause
+ ") select * from x",
new String[]{"c", "b"});
assertEquals("rowcount", -1, count);
// TODO: should SELECT produce rows through getResultSet or getGeneratedKeys?
ResultSet rs = stmt.getResultSet();
assertTrue(rs.next());
assertCB1(rs);
assertTrue(!rs.next());
}
@Test
public void testDelete() throws SQLException {
Statement stmt = con.createStatement();
@@ -16,6 +16,7 @@
Jdbc3CallableStatementTest.class,
GeneratedKeysTest.class,
CompositeQueryParseTest.class,
SqlCommandParseTest.class,
Jdbc3SavepointTest.class,
TypesTest.class,
ResultSetTest.class,
@@ -0,0 +1,54 @@
/*
* Copyright (c) 2018, PostgreSQL Global Development Group
* See the LICENSE file in the project root for more information.
*/
package org.postgresql.test.jdbc3;
import static org.junit.Assert.assertEquals;
import org.postgresql.core.NativeQuery;
import org.postgresql.core.Parser;
import org.postgresql.core.SqlCommandType;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import java.sql.SQLException;
import java.util.Arrays;
import java.util.List;
@RunWith(Parameterized.class)
public class SqlCommandParseTest {
@Parameterized.Parameter(0)
public SqlCommandType type;
@Parameterized.Parameter(1)
public String sql;
@Parameterized.Parameters(name = "expected={0}, sql={1}")
public static Iterable<Object[]> data() {
return Arrays.asList(new Object[][]{
{SqlCommandType.INSERT, "insert/**/ into table(select) values(1)"},
{SqlCommandType.SELECT, "select'abc'/**/ as insert"},
{SqlCommandType.INSERT, "INSERT/*fool /*nest comments -- parser*/*/ INTO genkeys (b,c) VALUES ('a', 2), ('b', 4) SELECT"},
{SqlCommandType.INSERT, "with update as (update foo set (a=?,b=?,c=?)) insert into table(select) values(1)"},
{SqlCommandType.INSERT, "with update as (update foo set (a=?,b=?,c=?)) insert into table(select) select * from update"},
{SqlCommandType.INSERT, "with update as (update foo set (a=?,b=?,c=?)) insert/**/ into table(select) values(1)"},
{SqlCommandType.INSERT, "with update as (update foo set (a=?,b=?,c=?)) insert /**/ into table(select) values(1)"},
{SqlCommandType.SELECT, "with update as (update foo set (a=?,b=?,c=?)) insert --\nas () select 1"},
{SqlCommandType.SELECT, "with update as (update foo set (a=?,b=?,c=?)) insert --\n/* dfhg \n*/\nas () select 1"},
{SqlCommandType.SELECT, "WITH x as (INSERT INTO genkeys(a,b,c) VALUES (1, 'a', 2) returning returning a, b) select * from x"},
// No idea if it works, but it should be parsed as WITH
{SqlCommandType.WITH, "with update as (update foo set (a=?,b=?,c=?)) copy from stdin"},
});
}
@Test
public void run() throws SQLException {
List<NativeQuery> queries;
queries = Parser.parseJdbcSql(sql, true, true, false, true);
NativeQuery query = queries.get(0);
assertEquals(sql, type, query.command.getType());
}
}

0 comments on commit 04e7666

Please sign in to comment.