Skip to content

Commit

Permalink
Fixed internal error in pinot queries caused due to question mark (?)…
Browse files Browse the repository at this point in the history
… in query parameter. (#73)

Pinot PreparedStatement creates invalid query if one of the parameters has '?' in its value.
Sample pinot query : `select * from table where team in (?, ?, ?)`..
Now say parameters are: 'abc', 'pqr with (?)' and 'xyz'..

Now, on executing PreparedStatement:fillStatementWithParameters method on this will return
`select * from table where team in ('abc', 'pqr with ('xyz')', ?)`
-- which is clearly wrong
(what we wanted was `select * from table where team in ('abc', 'pqr with (?)', 'xyz')`)..

The reason is the usage of replaceFirst iteration in the pinot PreparedStatement method..
Hence written a custom method to resolve the query statement rather than relying on Pinot's library method.

Also raised an issue in incubator-pinot github repo: apache/pinot#6834
  • Loading branch information
singhalprerana authored Apr 25, 2021
1 parent b67c30d commit 1a80901
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,19 @@ private PinotClient(String pathType, String path) {
}

public ResultSetGroup executeQuery(String statement, Params params) {
/*
PreparedStatement preparedStatement = buildPreparedStatement(statement, params);
return preparedStatement.execute();
*/
return connection.execute(new Request(SQL_FORMAT, resolveStatement(statement, params)));
}

public Future<ResultSetGroup> executeQueryAsync(String statement, Params params) {
/*
PreparedStatement preparedStatement = buildPreparedStatement(statement, params);
return preparedStatement.executeAsync();
*/
return connection.executeAsync(new Request(SQL_FORMAT, resolveStatement(statement, params)));
}

private PreparedStatement buildPreparedStatement(String statement, Params params) {
Expand All @@ -102,5 +108,50 @@ private PreparedStatement buildPreparedStatement(String statement, Params params
.forEach((i, b) -> preparedStatement.setString(i, Hex.encodeHexString(b.toByteArray())));
return preparedStatement;
}

@VisibleForTesting
/*
* Pinot PreparedStatement creates invalid query if one of the parameters has '?' in its value.
* Sample pinot query : select * from table where team in (?, ?, ?).. Now say parameters are:
* 'abc', 'pqr with (?)' and 'xyz'..
*
* Now, on executing PreparedStatement:fillStatementWithParameters method on this will return
* select * from table where team in ('abc', 'pqr with ('xyz')', ?) -- which is clearly wrong
* (what we wanted was select * from table where team in ('abc', 'pqr with (?)', 'xyz'))..
*
* The reason is the usage of replaceFirst iteration in the pinot PreparedStatement method..
*
* Hence written this custom method to resolve the query statement rather than relying on Pinot's
* library method.
* This is a temporary fix and can be reverted when the Pinot issue gets resolved
* Raised an issue in incubator-pinot github repo: apache/incubator-pinot#6834
*/
static String resolveStatement(String query, Params params) {
if (query.isEmpty()) {
return query;
}
String[] queryParts = query.split("\\?");

String[] parameters = new String[queryParts.length];
params.getStringParams().forEach((i, p) -> parameters[i] = getStringParam(p));
params.getIntegerParams().forEach((i, p) -> parameters[i] = String.valueOf(p));
params.getLongParams().forEach((i, p) -> parameters[i] = String.valueOf(p));
params.getDoubleParams().forEach((i, p) -> parameters[i] = String.valueOf(p));
params.getFloatParams().forEach((i, p) -> parameters[i] = String.valueOf(p));
params
.getByteStringParams()
.forEach((i, p) -> parameters[i] = getStringParam(Hex.encodeHexString(p.toByteArray())));

StringBuilder sb = new StringBuilder();
for (int i = 0; i < queryParts.length; i++) {
sb.append(queryParts[i]);
sb.append(parameters[i] != null ? parameters[i] : "");
}
return sb.toString();
}

private static String getStringParam(String value) {
return "'" + value.replace("'", "''") + "'";
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,18 @@ public void testPinotQuery() {
Assertions.assertEquals(q2, q3);
Assertions.assertEquals(q3, q2);
}

@Test
public void testResolveStatement() {
String statement =
PinotClientFactory.PinotClient.resolveStatement(
"select * from table where team in (?, ?, ?)",
Params.newBuilder()
.addStringParam("abc")
.addStringParam("pqr with (?)")
.addStringParam("xyz")
.build());

Assertions.assertEquals("select * from table where team in ('abc', 'pqr with (?)', 'xyz')", statement);
}
}

0 comments on commit 1a80901

Please sign in to comment.