Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ private String translateOp2(String op, String name, RexLiteral right) {
requireNonNull(rowType.getField(name, true, false));
SqlTypeName typeName = field.getType().getSqlTypeName();
if (typeName != SqlTypeName.CHAR) {
valueString = "'" + valueString + "'";
valueString = "'" + valueString.replace("'", "''") + "'";

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the value is I''m fine, I think replace will return an unexpected results.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

String.replace here is literal, not regex, so it doubles every embedded quote independently. For the value I''m fine (two adjacent quotes) it produces 'I''''m fine', which the CQL/OQL parser reads back as exactly I''m fine, so it round-trips fine. This is the same doubling core already does in SqlDialect.quoteStringLiteral (val.replace(literalEndQuoteString, literalEscapedQuote)). I pushed a Geode test with that exact value to lock it in.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies, I misunderstood the situation. I think this change is good; you can create a Jira ticket and—following the example of other PRs—update the title and adjust the commit message accordingly.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries. I'll open a CALCITE ticket for this and update the PR title and commit message to the [CALCITE-XXXX] form to match the other PRs, and squash the two commits down to one while I'm at it.

}
}
return name + " " + op + " " + valueString;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,15 @@ static void load(CqlSession session) {
+ " CassandraTableScan(table=[[twissandra, userline]]");
}

@Test void testFilterWithSingleQuote() {
// A string literal containing a single quote must be escaped so it does not
// break out of the CQL string literal in the generated query.
CalciteAssert.that()
.with(TWISSANDRA)
.query("select * from \"userline\" where \"username\" = 'a''b'")
.returnsCount(0);
}

@Test void testFilterUUID() {
CalciteAssert.that()
.with(TWISSANDRA)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ private String translateBinary2(String op, RexNode left,
private static String quoteCharLiteral(RexLiteral literal) {
String value = literalValue(literal);
if (literal.getTypeName() == CHAR) {
value = "'" + value + "'";
value = "'" + value.replace("'", "''") + "'";
}
return value;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,17 @@ public void testJoin() {
GeodeAssertions.query(expectedQuery));
}

@Test void testFilterWithSingleQuoteLiteral() {
String expectedQuery = "SELECT city AS city FROM /zips "
+ "WHERE city = 'a''b'";
calciteAssert()
.query("SELECT city as city "
+ "FROM view WHERE city = 'a''b'")
.returnsCount(0)
.queryContains(
GeodeAssertions.query(expectedQuery));
}

@Test void testWhereWithOrForNumericField() {
calciteAssert()
.query("SELECT pop as pop "
Expand Down