rubenada commented on code in PR #5266:
URL: https://github.com/apache/calcite/pull/5266#discussion_r4024488307
##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchRules.java:
##########
@@ -136,10 +138,57 @@ static String stripQuotes(String s) {
? s.substring(1, s.length() - 1) : s;
}
- private static String escapeSpecialSymbols(String s) {
+ /** Escapes the two characters that would terminate a JSON string. A SQL
Review Comment:
Agree. Renamed to `escapeJsonString` to make the purpose of this method more
clear.
##########
cassandra/src/test/java/org/apache/calcite/adapter/cassandra/CassandraIdentifierQuotingTest.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.cassandra;
+
+import com.datastax.oss.driver.api.core.CqlIdentifier;
+
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+/**
+ * Unit tests documenting how {@link CqlIdentifier#fromInternal} is used in
+ * {@link CassandraTable#query} to emit the keyspace, column-family, column
+ * and alias tokens of a generated CQL {@code SELECT} statement. Any of
+ * those pieces is copied verbatim into the query text, so a value that
+ * contains punctuation or the {@code "} delimiter must be routed through
+ * the driver so that it cannot shift the boundaries of the emitted CQL.
Review Comment:
Not necessarily incorrect, but rather that it can "alter the semantics" of
the emitted CQL.
Changed it to use this expression, which is probably clearer.
##########
cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraTable.java:
##########
@@ -190,18 +197,20 @@ public Enumerable<Object> query(final CqlSession session,
List<Map.Entry<String,
whereClause += Util.toString(predicates, "", " AND ", "");
}
- // Build and issue the query and return an Enumerator over the results
+ // Build and issue the query and return an Enumerator over the results.
+ // Route the keyspace and column-family names through CqlIdentifier so
Review Comment:
ok, moved
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -3913,8 +3899,13 @@ private SqlDialect nonOrdinalDialect() {
@Test void testHiveAndSparkTrimWithBothSpecialCharacter() {
final String query = "SELECT TRIM(BOTH '$@*A' from
'$@*AABC$@*AADCAA$@*A')\n"
+ "from \"foodmart\".\"reserve_employee\"";
+ // Hive and Spark recognize backslash escape sequences inside single-quoted
+ // string constants, so each backslash produced by escapeSpecialChar is
+ // doubled by the dialect. The emitted regex therefore contains \\$\\@\\*A
+ // rather than \$\@\*A; when Hive/Spark parse the literal they collapse
+ // it back to \$\@\*A, which is what the REGEXP_REPLACE engine sees.
final String expected = "SELECT REGEXP_REPLACE('$@*AABC$@*AADCAA$@*A',"
- + " '^(\\$\\@\\*A)*|(\\$\\@\\*A)*$', '')\n"
+ + " '^(\\\\$\\\\@\\\\*A)*|(\\\\$\\\\@\\\\*A)*$', '')\n"
Review Comment:
The test was checking the conversion from TRIM BOTH into REGEXP_REPLACE, it
was just missing the backslash escape.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
