This is an automated email from the ASF dual-hosted git repository.
github-actions[bot] pushed a commit to branch site
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/site by this push:
new 2fbcb9c5b0 [CALCITE-7781] Improve identifier and literal handling in
SQL dialects and adapters
2fbcb9c5b0 is described below
commit 2fbcb9c5b037998b4d99071e886ebd2e52f3fb91
Author: Ruben Quesada Lopez <[email protected]>
AuthorDate: Tue Sep 15 12:12:30 2026 +0100
[CALCITE-7781] Improve identifier and literal handling in SQL dialects and
adapters
---
.../calcite/adapter/cassandra/CassandraTable.java | 19 ++-
.../cassandra/CassandraIdentifierQuotingTest.java | 80 ++++++++++++
.../calcite/config/CalciteSystemProperty.java | 16 +++
.../java/org/apache/calcite/sql/SqlDialect.java | 8 ++
.../apache/calcite/sql/SqlDialectFactoryImpl.java | 33 ++++-
.../calcite/sql/dialect/BigQuerySqlDialect.java | 18 +++
.../calcite/sql/dialect/ClickHouseSqlDialect.java | 18 +++
.../calcite/sql/dialect/FireboltSqlDialect.java | 9 ++
.../apache/calcite/sql/dialect/HiveSqlDialect.java | 10 ++
.../calcite/sql/dialect/InfobrightSqlDialect.java | 12 ++
.../calcite/sql/dialect/NetezzaSqlDialect.java | 13 ++
.../calcite/sql/dialect/SnowflakeSqlDialect.java | 10 ++
.../calcite/sql/dialect/SparkSqlDialect.java | 10 ++
.../calcite/rel/rel2sql/RelToSqlConverterTest.java | 73 ++++++++++-
.../org/apache/calcite/sql/SqlDialectsTest.java | 26 ++++
.../elasticsearch/ElasticsearchProject.java | 34 +++--
.../adapter/elasticsearch/ElasticsearchRules.java | 59 ++++++++-
.../elasticsearch/ElasticsearchRulesTest.java | 139 +++++++++++++++++++++
.../calcite/adapter/geode/rel/GeodeAggregate.java | 13 +-
.../calcite/adapter/geode/rel/GeodeProject.java | 6 +-
.../calcite/adapter/geode/rel/GeodeRules.java | 64 +++++++++-
.../calcite/adapter/geode/rel/GeodeSort.java | 7 +-
.../calcite/adapter/geode/rel/GeodeTable.java | 91 +++++++++++++-
.../calcite/adapter/geode/util/GeodeUtils.java | 32 +++++
.../calcite/adapter/geode/rel/GeodeRulesTest.java | 137 ++++++++++++++++++++
.../geode/rel/GeodeTableOqlValidationTest.java | 116 +++++++++++++++++
.../calcite/adapter/geode/util/GeodeUtilsTest.java | 128 +++++++++++++++++++
.../calcite/adapter/mongodb/MongoAggregate.java | 6 +-
.../calcite/adapter/mongodb/MongoRulesTest.java | 78 ++++++++++++
site/_docs/history.md | 9 ++
.../calcite/adapter/splunk/SplunkPushDownRule.java | 55 +++++++-
.../adapter/splunk/SplunkPushDownRuleTest.java | 107 ++++++++++++++++
32 files changed, 1400 insertions(+), 36 deletions(-)
diff --git
a/cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraTable.java
b/cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraTable.java
index c28ada76e9..de8464e25d 100644
---
a/cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraTable.java
+++
b/cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraTable.java
@@ -38,6 +38,7 @@
import org.apache.calcite.sql.type.SqlTypeFactoryImpl;
import org.apache.calcite.util.Util;
+import com.datastax.oss.driver.api.core.CqlIdentifier;
import com.datastax.oss.driver.api.core.CqlSession;
import com.datastax.oss.driver.api.core.cql.ResultSet;
import com.google.common.collect.ImmutableList;
@@ -173,7 +174,13 @@ public Enumerable<Object> query(final CqlSession session,
List<Map.Entry<String,
@Override public String next() {
Map.Entry<String, String> entry = selectIterator.next();
- return entry.getKey() + " AS " + entry.getValue();
+ // Route both the column reference and the alias through
+ // CqlIdentifier so that a name containing punctuation or the
+ // {@code "} delimiter cannot shift the boundaries of the
+ // emitted CQL statement.
+ return CqlIdentifier.fromInternal(entry.getKey()).asCql(true)
+ + " AS "
+ + CqlIdentifier.fromInternal(entry.getValue()).asCql(true);
}
@Override public void remove() {
@@ -194,14 +201,16 @@ public Enumerable<Object> query(final CqlSession session,
List<Map.Entry<String,
StringBuilder queryBuilder = new StringBuilder("SELECT ");
queryBuilder
.append(selectString)
- .append(" FROM \"");
+ .append(" FROM ");
+ // Route the keyspace and column-family names through CqlIdentifier so
+ // that a name containing punctuation or the {@code "} delimiter cannot
+ // shift the boundaries of the emitted CQL statement
keyspace.ifPresent(s ->
- queryBuilder.append(s).append("\".\""));
+
queryBuilder.append(CqlIdentifier.fromInternal(s).asCql(true)).append("."));
queryBuilder
- .append(columnFamily)
- .append("\"")
+ .append(CqlIdentifier.fromInternal(columnFamily).asCql(true))
.append(whereClause);
if (!order.isEmpty()) {
diff --git
a/cassandra/src/test/java/org/apache/calcite/adapter/cassandra/CassandraIdentifierQuotingTest.java
b/cassandra/src/test/java/org/apache/calcite/adapter/cassandra/CassandraIdentifierQuotingTest.java
new file mode 100644
index 0000000000..a053d58975
--- /dev/null
+++
b/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 alter the semantics of the emitted CQL.
+ */
+class CassandraIdentifierQuotingTest {
+
+ /** Plain names round-trip unchanged. */
+ @Test void plainNameUnquoted() {
+ assertThat(CqlIdentifier.fromInternal("field").asCql(true), is("field"));
+ }
+
+ /** An uppercase name gets a double-quoted rendering so its case is
+ * preserved on the server side. */
+ @Test void mixedCaseGetsQuoted() {
+ assertThat(CqlIdentifier.fromInternal("Field").asCql(true),
+ is("\"Field\""));
+ }
+
+ /** A name containing a space cannot be emitted verbatim; the driver
+ * wraps it in double quotes. */
+ @Test void nameWithSpaceGetsQuoted() {
+ assertThat(CqlIdentifier.fromInternal("a b").asCql(true),
+ is("\"a b\""));
+ }
+
+ /** A double quote inside the value would otherwise terminate the
+ * enclosing quoted identifier and shift the token boundary; the
+ * driver escapes it by doubling. */
+ @Test void embeddedDoubleQuoteIsDoubled() {
+ assertThat(CqlIdentifier.fromInternal("a\"b").asCql(true),
+ is("\"a\"\"b\""));
+ }
+
+ /** A trailing double quote must also be doubled or it would consume
+ * the closing quote of the enclosing identifier. */
+ @Test void trailingDoubleQuoteIsDoubled() {
+ assertThat(CqlIdentifier.fromInternal("a\"").asCql(true),
+ is("\"a\"\"\""));
+ }
+
+ /** Punctuation that would carry CQL structure (a comma, a
+ * semicolon, a parenthesis) is neutralised by the same quoting. */
+ @Test void punctuationIsQuoted() {
+ assertThat(CqlIdentifier.fromInternal("a,b").asCql(true),
+ is("\"a,b\""));
+ assertThat(CqlIdentifier.fromInternal("a;b").asCql(true),
+ is("\"a;b\""));
+ assertThat(CqlIdentifier.fromInternal("a(b)").asCql(true),
+ is("\"a(b)\""));
+ }
+}
diff --git
a/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
b/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
index 757f34cb26..dd8fdd3c7e 100644
--- a/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
+++ b/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
@@ -121,6 +121,22 @@ public final class CalciteSystemProperty<T> {
public static final CalciteSystemProperty<Boolean> UUID_UNSIGNED_COMPARISON =
booleanProperty("calcite.uuid.unsigned.comparison", true);
+ /**
+ * Whether the fallback dialect that
+ * {@link org.apache.calcite.sql.SqlDialectFactory} uses for an
+ * unrecognized database product doubles backslashes when quoting string
+ * literals.
+ *
+ * <p>The unrecognized backend's string-lexing rules are unknown, so the
+ * default is to double backslashes: this keeps a value from terminating a
+ * generated string literal early on a backend that treats backslash as an
+ * in-string escape character (the MySQL family, among others). Set to
+ * "false" to restore the legacy behavior for backends known to follow the
+ * SQL standard.
+ */
+ public static final CalciteSystemProperty<Boolean>
UNKNOWN_DIALECT_ESCAPES_BACKSLASH =
+ booleanProperty("calcite.sql.dialect.unknown.escapes.backslash", true);
+
/**
* Whether to include a GraphViz representation when dumping the state of the
* Volcano planner.
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlDialect.java
b/core/src/main/java/org/apache/calcite/sql/SqlDialect.java
index 869976f0d4..fe84c7808a 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlDialect.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlDialect.java
@@ -429,6 +429,14 @@ public final String quoteStringLiteral(String val) {
}
/** Appends a string literal to a buffer.
+ *
+ * <p>The default implementation escapes only the literal quote character
+ * (by doubling it). If the target backend also treats backslash as an
+ * in-string escape character (as MySQL, MariaDB and BigQuery do), the
+ * dialect must override this method and double backslashes as well via
+ * {@link #escapeBackslash(String)}; otherwise a value ending in a
+ * backslash escapes the closing quote, shifts the boundary of the emitted
+ * literal, and changes the structure of the generated statement.
*
* @param buf Buffer
* @param charsetName Character set name, e.g. "utf16", or null
diff --git
a/core/src/main/java/org/apache/calcite/sql/SqlDialectFactoryImpl.java
b/core/src/main/java/org/apache/calcite/sql/SqlDialectFactoryImpl.java
index 3b8de0d65c..486442f72a 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlDialectFactoryImpl.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlDialectFactoryImpl.java
@@ -16,6 +16,7 @@
*/
package org.apache.calcite.sql;
+import org.apache.calcite.config.CalciteSystemProperty;
import org.apache.calcite.sql.dialect.AccessSqlDialect;
import org.apache.calcite.sql.dialect.AnsiSqlDialect;
import org.apache.calcite.sql.dialect.BigQuerySqlDialect;
@@ -168,10 +169,40 @@ public class SqlDialectFactoryImpl implements
SqlDialectFactory {
} else if (upperProductName.contains("SPARK")) {
return new SparkSqlDialect(c);
} else {
- return new AnsiSqlDialect(c);
+ return ansiFallback(c);
}
}
+ /** Creates the dialect used when the database product name is not
+ * recognized.
+ *
+ * <p>Because the backend's string-literal lexing rules are unknown, the
+ * fallback doubles backslashes when quoting string literals by default:
+ * on a backend that treats {@code \} as an in-string escape character
+ * (the MySQL family, among others), an unescaped backslash at the end of
+ * a value such as {@code "x\"} would otherwise consume the closing quote
+ * and shift the string boundary, so the rest of the value would be parsed
+ * as SQL rather than data. On a backend that lexes string literals per
+ * the SQL standard, the doubled backslash is at worst rendered verbatim;
+ * it cannot change the structure of the generated statement.
+ *
+ * <p>Set the system property
+ * {@code calcite.sql.dialect.unknown.escapes.backslash} to {@code false}
+ * to restore the legacy behavior (no backslash escaping) for a backend
+ * known to follow the SQL standard; the robust fix is to add a proper
+ * {@link SqlDialect} for the product. */
+ static SqlDialect ansiFallback(SqlDialect.Context context) {
+ return new AnsiSqlDialect(context) {
+ @Override public void quoteStringLiteral(StringBuilder buf,
+ @Nullable String charsetName, String val) {
+ super.quoteStringLiteral(buf, charsetName,
+ CalciteSystemProperty.UNKNOWN_DIALECT_ESCAPES_BACKSLASH.value()
+ ? escapeBackslash(val)
+ : val);
+ }
+ };
+ }
+
/** Returns a basic dialect for a given product, or null if none is known. */
static @Nullable SqlDialect simple(SqlDialect.DatabaseProduct
databaseProduct) {
switch (databaseProduct) {
diff --git
a/core/src/main/java/org/apache/calcite/sql/dialect/BigQuerySqlDialect.java
b/core/src/main/java/org/apache/calcite/sql/dialect/BigQuerySqlDialect.java
index 6e5ebeb4c0..66cc02688c 100644
--- a/core/src/main/java/org/apache/calcite/sql/dialect/BigQuerySqlDialect.java
+++ b/core/src/main/java/org/apache/calcite/sql/dialect/BigQuerySqlDialect.java
@@ -117,6 +117,24 @@ public BigQuerySqlDialect(SqlDialect.Context context) {
|| RESERVED_KEYWORDS.contains(val.toUpperCase(Locale.ROOT));
}
+ @Override public void quoteStringLiteral(StringBuilder buf,
+ @Nullable String charsetName, String val) {
+ // BigQuery treats backslash as an escape character inside string literals,
+ // so a literal backslash must be doubled before the base method escapes
the
+ // enclosing quote as \'. Otherwise a value containing a backslash (e.g.
+ // "x\" or "\'; ...") terminates the literal early and the trailing text is
+ // parsed as SQL rather than data.
+ super.quoteStringLiteral(buf, charsetName, escapeBackslash(val));
+ }
+
+ @Override public StringBuilder quoteIdentifier(StringBuilder buf, String
val) {
+ // BigQuery applies the same backslash escape sequences inside back-tick
+ // quoted identifiers as inside string literals, so a backslash in an
+ // identifier must be doubled or it would escape the closing back-tick and
+ // shift the identifier boundary.
+ return super.quoteIdentifier(buf, escapeBackslash(val));
+ }
+
@Override public boolean supportsImplicitTypeCoercion(RexCall call) {
return super.supportsImplicitTypeCoercion(call)
&& RexUtil.isLiteral(call.getOperands().get(0), false)
diff --git
a/core/src/main/java/org/apache/calcite/sql/dialect/ClickHouseSqlDialect.java
b/core/src/main/java/org/apache/calcite/sql/dialect/ClickHouseSqlDialect.java
index fd5a719e1a..d2c1df2c46 100644
---
a/core/src/main/java/org/apache/calcite/sql/dialect/ClickHouseSqlDialect.java
+++
b/core/src/main/java/org/apache/calcite/sql/dialect/ClickHouseSqlDialect.java
@@ -99,6 +99,24 @@ public ClickHouseSqlDialect(Context context) {
return false;
}
+ @Override public void quoteStringLiteral(StringBuilder buf,
+ @Nullable String charsetName, String val) {
+ // ClickHouse treats backslash as an escape character inside string
+ // literals, so a literal backslash must be doubled before the base method
+ // escapes the enclosing quote. Otherwise a value ending in a backslash
+ // terminates the literal early and the trailing text is parsed as SQL
+ // rather than data.
+ super.quoteStringLiteral(buf, charsetName, escapeBackslash(val));
+ }
+
+ @Override public StringBuilder quoteIdentifier(StringBuilder buf, String
val) {
+ // ClickHouse applies the same backslash escape sequences inside
+ // back-tick-quoted identifiers as inside string literals, so a backslash
+ // in an identifier must be doubled or it would escape the closing
+ // back-tick and shift the identifier boundary.
+ return super.quoteIdentifier(buf, escapeBackslash(val));
+ }
+
@Override public boolean supportsNestedAggregations() {
return false;
}
diff --git
a/core/src/main/java/org/apache/calcite/sql/dialect/FireboltSqlDialect.java
b/core/src/main/java/org/apache/calcite/sql/dialect/FireboltSqlDialect.java
index 49b74d07ed..681280d374 100644
--- a/core/src/main/java/org/apache/calcite/sql/dialect/FireboltSqlDialect.java
+++ b/core/src/main/java/org/apache/calcite/sql/dialect/FireboltSqlDialect.java
@@ -100,6 +100,15 @@ public FireboltSqlDialect(Context context) {
return false;
}
+ @Override public void quoteStringLiteral(StringBuilder buf,
+ @Nullable String charsetName, String val) {
+ // Firebolt treats backslash as an escape character inside string literals,
+ // so a literal backslash must be doubled before the base method escapes
the
+ // enclosing quote. Otherwise a value ending in a backslash terminates the
+ // literal early and the trailing text is parsed as SQL rather than data.
+ super.quoteStringLiteral(buf, charsetName, escapeBackslash(val));
+ }
+
@Override public void unparseOffsetFetch(SqlWriter writer, @Nullable SqlNode
offset,
@Nullable SqlNode fetch) {
unparseFetchUsingLimit(writer, offset, fetch);
diff --git
a/core/src/main/java/org/apache/calcite/sql/dialect/HiveSqlDialect.java
b/core/src/main/java/org/apache/calcite/sql/dialect/HiveSqlDialect.java
index 689ecc1ecf..2e2bcfcdf4 100644
--- a/core/src/main/java/org/apache/calcite/sql/dialect/HiveSqlDialect.java
+++ b/core/src/main/java/org/apache/calcite/sql/dialect/HiveSqlDialect.java
@@ -79,6 +79,16 @@ public HiveSqlDialect(Context context) {
return false;
}
+ @Override public void quoteStringLiteral(StringBuilder buf,
+ @Nullable String charsetName, String val) {
+ // Hive treats backslash as an escape character inside string literals
+ // (C-style escapes), so a literal backslash must be doubled before the
+ // base method escapes the enclosing quote. Otherwise a value ending in
+ // a backslash terminates the literal early and the trailing text is
+ // parsed as SQL rather than data.
+ super.quoteStringLiteral(buf, charsetName, escapeBackslash(val));
+ }
+
@Override public void unparseOffsetFetch(SqlWriter writer, @Nullable SqlNode
offset,
@Nullable SqlNode fetch) {
unparseFetchUsingLimit(writer, offset, fetch);
diff --git
a/core/src/main/java/org/apache/calcite/sql/dialect/InfobrightSqlDialect.java
b/core/src/main/java/org/apache/calcite/sql/dialect/InfobrightSqlDialect.java
index 9a0031d830..697bcd44a1 100644
---
a/core/src/main/java/org/apache/calcite/sql/dialect/InfobrightSqlDialect.java
+++
b/core/src/main/java/org/apache/calcite/sql/dialect/InfobrightSqlDialect.java
@@ -18,6 +18,8 @@
import org.apache.calcite.sql.SqlDialect;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
/**
* A <code>SqlDialect</code> implementation for the Infobright database.
*/
@@ -33,6 +35,16 @@ public InfobrightSqlDialect(Context context) {
super(context);
}
+ @Override public void quoteStringLiteral(StringBuilder buf,
+ @Nullable String charsetName, String val) {
+ // Infobright is MySQL-compatible and treats backslash as an escape
+ // character inside string literals, so a literal backslash must be doubled
+ // before the base method escapes the enclosing quote. Otherwise a value
+ // ending in a backslash terminates the literal early and the trailing text
+ // is parsed as SQL rather than data.
+ super.quoteStringLiteral(buf, charsetName, escapeBackslash(val));
+ }
+
@Override public boolean supportsWindowFunctions() {
return false;
}
diff --git
a/core/src/main/java/org/apache/calcite/sql/dialect/NetezzaSqlDialect.java
b/core/src/main/java/org/apache/calcite/sql/dialect/NetezzaSqlDialect.java
index 21378cc344..7d7fba61b2 100644
--- a/core/src/main/java/org/apache/calcite/sql/dialect/NetezzaSqlDialect.java
+++ b/core/src/main/java/org/apache/calcite/sql/dialect/NetezzaSqlDialect.java
@@ -18,6 +18,8 @@
import org.apache.calcite.sql.SqlDialect;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
/**
* A <code>SqlDialect</code> implementation for the Netezza database.
*/
@@ -32,4 +34,15 @@ public class NetezzaSqlDialect extends SqlDialect {
public NetezzaSqlDialect(Context context) {
super(context);
}
+
+ @Override public void quoteStringLiteral(StringBuilder buf,
+ @Nullable String charsetName, String val) {
+ // Netezza accepts C-style backslash escape sequences inside single-quoted
+ // string literals unless the session's standard-conforming-strings option
+ // is enabled, so a literal backslash must be doubled before the base
+ // method escapes the enclosing quote. Otherwise a value ending in a
+ // backslash terminates the literal early and the trailing text is parsed
+ // as SQL rather than data.
+ super.quoteStringLiteral(buf, charsetName, escapeBackslash(val));
+ }
}
diff --git
a/core/src/main/java/org/apache/calcite/sql/dialect/SnowflakeSqlDialect.java
b/core/src/main/java/org/apache/calcite/sql/dialect/SnowflakeSqlDialect.java
index 9deb0836ee..7ecff7915b 100644
--- a/core/src/main/java/org/apache/calcite/sql/dialect/SnowflakeSqlDialect.java
+++ b/core/src/main/java/org/apache/calcite/sql/dialect/SnowflakeSqlDialect.java
@@ -48,6 +48,16 @@ public SnowflakeSqlDialect(Context context) {
super(context);
}
+ @Override public void quoteStringLiteral(StringBuilder buf,
+ @Nullable String charsetName, String val) {
+ // Snowflake recognizes backslash escape sequences inside single-quoted
+ // string constants, so a literal backslash must be doubled before the base
+ // method escapes the enclosing quote. Otherwise a value ending in a
+ // backslash terminates the literal early and the trailing text is parsed
+ // as SQL rather than data.
+ super.quoteStringLiteral(buf, charsetName, escapeBackslash(val));
+ }
+
@Override public void unparseCall(final SqlWriter writer, final SqlCall
call, final int leftPrec,
final int rightPrec) {
switch (call.getKind()) {
diff --git
a/core/src/main/java/org/apache/calcite/sql/dialect/SparkSqlDialect.java
b/core/src/main/java/org/apache/calcite/sql/dialect/SparkSqlDialect.java
index 6f6e3d3917..87432e52d3 100644
--- a/core/src/main/java/org/apache/calcite/sql/dialect/SparkSqlDialect.java
+++ b/core/src/main/java/org/apache/calcite/sql/dialect/SparkSqlDialect.java
@@ -72,6 +72,16 @@ public SparkSqlDialect(SqlDialect.Context context) {
return false;
}
+ @Override public void quoteStringLiteral(StringBuilder buf,
+ @Nullable String charsetName, String val) {
+ // Spark SQL treats backslash as an escape character inside string
+ // literals, so a literal backslash must be doubled before the base
+ // method escapes the enclosing quote. Otherwise a value ending in a
+ // backslash terminates the literal early and the trailing text is
+ // parsed as SQL rather than data.
+ super.quoteStringLiteral(buf, charsetName, escapeBackslash(val));
+ }
+
@Override public JoinType emulateJoinTypeForCrossJoin() {
return JoinType.CROSS;
}
diff --git
a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
index 2953f3b186..8cef620c09 100644
---
a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
+++
b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
@@ -3507,8 +3507,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"
+ "FROM `foodmart`.`reserve_employee`";
sql(query)
.withHive().ok(expected)
@@ -8627,6 +8632,20 @@ private void checkLiteral2(String expression, String
expected) {
sql(bitQuery).withClickHouse().ok(clickHouseExpected);
}
+ /** ClickHouse applies the same backslash escape sequences inside
+ * back-tick-quoted identifiers as inside string literals, so
+ * {@link SqlDialect#quoteIdentifier(StringBuilder, String)} must double it.
*/
+ @Test void testClickHouseQuoteIdentifierWithBackslash() {
+ final SqlDialect dialect = DatabaseProduct.CLICKHOUSE.getDialect();
+ assertThat(dialect.quoteIdentifier("x\\"), is("`x\\\\`"));
+ assertThat(dialect.quoteIdentifier("\\x"), is("`\\\\x`"));
+ assertThat(dialect.quoteIdentifier("x\\y"), is("`x\\\\y`"));
+ assertThat(dialect.quoteIdentifier("x\\\\"), is("`x\\\\\\\\`"));
+ // A back-tick is still escaped as `` after backslashes are doubled
+ assertThat(dialect.quoteIdentifier("x`y"), is("`x``y`"));
+ assertThat(dialect.quoteIdentifier("x\\`y"), is("`x\\\\``y`"));
+ }
+
@Test void testClickHouseArrayFunctions() {
}
@@ -9222,6 +9241,58 @@ private void checkLiteral2(String expression, String
expected) {
});
}
+ /** Dialects whose backend treats {@code \} as an in-string escape
+ * character must double it in
+ * {@link SqlDialect#quoteStringLiteral(StringBuilder, String, String)},
+ * so that a value ending in a backslash cannot terminate the emitted
+ * literal early. */
+ @Test void testDialectQuoteStringLiteralWithBackslash() {
+ dialects().forEach((dialect, databaseProduct) -> {
+ final boolean escapesBackslash =
+ databaseProduct == DatabaseProduct.BIG_QUERY
+ || databaseProduct == DatabaseProduct.MYSQL
+ || databaseProduct == DatabaseProduct.STARROCKS
+ || databaseProduct == DatabaseProduct.DORIS
+ || databaseProduct == DatabaseProduct.HIVE
+ || databaseProduct == DatabaseProduct.SPARK
+ || databaseProduct == DatabaseProduct.SNOWFLAKE
+ || databaseProduct == DatabaseProduct.FIREBOLT
+ || databaseProduct == DatabaseProduct.CLICKHOUSE
+ || databaseProduct == DatabaseProduct.NETEZZA
+ || databaseProduct == DatabaseProduct.INFOBRIGHT;
+
+ // Trailing backslash
+ assertThat(dialect.quoteStringLiteral("x\\"),
+ escapesBackslash ? is("'x\\\\'") : is("'x\\'"));
+
+ // Leading backslash
+ assertThat(dialect.quoteStringLiteral("\\x"),
+ escapesBackslash ? is("'\\\\x'") : is("'\\x'"));
+
+ // Backslash followed by content
+ assertThat(dialect.quoteStringLiteral("x\\y"),
+ escapesBackslash ? is("'x\\\\y'") : is("'x\\y'"));
+
+ // Two consecutive backslashes must both be doubled
+ assertThat(dialect.quoteStringLiteral("x\\\\"),
+ escapesBackslash ? is("'x\\\\\\\\'") : is("'x\\\\'"));
+ });
+ }
+
+ /** BigQuery applies the same backslash escape sequences inside
+ * back-tick-quoted identifiers as inside string literals, so
+ * {@link SqlDialect#quoteIdentifier(StringBuilder, String)} must double it.
*/
+ @Test void testBigQueryQuoteIdentifierWithBackslash() {
+ final SqlDialect dialect = DatabaseProduct.BIG_QUERY.getDialect();
+ assertThat(dialect.quoteIdentifier("x\\"), is("`x\\\\`"));
+ assertThat(dialect.quoteIdentifier("\\x"), is("`\\\\x`"));
+ assertThat(dialect.quoteIdentifier("x\\y"), is("`x\\\\y`"));
+ assertThat(dialect.quoteIdentifier("x\\\\"), is("`x\\\\\\\\`"));
+ // A back-tick is still escaped as \` after backslashes are doubled
+ assertThat(dialect.quoteIdentifier("x`y"), is("`x\\`y`"));
+ assertThat(dialect.quoteIdentifier("x\\`y"), is("`x\\\\\\`y`"));
+ }
+
@Test void testSelectCountStar() {
final String query = "select count(*) from \"product\"";
final String expected = "SELECT COUNT(*)\n"
diff --git a/core/src/test/java/org/apache/calcite/sql/SqlDialectsTest.java
b/core/src/test/java/org/apache/calcite/sql/SqlDialectsTest.java
index d77f02d45c..10e6c14f52 100644
--- a/core/src/test/java/org/apache/calcite/sql/SqlDialectsTest.java
+++ b/core/src/test/java/org/apache/calcite/sql/SqlDialectsTest.java
@@ -17,6 +17,7 @@
package org.apache.calcite.sql;
import org.apache.calcite.jdbc.Driver;
+import org.apache.calcite.sql.dialect.AnsiSqlDialect;
import org.junit.jupiter.api.Test;
@@ -43,4 +44,29 @@ public class SqlDialectsTest {
assertThat(context.databaseMajorVersion(),
is(metaData.getDatabaseMajorVersion()));
}
+
+ /** The fallback dialect for an unrecognized database product doubles
+ * backslashes when quoting string literals: the backend's lexer is
+ * unknown, so a trailing backslash must not be able to consume the
+ * closing quote and shift the string boundary. */
+ @Test void testUnknownProductFallbackEscapesBackslash() {
+ SqlDialect dialect =
+ SqlDialectFactoryImpl.ansiFallback(AnsiSqlDialect.DEFAULT_CONTEXT);
+ // Quote doubling.
+ assertThat(dialect.quoteStringLiteral("can't run"), is("'can''t run'"));
+ // Backslashes are doubled defensively.
+ assertThat(dialect.quoteStringLiteral("x\\"), is("'x\\\\'"));
+ assertThat(dialect.quoteStringLiteral("x\\' extra"),
+ is("'x\\\\'' extra'"));
+ }
+
+ /** An unrecognized JDBC product name (Calcite's own driver has no entry in
+ * {@link SqlDialectFactoryImpl}) resolves to the fallback dialect. */
+ @Test void testFactoryFallbackEscapesBackslash() throws SQLException {
+ Connection connection =
+ DriverManager.getConnection(Driver.CONNECT_STRING_PREFIX);
+ SqlDialect dialect =
+ new SqlDialectFactoryImpl().create(connection.getMetaData());
+ assertThat(dialect.quoteStringLiteral("x\\"), is("'x\\\\'"));
+ }
}
diff --git
a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchProject.java
b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchProject.java
index 640b225434..7dd350f07b 100644
---
a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchProject.java
+++
b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchProject.java
@@ -88,15 +88,14 @@ public class ElasticsearchProject extends Project
implements ElasticsearchRel {
} else if (expr.equals(name)) {
fields.add(name);
} else if (expr.matches("\"literal\":.+")) {
- scriptFields.add(ElasticsearchRules.quote(name)
+ scriptFields.add(quoteName(name)
+ ":{\"script\": "
+ expr.split(":")[1] + "}");
} else {
- scriptFields.add(ElasticsearchRules.quote(name)
+ scriptFields.add(quoteName(name)
+ ":{\"script\":"
// _source (ES2) vs params._source (ES5)
- + "\"" + implementor.elasticsearchTable.scriptedFieldPrefix()
+ "."
- + expr.replace("\"", "") + "\"}");
+ + quoteScript(implementor, expr.replace("\"", "")) + "}");
}
}
@@ -111,7 +110,7 @@ public class ElasticsearchProject extends Project
implements ElasticsearchRel {
List<String> newList = fields.stream()
// _id field is available implicitly
.filter(f -> !ElasticsearchConstants.ID.equals(f))
- .map(ElasticsearchRules::quote)
+ .map(ElasticsearchProject::quoteName)
.collect(Collectors.toList());
final String findString = String.join(", ", newList);
@@ -119,10 +118,9 @@ public class ElasticsearchProject extends Project
implements ElasticsearchRel {
} else {
// if scripted fields are present, ES ignores _source attribute
for (String field : fields) {
- scriptFields.add(ElasticsearchRules.quote(field) + ":{\"script\": "
+ scriptFields.add(quoteName(field) + ":{\"script\": "
// _source (ES2) vs params._source (ES5)
- + "\"" + implementor.elasticsearchTable.scriptedFieldPrefix()
+ "."
- + field + "\"}");
+ + quoteScript(implementor, field) + "}");
}
query.append("\"script_fields\": {")
.append(String.join(", ", scriptFields))
@@ -132,4 +130,24 @@ public class ElasticsearchProject extends Project
implements ElasticsearchRel {
implementor.list.removeIf(l -> l.startsWith("\"_source\""));
implementor.add("{" + query + "}");
}
+
+ /** Renders a name as a JSON string. The value is routed through
+ * {@link ElasticsearchRules#escapeJsonString} so that a name
+ * containing {@code "} or {@code \} stays confined to the surrounding
+ * JSON string. */
+ private static String quoteName(String name) {
+ return ElasticsearchRules.quote(ElasticsearchRules.escapeJsonString(name));
+ }
+
+ /** Renders the scripted-field script that reads {@code path} from the
+ * document source as a JSON string. The path is routed through
+ * {@link ElasticsearchRules#scriptedFieldAccess} so that a field name
+ * containing script punctuation is emitted as a subscript string rather
+ * than concatenated into the script source. */
+ private static String quoteScript(Implementor implementor, String path) {
+ return ElasticsearchRules.quote(
+ ElasticsearchRules.escapeJsonString(
+ ElasticsearchRules.scriptedFieldAccess(
+ implementor.elasticsearchTable.scriptedFieldPrefix(), path)));
+ }
}
diff --git
a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchRules.java
b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchRules.java
index 3c44992b0a..b4c78dc471 100644
---
a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchRules.java
+++
b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchRules.java
@@ -48,6 +48,8 @@
import java.util.AbstractList;
import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
/**
* Rules and relational operators for
@@ -136,10 +138,59 @@ 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
+ * name or alias concatenated between double quotes stays a single JSON
+ * string only if backslash and double quote are doubled. Callers that
+ * embed the value inside a JSON body must route it through this helper
+ * before calling {@link #quote}. */
+ static String escapeJsonString(String s) {
return s.replace("\\", "\\\\").replace("\"", "\\\"");
}
+ /** Shape of field paths that may be embedded in a scripted-field
+ * script using the historical dot notation, for example {@code city},
+ * {@code b.a} or {@code loc[0]}. */
+ private static final Pattern SIMPLE_SCRIPT_PATH =
+ // Use possessive quantifier *+ instead of greedy ones to avoid
backtracking on inputs
+ // that almost, but do not quite, match (avoiding potential catastrophic
backtracking)
+ Pattern.compile("[A-Za-z_][A-Za-z0-9_]*+"
+ + "(?:\\.[A-Za-z_][A-Za-z0-9_]*+)*+(?:\\[[0-9]++\\])*+");
+
+ /** Pattern matching trailing subscripts of a field path, for example {@code
[0]}
+ * in {@code loc[0]}. */
+ private static final Pattern TRAILING_SUBSCRIPTS =
+ // Possessive quantifiers as above
+ Pattern.compile("(?:\\[[0-9]++\\])++$");
+
+ /** Builds the script snippet (Painless/Groovy) that reads {@code path}
+ * from the document source, for example {@code params._source.city}.
+ *
+ * <p>Simple paths keep the historical dot notation. Any other path is
+ * accessed via map subscripts (for example
+ * {@code params._source['a-b']}), with the field name confined to a
+ * single-quoted script string literal (backslashes and quotes escaped),
+ * so that a field name containing script punctuation is always treated
+ * as data instead of being parsed as Painless/Groovy code. */
+ static String scriptedFieldAccess(String prefix, String path) {
+ if (SIMPLE_SCRIPT_PATH.matcher(path).matches()) {
+ return prefix + "." + path;
+ }
+ String name = path;
+ String subscripts = "";
+ final Matcher matcher = TRAILING_SUBSCRIPTS.matcher(path);
+ if (matcher.find()) {
+ name = path.substring(0, matcher.start());
+ subscripts = path.substring(matcher.start());
+ }
+ final StringBuilder builder = new StringBuilder(prefix);
+ for (String step : name.split("\\.", -1)) {
+ builder.append("['")
+ .append(step.replace("\\", "\\\\").replace("'", "\\'"))
+ .append("']");
+ }
+ return builder.append(subscripts).toString();
+ }
+
/**
* Translator from {@link RexNode} to strings in Elasticsearch's expression
* language.
@@ -160,9 +211,9 @@ static class RexToElasticsearchTranslator extends
RexVisitorImpl<String> {
}
return "\"literal\":"
+ quote(
- escapeSpecialSymbols(
- RexToLixTranslator.translateLiteral(literal, literal.getType(),
- typeFactory, RexImpTable.NullAs.NOT_POSSIBLE).toString()));
+ escapeJsonString(
+ RexToLixTranslator.translateLiteral(literal,
literal.getType(),
+ typeFactory,
RexImpTable.NullAs.NOT_POSSIBLE).toString()));
}
@Override public String visitInputRef(RexInputRef inputRef) {
diff --git
a/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchRulesTest.java
b/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchRulesTest.java
new file mode 100644
index 0000000000..d543cb2a89
--- /dev/null
+++
b/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchRulesTest.java
@@ -0,0 +1,139 @@
+/*
+ * 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.elasticsearch;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+/**
+ * Unit tests for the escaping helpers in {@link ElasticsearchRules} that
+ * {@link ElasticsearchProject} uses when field names are concatenated into
+ * the JSON search request.
+ *
+ * <p>Field names and column aliases are arbitrary SQL-controlled strings;
+ * without escaping, a value containing {@code "} or {@code \} would close
+ * the surrounding JSON string and let sibling top-level keys leak into
+ * the request body, or be executed as script code inside a scripted
+ * field. The tests exercise the helpers directly and do not require a
+ * running Elasticsearch cluster.
+ */
+class ElasticsearchRulesTest {
+
+ private final ObjectMapper mapper = new ObjectMapper();
+
+ @Test void quoteWrapsInDoubleQuotes() {
+ // quote() is deliberately raw: callers that embed the value inside a
+ // JSON body route it through escapeSpecialSymbols first.
+ assertThat(ElasticsearchRules.quote("field"), is("\"field\""));
+ assertThat(ElasticsearchRules.quote(""), is("\"\""));
+ }
+
+ @Test void plainNamesUnchangedByEscaping() {
+ assertThat(ElasticsearchRules.escapeJsonString("city"), is("city"));
+ assertThat(ElasticsearchRules.escapeJsonString("b.a"), is("b.a"));
+ }
+
+ @Test void quotesAndBackslashesEscaped() {
+ // Double quote and backslash are the two characters that would let
+ // the value terminate the surrounding JSON string.
+ assertThat(ElasticsearchRules.escapeJsonString("a\"b"), is("a\\\"b"));
+ assertThat(ElasticsearchRules.escapeJsonString("a\\b"), is("a\\\\b"));
+ assertThat(ElasticsearchRules.escapeJsonString("a\\"), is("a\\\\"));
+ // Order matters: escape backslash first, then double quote, so
+ // "a\"b" doesn't become "a\\\"b" with a lost closing quote.
+ assertThat(ElasticsearchRules.escapeJsonString("a\\\"b"),
+ is("a\\\\\\\"b"));
+ }
+
+ @Test void namesWithJsonMetacharactersStayASingleSourceField() throws
Exception {
+ // A name that tries to close the "_source" array and add a sibling
+ // "query" key must stay confined to the string, so the emitted JSON
+ // remains a single top-level object with one member.
+ final String awkward =
+ "a\"], \"query\": {\"bool\": {\"filter\": {\"script\": "
+ + "{\"script\": \"while(true){}\"}}}}, \"docvalue_fields\": [\"b";
+ final String json = "{\"_source\" : ["
+ + ElasticsearchRules.quote(
+ ElasticsearchRules.escapeJsonString(awkward)) + "]}";
+ final JsonNode node = mapper.readTree(json);
+ final int rootMembers = node.size();
+ final int sourceEntries = node.get("_source").size();
+ assertThat(rootMembers, is(1));
+ assertThat(sourceEntries, is(1));
+ assertThat(node.get("_source").get(0).asText(), is(awkward));
+ }
+
+ @Test void simplePathsKeepDotNotation() {
+ // Historical dot notation stays in place for identifier-shaped paths
+ // so existing scripted fields render exactly as before.
+ assertThat(ElasticsearchRules.scriptedFieldAccess("params._source",
"city"),
+ is("params._source.city"));
+ assertThat(ElasticsearchRules.scriptedFieldAccess("params._source", "b.a"),
+ is("params._source.b.a"));
+ assertThat(ElasticsearchRules.scriptedFieldAccess("params._source",
"loc[0]"),
+ is("params._source.loc[0]"));
+ }
+
+ @Test void nonIdentifierPathsBecomeMapSubscripts() {
+ // Any path that does not fit the historical dot notation is rendered
+ // via map subscripts so the segments stay data.
+ assertThat(ElasticsearchRules.scriptedFieldAccess("params._source",
"@timestamp"),
+ is("params._source['@timestamp']"));
+ assertThat(ElasticsearchRules.scriptedFieldAccess("params._source", "we
ird[3]"),
+ is("params._source['we ird'][3]"));
+ assertThat(ElasticsearchRules.scriptedFieldAccess("params._source", "a.b
c"),
+ is("params._source['a']['b c']"));
+ }
+
+ @Test void fieldNameWithScriptPunctuationIsSubscripted() {
+ // A field name that contains {@code '} or {@code ;} would otherwise
+ // be spliced into script source and parsed as Painless/Groovy code;
+ // the map-subscript form quotes those characters so the whole name
+ // stays a script string literal.
+ assertThat(
+ ElasticsearchRules.scriptedFieldAccess("params._source",
+ "a'; long x=0; for(int i=0;i<10;i++){x+=i}; '"),
+ is("params._source['a\\'; long x=0; for(int i=0;i<10;i++){x+=i};
\\'']"));
+ }
+
+ @Test void scriptFieldEntryStaysWellFormedJsonForAwkwardName() throws
Exception {
+ // End-to-end: an entry built the way ElasticsearchProject builds one
+ // must still parse as a single-member JSON object with the value
+ // recovered exactly.
+ final String awkward = "a\"}}, \"other\": {\"script\": \"boom";
+ final String entry =
+ ElasticsearchRules.quote(ElasticsearchRules.escapeJsonString(awkward))
+ + ":{\"script\": "
+ + ElasticsearchRules.quote(
+ ElasticsearchRules.escapeJsonString(
+ ElasticsearchRules.scriptedFieldAccess("params._source",
+ awkward)))
+ + "}";
+ final JsonNode node = mapper.readTree("{\"script_fields\": {" + entry +
"}}");
+ final int rootMembers = node.size();
+ final int scriptFieldsMembers = node.get("script_fields").size();
+ assertThat(rootMembers, is(1));
+ assertThat(scriptFieldsMembers, is(1));
+ assertThat(node.get("script_fields").get(awkward).get("script").asText(),
+ is("params._source['a\"}}, \"other\": {\"script\": \"boom']"));
+ }
+}
diff --git
a/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeAggregate.java
b/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeAggregate.java
index baf850e0f7..af21090a7d 100644
---
a/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeAggregate.java
+++
b/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeAggregate.java
@@ -16,6 +16,7 @@
*/
package org.apache.calcite.adapter.geode.rel;
+import org.apache.calcite.adapter.geode.util.GeodeUtils;
import org.apache.calcite.plan.RelOptCluster;
import org.apache.calcite.plan.RelOptCost;
import org.apache.calcite.plan.RelOptPlanner;
@@ -100,7 +101,10 @@ public GeodeAggregate(RelOptCluster cluster,
List<String> groupByFields = new ArrayList<>();
for (int group : groupSet) {
- groupByFields.add(inputFields.get(group));
+ // OQL provides no way to quote identifiers, so the group key must
+ // match a strict allowlist before it can be copied verbatim into the
+ // generated statement.
+
groupByFields.add(GeodeUtils.checkSafeOqlIdentifier(inputFields.get(group)));
}
geodeImplementContext.addGroupBy(groupByFields);
@@ -111,7 +115,7 @@ public GeodeAggregate(RelOptCluster cluster,
List<String> aggCallFieldNames = new ArrayList<>();
for (int i : aggCall.getArgList()) {
- aggCallFieldNames.add(inputFields.get(i));
+
aggCallFieldNames.add(GeodeUtils.checkSafeOqlIdentifier(inputFields.get(i)));
}
String functionName = aggCall.getAggregation().getName();
@@ -119,13 +123,14 @@ public GeodeAggregate(RelOptCluster cluster,
// 'count(*)' but allows it for count('any column name'). So we are
// converting the count(*) into count (first input ColumnName).
if ("COUNT".equalsIgnoreCase(functionName) &&
aggCallFieldNames.isEmpty()) {
- aggCallFieldNames.add(inputFields.get(0));
+
aggCallFieldNames.add(GeodeUtils.checkSafeOqlIdentifier(inputFields.get(0)));
}
String oqlAggregateCall =
Util.toString(aggCallFieldNames, functionName + "(", ", ", ")");
- aggregateFunctionMap.put(aggCall.getName(), oqlAggregateCall);
+
aggregateFunctionMap.put(GeodeUtils.checkSafeOqlIdentifier(aggCall.getName()),
+ oqlAggregateCall);
}
geodeImplementContext.addAggregateFunctions(aggregateFunctionMap.build());
diff --git
a/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeProject.java
b/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeProject.java
index 9cdcfa3eaf..b11a2e1e3b 100644
--- a/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeProject.java
+++ b/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeProject.java
@@ -17,6 +17,7 @@
package org.apache.calcite.adapter.geode.rel;
import org.apache.calcite.adapter.geode.rel.GeodeRules.RexToGeodeTranslator;
+import org.apache.calcite.adapter.geode.util.GeodeUtils;
import org.apache.calcite.plan.RelOptCluster;
import org.apache.calcite.plan.RelOptCost;
import org.apache.calcite.plan.RelOptPlanner;
@@ -72,7 +73,10 @@ public class GeodeProject extends Project implements
GeodeRel {
GeodeRules.geodeFieldNames(getInput().getRowType()));
final Map<String, String> fields = new LinkedHashMap<>();
for (Pair<RexNode, String> pair : getNamedProjects()) {
- final String name = pair.right;
+ // OQL provides no way to quote identifiers, so the projection alias
+ // must match a strict allowlist before it can be copied verbatim into
+ // the generated statement.
+ final String name = GeodeUtils.checkSafeOqlIdentifier(pair.right);
final String originalName = pair.left.accept(translator);
fields.put(originalName, name);
}
diff --git
a/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeRules.java
b/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeRules.java
index b8beb854e0..e6a8491590 100644
--- a/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeRules.java
+++ b/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeRules.java
@@ -39,6 +39,7 @@
import org.apache.calcite.sql.fun.SqlStdOperatorTable;
import org.apache.calcite.sql.type.SqlTypeName;
import org.apache.calcite.sql.validate.SqlValidatorUtil;
+import org.apache.calcite.util.Util;
import com.google.common.collect.ImmutableList;
@@ -47,6 +48,7 @@
import java.util.ArrayList;
import java.util.List;
+import java.util.regex.Pattern;
import static com.google.common.base.Preconditions.checkArgument;
@@ -90,6 +92,44 @@ static List<String> geodeFieldNames(final RelDataType
rowType) {
return SqlValidatorUtil.uniquify(rowType.getFieldNames(), true);
}
+ /** Shape of ITEM string keys that may be embedded in a {@code path.key}
+ * OQL expression. OQL has no way to quote identifiers, so any other key
+ * (quotes, spaces, operators, parentheses) could change the structure of
+ * the statement sent to Geode and must never be pushed down. */
+ private static final Pattern OQL_ITEM_KEY =
+ Pattern.compile("[A-Za-z_$][A-Za-z0-9_$]*");
+
+ /** Returns whether an ITEM string key may be embedded in OQL text. */
+ static boolean isSafeItemKey(String key) {
+ return OQL_ITEM_KEY.matcher(key).matches();
+ }
+
+ /** Returns whether every ITEM call (if any) inside {@code node} uses a
+ * string key that is safe to embed in OQL text. Push-down rules use this
+ * to keep expressions with unsafe keys on the Calcite side, where they
+ * are evaluated after the scan instead of being concatenated into the
+ * OQL statement. */
+ static boolean hasOnlySafeItemKeys(RexNode node) {
+ try {
+ node.accept(new RexVisitorImpl<Void>(true) {
+ @Override public Void visitCall(RexCall call) {
+ if (call.getOperator() == SqlStdOperatorTable.ITEM) {
+ final RexNode op1 = call.getOperands().get(1);
+ if (op1 instanceof RexLiteral
+ && ((RexLiteral) op1).getValue2() instanceof String
+ && !isSafeItemKey((String) ((RexLiteral) op1).getValue2())) {
+ throw Util.FoundOne.NULL;
+ }
+ }
+ return super.visitCall(call);
+ }
+ });
+ return true;
+ } catch (Util.FoundOne e) {
+ return false;
+ }
+ }
+
/**
* Translator from {@link RexNode} to strings in Geode's expression language.
*/
@@ -115,7 +155,16 @@ protected RexToGeodeTranslator(List<String> inFields) {
if (op1.getType().getSqlTypeName() == SqlTypeName.INTEGER) {
return stripQuotes(strings.get(0)) + "[" + ((RexLiteral)
op1).getValue2() + "]";
} else if (op1.getType().getSqlTypeName() == SqlTypeName.CHAR) {
- return stripQuotes(strings.get(0)) + "." + ((RexLiteral)
op1).getValue2();
+ final String key = String.valueOf(((RexLiteral) op1).getValue2());
+ // The key becomes part of the OQL text, so restrict it to a
+ // plain identifier; anything else could alter the structure of
+ // the statement. The push-down rules refuse to match such
+ // expressions, so this check is defense-in-depth.
+ if (!isSafeItemKey(key)) {
+ throw new IllegalArgumentException("Geode adapter cannot push "
+ + "ITEM key '" + key + "' into OQL: not a plain identifier");
+ }
+ return stripQuotes(strings.get(0)) + "." + key;
}
}
}
@@ -149,6 +198,12 @@ protected GeodeProjectRule(Config config) {
// For spatial Functions Drop to Calcite Enumerable
return false;
}
+ if (!hasOnlySafeItemKeys(e)) {
+ // An ITEM key inside this projection would be concatenated into
+ // the OQL SELECT list; keep the projection on the Calcite side,
+ // where it is evaluated after the scan.
+ return false;
+ }
}
return project.getVariablesSet().isEmpty();
}
@@ -268,6 +323,13 @@ protected GeodeFilterRule(GeodeFilterRuleConfig config) {
LogicalFilter filter = call.rel(0);
RexNode condition = filter.getCondition();
+ if (!hasOnlySafeItemKeys(condition)) {
+ // An ITEM key inside the condition would be concatenated into the
+ // OQL WHERE clause; keep the filter on the Calcite side, where it
+ // is evaluated after the scan.
+ return false;
+ }
+
List<String> fieldNames =
GeodeRules.geodeFieldNames(filter.getInput().getRowType());
List<RexNode> disjunctions = RelOptUtil.disjunctions(condition);
diff --git
a/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeSort.java
b/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeSort.java
index c9b33252d9..41d850a521 100644
--- a/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeSort.java
+++ b/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeSort.java
@@ -16,6 +16,7 @@
*/
package org.apache.calcite.adapter.geode.rel;
+import org.apache.calcite.adapter.geode.util.GeodeUtils;
import org.apache.calcite.plan.RelOptCluster;
import org.apache.calcite.plan.RelOptCost;
import org.apache.calcite.plan.RelOptPlanner;
@@ -79,7 +80,11 @@ public class GeodeSort extends Sort implements GeodeRel {
List<String> orderByFields = new ArrayList<>();
for (RelFieldCollation fieldCollation : sortCollations) {
- final String name = fieldName(fieldCollation.getFieldIndex());
+ // OQL provides no way to quote identifiers, so the ORDER BY field
+ // name must match a strict allowlist before it can be copied
+ // verbatim into the generated statement.
+ final String name =
+
GeodeUtils.checkSafeOqlIdentifier(fieldName(fieldCollation.getFieldIndex()));
orderByFields.add(name + " " +
direction(fieldCollation.getDirection()));
}
geodeImplementContext.addOrderByFields(orderByFields);
diff --git
a/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeTable.java
b/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeTable.java
index 584b5cacae..20222c496c 100644
--- a/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeTable.java
+++ b/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeTable.java
@@ -53,6 +53,7 @@
import java.util.List;
import java.util.Locale;
import java.util.Map;
+import java.util.regex.Pattern;
import static java.util.Objects.requireNonNull;
@@ -63,6 +64,30 @@ public class GeodeTable extends AbstractQueryableTable
implements TranslatableTa
private static final Logger LOGGER =
LoggerFactory.getLogger(GeodeTable.class.getName());
+ /** An OQL identifier: the only shape accepted for a SQL-side alias that
+ * becomes part of the OQL statement text. */
+ private static final Pattern OQL_IDENTIFIER =
+ Pattern.compile("[A-Za-z_$][A-Za-z0-9_$]*");
+
+ /** A field path as produced by {@link GeodeRules.RexToGeodeTranslator},
+ * for example {@code primaryAddress.postalCode} or {@code loc[0]}.
+ * Possessive quantifiers avoid backtracking on inputs that almost, but
+ * do not quite, match. */
+ private static final Pattern OQL_FIELD_PATH =
+ Pattern.compile("[A-Za-z_$][A-Za-z0-9_$]*+"
+ + "(?:\\.[A-Za-z_$][A-Za-z0-9_$]*+|\\[[0-9]++\\])*+");
+
+ /** An aggregate call as produced by {@link GeodeAggregate},
+ * for example {@code SUM(pop)}. */
+ private static final Pattern OQL_AGGREGATE_CALL =
+ Pattern.compile("[A-Za-z_$][A-Za-z0-9_$]*\\((?:" +
OQL_FIELD_PATH.pattern()
+ + "(?:, " + OQL_FIELD_PATH.pattern() + ")*)?\\)");
+
+ /** An ORDER BY entry as produced by {@link GeodeSort},
+ * for example {@code state ASC}. */
+ private static final Pattern OQL_ORDER_BY_ENTRY =
+ Pattern.compile(OQL_FIELD_PATH.pattern() + " (?:ASC|DESC)");
+
private final String regionName;
private final RelDataType rowType;
@@ -76,6 +101,51 @@ public class GeodeTable extends AbstractQueryableTable
implements TranslatableTa
return "GeodeTable {" + regionName + "}";
}
+ /** Checks that a string that is about to be concatenated into the OQL
+ * statement matches the expected shape, and returns it.
+ *
+ * <p>SQL quoted identifiers may contain arbitrary characters, so an
+ * unvalidated alias would let arbitrary OQL text (for example a nested
+ * query on a region that is not exposed by the schema, or a method
+ * invocation on a server object) be concatenated into the emitted
+ * statement. OQL has no identifier quoting mechanism, therefore values
+ * that do not look like a plain identifier/path are rejected rather than
+ * escaped.
+ *
+ * <p>The value must be non-null; a null argument is a caller bug and
+ * surfaces as {@link NullPointerException}.
+ *
+ * @throws IllegalArgumentException if the value does not match
+ */
+ private static String checkOql(Pattern pattern, String kind, String value) {
+ requireNonNull(value, kind);
+ if (!pattern.matcher(value).matches()) {
+ throw new IllegalArgumentException("Cannot use " + kind + " '" + value
+ + "' in a Geode OQL query; only plain identifiers are supported");
+ }
+ return value;
+ }
+
+ /** Validates an alias (output name) used in the OQL select list. */
+ static String checkOqlIdentifier(String alias) {
+ return checkOql(OQL_IDENTIFIER, "alias", alias);
+ }
+
+ /** Validates a field path used in the OQL select or GROUP BY list. */
+ static String checkOqlFieldPath(String field) {
+ return checkOql(OQL_FIELD_PATH, "field", field);
+ }
+
+ /** Validates an aggregate call used in the OQL select list. */
+ static String checkOqlAggregateCall(String aggregateCall) {
+ return checkOql(OQL_AGGREGATE_CALL, "aggregate call", aggregateCall);
+ }
+
+ /** Validates an ORDER BY entry ({@code field ASC|DESC}). */
+ static String checkOqlOrderByEntry(String entry) {
+ return checkOql(OQL_ORDER_BY_ENTRY, "ORDER BY field", entry);
+ }
+
/**
* Executes an OQL query on the underlying table.
*
@@ -130,24 +200,30 @@ public Enumerable<Object> query(final GemFireCache
clientCache,
aggFuncMap = aggFuncMapBuilder.build();
}
- // Construct the list of fields to project
+ // Construct the list of fields to project. OQL provides no way to quote
+ // identifiers, so every field name, alias, and GROUP BY key that is copied
+ // verbatim into the generated statement is validated against a strict
+ // allowlist; anything else refuses pushdown.
ImmutableList.Builder<String> selectBuilder = ImmutableList.builder();
if (!groupByFields.isEmpty()) {
// manually add GROUP BY to select clause (GeodeProjection was not
visited)
for (String groupByField : groupByFields) {
+ checkOqlFieldPath(groupByField);
selectBuilder.add(groupByField + " AS " + groupByField);
}
if (!aggFuncMap.isEmpty()) {
for (Map.Entry<String, String> e : aggFuncMap.entrySet()) {
- selectBuilder.add(e.getValue() + " AS " + e.getKey());
+ selectBuilder.add(checkOqlAggregateCall(e.getValue())
+ + " AS " + checkOqlIdentifier(e.getKey()));
}
}
} else {
if (selectFields.isEmpty()) {
if (!aggFuncMap.isEmpty()) {
for (Map.Entry<String, String> e : aggFuncMap.entrySet()) {
- selectBuilder.add(e.getValue() + " AS " + e.getKey());
+ selectBuilder.add(checkOqlAggregateCall(e.getValue())
+ + " AS " + checkOqlIdentifier(e.getKey()));
}
} else {
selectBuilder.add("*");
@@ -155,11 +231,13 @@ public Enumerable<Object> query(final GemFireCache
clientCache,
} else {
if (!aggFuncMap.isEmpty()) {
for (Map.Entry<String, String> e : aggFuncMap.entrySet()) {
- selectBuilder.add(e.getValue() + " AS " + e.getKey());
+ selectBuilder.add(checkOqlAggregateCall(e.getValue())
+ + " AS " + checkOqlIdentifier(e.getKey()));
}
} else {
for (Map.Entry<String, String> field : selectFields) {
- selectBuilder.add(field.getKey() + " AS " + field.getValue());
+ selectBuilder.add(checkOqlFieldPath(field.getKey())
+ + " AS " + checkOqlIdentifier(field.getValue()));
}
}
}
@@ -186,6 +264,9 @@ public Enumerable<Object> query(final GemFireCache
clientCache,
}
if (!orderByFields.isEmpty()) {
+ for (String orderByField : orderByFields) {
+ checkOqlOrderByEntry(orderByField);
+ }
queryBuilder.append(Util.toString(orderByFields, " ORDER BY ", ", ",
""));
}
if (limit != null) {
diff --git
a/geode/src/main/java/org/apache/calcite/adapter/geode/util/GeodeUtils.java
b/geode/src/main/java/org/apache/calcite/adapter/geode/util/GeodeUtils.java
index 3cc43ca4b2..1464330348 100644
--- a/geode/src/main/java/org/apache/calcite/adapter/geode/util/GeodeUtils.java
+++ b/geode/src/main/java/org/apache/calcite/adapter/geode/util/GeodeUtils.java
@@ -46,6 +46,7 @@
import java.util.Locale;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.regex.Pattern;
import static java.util.Objects.requireNonNull;
@@ -338,6 +339,37 @@ public static RelDataType
autodetectRelTypeFromRegion(Region<?, ?> region) {
return createRelDataType(entry);
}
+ /**
+ * OQL has no way to escape identifiers, so an identifier that is copied
+ * verbatim into the generated query must be a simple alphanumeric name (with
+ * optional dotted path or bracketed integer subscript for nested access).
+ * Anything else could shift the boundaries of the emitted statement.
+ */
+ private static final Pattern SAFE_OQL_IDENTIFIER =
+ Pattern.compile(
+
"[A-Za-z_$][A-Za-z0-9_$]*+(?:\\.[A-Za-z_$][A-Za-z0-9_$]*+|\\[[0-9]++\\])*+");
+
+ /**
+ * Returns whether {@code name} may be emitted verbatim as an OQL identifier
+ * (a field name, alias, or dotted path). Since OQL provides no way to quote
+ * an identifier, callers should refuse to push down when this returns false.
+ */
+ public static boolean isSafeOqlIdentifier(String name) {
+ return name != null && SAFE_OQL_IDENTIFIER.matcher(name).matches();
+ }
+
+ /**
+ * Validates that {@code name} may be emitted verbatim into a generated OQL
+ * statement, throwing a {@link RuntimeException} otherwise.
+ */
+ public static String checkSafeOqlIdentifier(String name) {
+ if (!isSafeOqlIdentifier(name)) {
+ throw new RuntimeException(
+ "Cannot push down expression: not a simple OQL identifier: " + name);
+ }
+ return name;
+ }
+
// Create Relational Type by inferring a Geode entry or response instance.
private static RelDataType createRelDataType(Object regionEntry) {
JavaTypeFactoryExtImpl typeFactory = new JavaTypeFactoryExtImpl();
diff --git
a/geode/src/test/java/org/apache/calcite/adapter/geode/rel/GeodeRulesTest.java
b/geode/src/test/java/org/apache/calcite/adapter/geode/rel/GeodeRulesTest.java
new file mode 100644
index 0000000000..2164947913
--- /dev/null
+++
b/geode/src/test/java/org/apache/calcite/adapter/geode/rel/GeodeRulesTest.java
@@ -0,0 +1,137 @@
+/*
+ * 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.geode.rel;
+
+import org.apache.calcite.jdbc.JavaTypeFactoryImpl;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexInputRef;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.sql.type.SqlTypeName;
+
+import org.junit.jupiter.api.Test;
+
+import java.lang.reflect.Constructor;
+import java.util.Collections;
+import java.util.List;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+/**
+ * Unit tests for {@link GeodeRules} covering the ITEM-key plan-time
+ * short-circuit and the dotted-path emission for {@code ITEM(field, 'key')}
+ * calls.
+ *
+ * <p>OQL provides no way to quote identifiers, so a CHAR literal that
+ * becomes part of a dotted field path must match a strict allowlist before
+ * it can be copied verbatim into the generated statement; any other key
+ * keeps the expression on the Calcite side.
+ */
+class GeodeRulesTest {
+
+ private static final RelDataTypeFactory TYPE_FACTORY = new
JavaTypeFactoryImpl();
+ private static final RexBuilder REX_BUILDER = new RexBuilder(TYPE_FACTORY);
+
+ /** Builds a translator over a single-column ANY-typed row so ITEM
+ * calls type-check. */
+ private static GeodeRules.RexToGeodeTranslator translator(List<String>
fields)
+ throws Exception {
+ Constructor<GeodeRules.RexToGeodeTranslator> ctor =
+ GeodeRules.RexToGeodeTranslator.class
+ .getDeclaredConstructor(List.class);
+ ctor.setAccessible(true);
+ return ctor.newInstance(fields);
+ }
+
+ private static RexNode itemCall(String key) {
+ RelDataType any = TYPE_FACTORY.createSqlType(SqlTypeName.ANY);
+ RexInputRef ref = REX_BUILDER.makeInputRef(any, 0);
+ RexNode lit = REX_BUILDER.makeLiteral(key);
+ return REX_BUILDER.makeCall(SqlStdOperatorTable.ITEM, ref, lit);
+ }
+
+ @Test void plainKeyAccepted() throws Exception {
+ GeodeRules.RexToGeodeTranslator t =
+ translator(Collections.singletonList("root"));
+ assertThat(itemCall("child").accept(t), is("root.child"));
+ }
+
+ @Test void syntheticExprKeyAccepted() throws Exception {
+ GeodeRules.RexToGeodeTranslator t =
+ translator(Collections.singletonList("root"));
+ assertThat(itemCall("EXPR$0").accept(t), is("root.EXPR$0"));
+ }
+
+ @Test void keyWithSpaceRejected() throws Exception {
+ GeodeRules.RexToGeodeTranslator t =
+ translator(Collections.singletonList("root"));
+ IllegalArgumentException e =
+ assertThrows(IllegalArgumentException.class,
+ () -> itemCall("a b").accept(t));
+ assertThat(e.getMessage(), containsString("not a plain identifier"));
+ }
+
+ @Test void keyWithSingleQuoteRejected() throws Exception {
+ // An apostrophe in the key would terminate the surrounding OQL string
+ // token and shift the boundaries of the emitted statement.
+ GeodeRules.RexToGeodeTranslator t =
+ translator(Collections.singletonList("root"));
+ assertThrows(IllegalArgumentException.class,
+ () -> itemCall("a'b").accept(t));
+ }
+
+ @Test void keyWithPunctuationRejected() throws Exception {
+ GeodeRules.RexToGeodeTranslator t =
+ translator(Collections.singletonList("root"));
+ assertThrows(IllegalArgumentException.class,
+ () -> itemCall("a,b").accept(t));
+ assertThrows(IllegalArgumentException.class,
+ () -> itemCall("a(b)").accept(t));
+ assertThrows(IllegalArgumentException.class,
+ () -> itemCall("a.b").accept(t));
+ }
+
+ @Test void safeItemKeyPredicate() {
+ assertThat(GeodeRules.isSafeItemKey("postalCode"), is(true));
+ assertThat(GeodeRules.isSafeItemKey("_key$1"), is(true));
+ assertThat(GeodeRules.isSafeItemKey("EXPR$0"), is(true));
+ assertThat(GeodeRules.isSafeItemKey(""), is(false));
+ assertThat(GeodeRules.isSafeItemKey("a b"), is(false));
+ assertThat(GeodeRules.isSafeItemKey("a.b"), is(false));
+ assertThat(GeodeRules.isSafeItemKey("e.getClass()"), is(false));
+ }
+
+ @Test void hasOnlySafeItemKeysInspectsWholeTree() {
+ // A plain expression walks through as safe.
+ RexNode safe =
+ REX_BUILDER.makeCall(SqlStdOperatorTable.EQUALS, itemCall("k"),
+ REX_BUILDER.makeLiteral("v"));
+ assertThat(GeodeRules.hasOnlySafeItemKeys(safe), is(true));
+
+ // An unsafe key anywhere in the tree flips the answer.
+ RexNode unsafe =
+ REX_BUILDER.makeCall(SqlStdOperatorTable.EQUALS,
+ itemCall("k' OR '1' = '1"),
+ REX_BUILDER.makeLiteral("v"));
+ assertThat(GeodeRules.hasOnlySafeItemKeys(unsafe), is(false));
+ }
+}
diff --git
a/geode/src/test/java/org/apache/calcite/adapter/geode/rel/GeodeTableOqlValidationTest.java
b/geode/src/test/java/org/apache/calcite/adapter/geode/rel/GeodeTableOqlValidationTest.java
new file mode 100644
index 0000000000..aba600f95b
--- /dev/null
+++
b/geode/src/test/java/org/apache/calcite/adapter/geode/rel/GeodeTableOqlValidationTest.java
@@ -0,0 +1,116 @@
+/*
+ * 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.geode.rel;
+
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+/**
+ * Unit tests for the identifier validation {@link GeodeTable} applies to
+ * every SQL-controlled name (aliases, field paths, aggregate calls, ORDER BY
+ * entries) before concatenating it into an OQL statement.
+ *
+ * <p>SQL quoted identifiers may contain arbitrary characters, and OQL has
+ * no identifier quoting mechanism, so anything that is not a plain
+ * identifier/path must be rejected: otherwise a SQL-supplied alias could
+ * shift the structure of the emitted OQL (for example a nested query on a
+ * region that is not exposed by the schema). The tests do not require a
+ * running Geode cluster.
+ */
+class GeodeTableOqlValidationTest {
+
+ @Test void plainAliasesAccepted() {
+ assertThat(GeodeTable.checkOqlIdentifier("state"), is("state"));
+ assertThat(GeodeTable.checkOqlIdentifier("_id"), is("_id"));
+ // Calcite-generated aliases contain '$'
+ assertThat(GeodeTable.checkOqlIdentifier("EXPR$0"), is("EXPR$0"));
+ }
+
+ @Test void aliasWithEmbeddedOqlRejected() {
+ // OQL supports method invocation: without validation a SQL-supplied
+ // alias could call methods on server-side objects.
+ IllegalArgumentException e =
+ assertThrows(IllegalArgumentException.class, () ->
+ GeodeTable.checkOqlIdentifier(
+ "f, e.getClass().getClassLoader() AS leak"));
+ assertThat(e.getMessage(), containsString("only plain identifiers"));
+ }
+
+ @Test void aliasWithNestedQueryRejected() {
+ assertThrows(IllegalArgumentException.class, () ->
+ GeodeTable.checkOqlIdentifier(
+ "x FROM (SELECT s.ssn FROM /SecretRegion s)"));
+ }
+
+ @Test void aliasWithWhitespaceOrEmptyRejected() {
+ assertThrows(IllegalArgumentException.class, () ->
+ GeodeTable.checkOqlIdentifier("a b"));
+ assertThrows(IllegalArgumentException.class, () ->
+ GeodeTable.checkOqlIdentifier(""));
+ // Null is a caller bug and surfaces as NullPointerException, not as a
shape violation
+ assertThrows(NullPointerException.class, () ->
+ GeodeTable.checkOqlIdentifier(null));
+ }
+
+ @Test void fieldPathsAccepted() {
+ assertThat(GeodeTable.checkOqlFieldPath("pop"), is("pop"));
+ assertThat(GeodeTable.checkOqlFieldPath("primaryAddress.postalCode"),
+ is("primaryAddress.postalCode"));
+ assertThat(GeodeTable.checkOqlFieldPath("loc[0]"), is("loc[0]"));
+ }
+
+ @Test void fieldPathWithEmbeddedPredicateRejected() {
+ assertThrows(IllegalArgumentException.class, () ->
+ GeodeTable.checkOqlFieldPath(
+ "pop AND (SELECT COUNT(*) FROM /SecretRegion) > 0"));
+ assertThrows(IllegalArgumentException.class, () ->
+ GeodeTable.checkOqlFieldPath("loc['0']"));
+ }
+
+ @Test void aggregateCallsAccepted() {
+ assertThat(GeodeTable.checkOqlAggregateCall("SUM(pop)"), is("SUM(pop)"));
+ assertThat(GeodeTable.checkOqlAggregateCall("COUNT(itemNumber)"),
+ is("COUNT(itemNumber)"));
+ assertThat(GeodeTable.checkOqlAggregateCall("AVG(loc[1])"),
+ is("AVG(loc[1])"));
+ }
+
+ @Test void aggregateCallWithExtraContentRejected() {
+ assertThrows(IllegalArgumentException.class, () ->
+ GeodeTable.checkOqlAggregateCall("SUM(pop), e.getClass()"));
+ assertThrows(IllegalArgumentException.class, () ->
+ GeodeTable.checkOqlAggregateCall("SUM(pop) FROM /Other"));
+ }
+
+ @Test void orderByEntriesAccepted() {
+ assertThat(GeodeTable.checkOqlOrderByEntry("state ASC"), is("state ASC"));
+ assertThat(GeodeTable.checkOqlOrderByEntry("loc[0] DESC"),
+ is("loc[0] DESC"));
+ }
+
+ @Test void orderByEntryWithExtraContentRejected() {
+ assertThrows(IllegalArgumentException.class, () ->
+ GeodeTable.checkOqlOrderByEntry(
+ "state ASC LIMIT 1 -- extra DESC"));
+ assertThrows(IllegalArgumentException.class, () ->
+ GeodeTable.checkOqlOrderByEntry("state"));
+ }
+}
diff --git
a/geode/src/test/java/org/apache/calcite/adapter/geode/util/GeodeUtilsTest.java
b/geode/src/test/java/org/apache/calcite/adapter/geode/util/GeodeUtilsTest.java
new file mode 100644
index 0000000000..3fb844ed39
--- /dev/null
+++
b/geode/src/test/java/org/apache/calcite/adapter/geode/util/GeodeUtilsTest.java
@@ -0,0 +1,128 @@
+/*
+ * 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.geode.util;
+
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+/**
+ * Unit tests for the OQL identifier allowlist in {@link GeodeUtils}.
+ *
+ * <p>OQL provides no way to quote an identifier, so a name that is
+ * copied verbatim into a generated query must match a strict
+ * allowlist; otherwise it could shift the boundaries of the emitted
+ * statement.
+ */
+class GeodeUtilsTest {
+
+ @Test void simpleNameAccepted() {
+ assertThat(GeodeUtils.isSafeOqlIdentifier("field"), is(true));
+ assertThat(GeodeUtils.isSafeOqlIdentifier("_field"), is(true));
+ assertThat(GeodeUtils.isSafeOqlIdentifier("$field"), is(true));
+ assertThat(GeodeUtils.isSafeOqlIdentifier("Field1"), is(true));
+ }
+
+ @Test void syntheticExprNameAccepted() {
+ // Calcite emits synthetic aggregate column names like EXPR$0; the
+ // pattern must accept them so a pushed-down aggregate is not
+ // rejected on legitimate input.
+ assertThat(GeodeUtils.isSafeOqlIdentifier("EXPR$0"), is(true));
+ assertThat(GeodeUtils.isSafeOqlIdentifier("EXPR$10"), is(true));
+ }
+
+ @Test void dottedPathAccepted() {
+ assertThat(GeodeUtils.isSafeOqlIdentifier("a.b"), is(true));
+ assertThat(GeodeUtils.isSafeOqlIdentifier("a.b.c"), is(true));
+ assertThat(GeodeUtils.isSafeOqlIdentifier("a.$b"), is(true));
+ }
+
+ @Test void bracketedIndexAccepted() {
+ assertThat(GeodeUtils.isSafeOqlIdentifier("a[0]"), is(true));
+ assertThat(GeodeUtils.isSafeOqlIdentifier("a[123]"), is(true));
+ assertThat(GeodeUtils.isSafeOqlIdentifier("a.b[7]"), is(true));
+ // A subscript may be followed by a further dotted field.
+ assertThat(GeodeUtils.isSafeOqlIdentifier("a[0].c"), is(true));
+ }
+
+ @Test void nullRejected() {
+ assertThat(GeodeUtils.isSafeOqlIdentifier(null), is(false));
+ }
+
+ @Test void emptyRejected() {
+ assertThat(GeodeUtils.isSafeOqlIdentifier(""), is(false));
+ }
+
+ @Test void leadingDigitRejected() {
+ assertThat(GeodeUtils.isSafeOqlIdentifier("1field"), is(false));
+ }
+
+ @Test void whitespaceRejected() {
+ assertThat(GeodeUtils.isSafeOqlIdentifier("a b"), is(false));
+ assertThat(GeodeUtils.isSafeOqlIdentifier("a\tb"), is(false));
+ assertThat(GeodeUtils.isSafeOqlIdentifier("a\nb"), is(false));
+ }
+
+ @Test void quotesRejected() {
+ // A value containing a single or double quote would let a fragment
+ // pushed as an alias terminate the surrounding OQL token.
+ assertThat(GeodeUtils.isSafeOqlIdentifier("a'b"), is(false));
+ assertThat(GeodeUtils.isSafeOqlIdentifier("a\"b"), is(false));
+ }
+
+ @Test void commaRejected() {
+ // A comma in an alias would split the select list.
+ assertThat(GeodeUtils.isSafeOqlIdentifier("a,b"), is(false));
+ }
+
+ @Test void punctuationRejected() {
+ assertThat(GeodeUtils.isSafeOqlIdentifier("a-b"), is(false));
+ assertThat(GeodeUtils.isSafeOqlIdentifier("a+b"), is(false));
+ assertThat(GeodeUtils.isSafeOqlIdentifier("a b"), is(false));
+ assertThat(GeodeUtils.isSafeOqlIdentifier("(a)"), is(false));
+ }
+
+ @Test void bracketedNonIntegerRejected() {
+ // Only integer subscripts are safe; anything else could carry
+ // OQL punctuation into the emitted statement.
+ assertThat(GeodeUtils.isSafeOqlIdentifier("a[b]"), is(false));
+ assertThat(GeodeUtils.isSafeOqlIdentifier("a['b']"), is(false));
+ assertThat(GeodeUtils.isSafeOqlIdentifier("a[]"), is(false));
+ assertThat(GeodeUtils.isSafeOqlIdentifier("a[0"), is(false));
+ }
+
+ @Test void checkSafeOqlIdentifierReturnsName() {
+ assertThat(GeodeUtils.checkSafeOqlIdentifier("field"), is("field"));
+ assertThat(GeodeUtils.checkSafeOqlIdentifier("EXPR$0"), is("EXPR$0"));
+ }
+
+ @Test void checkSafeOqlIdentifierThrowsOnUnsafe() {
+ RuntimeException e =
+ assertThrows(RuntimeException.class,
+ () -> GeodeUtils.checkSafeOqlIdentifier("a b"));
+ assertThat(e.getMessage(),
+ containsString("not a simple OQL identifier"));
+ }
+
+ @Test void checkSafeOqlIdentifierThrowsOnNull() {
+ assertThrows(RuntimeException.class,
+ () -> GeodeUtils.checkSafeOqlIdentifier(null));
+ }
+}
diff --git
a/mongodb/src/main/java/org/apache/calcite/adapter/mongodb/MongoAggregate.java
b/mongodb/src/main/java/org/apache/calcite/adapter/mongodb/MongoAggregate.java
index 4e368607de..d9a4358bc6 100644
---
a/mongodb/src/main/java/org/apache/calcite/adapter/mongodb/MongoAggregate.java
+++
b/mongodb/src/main/java/org/apache/calcite/adapter/mongodb/MongoAggregate.java
@@ -109,7 +109,11 @@ public MongoAggregate(RelOptCluster cluster, RelTraitSet
traitSet,
List<String> keys = new ArrayList<>();
for (int group : groupSet) {
final String inName = inNames.get(group);
- keys.add(inName + ": " + MongoRules.quote("$" + inName));
+ // The $group _id key is emitted verbatim into the BSON document, so
+ // route the key through the same quoting helper as the aggregate
+ // output names to prevent a value containing punctuation from
+ // shifting the document boundaries.
+ keys.add(MongoRules.maybeQuote(inName) + ": " + MongoRules.quote("$" +
inName));
++i;
}
list.add("_id: " + Util.toString(keys, "{", ", ", "}"));
diff --git
a/mongodb/src/test/java/org/apache/calcite/adapter/mongodb/MongoRulesTest.java
b/mongodb/src/test/java/org/apache/calcite/adapter/mongodb/MongoRulesTest.java
new file mode 100644
index 0000000000..970536b075
--- /dev/null
+++
b/mongodb/src/test/java/org/apache/calcite/adapter/mongodb/MongoRulesTest.java
@@ -0,0 +1,78 @@
+/*
+ * 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.mongodb;
+
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+/**
+ * Unit tests for the BSON identifier-quoting helpers in
+ * {@link MongoRules}. Field names emitted verbatim into a BSON
+ * document (as {@code $group._id} keys or aggregate output names)
+ * must be quoted whenever they contain a character that BsonDocument's
+ * parser would otherwise treat as part of a different token, so a
+ * value containing punctuation cannot shift the document boundaries.
+ */
+class MongoRulesTest {
+
+ @Test void plainNameNotQuoted() {
+ assertThat(MongoRules.maybeQuote("field"), is("field"));
+ assertThat(MongoRules.maybeQuote("_field"), is("_field"));
+ assertThat(MongoRules.maybeQuote("Field1"), is("Field1"));
+ }
+
+ @Test void dollarNameQuoted() {
+ // A leading {@code $} would otherwise be parsed by BsonDocument
+ // as an operator reference; quote it so the name stays literal.
+ assertThat(MongoRules.maybeQuote("$field"), is("'$field'"));
+ }
+
+ @Test void syntheticExprNameQuoted() {
+ // Calcite emits synthetic aggregate column names like EXPR$0.
+ // The $ makes them fail the "plain identifier" check, so
+ // maybeQuote must wrap them in quotes.
+ assertThat(MongoRules.maybeQuote("EXPR$0"), is("'EXPR$0'"));
+ }
+
+ @Test void nameWithPunctuationQuoted() {
+ assertThat(MongoRules.maybeQuote("a b"), is("'a b'"));
+ assertThat(MongoRules.maybeQuote("a.b"), is("'a.b'"));
+ assertThat(MongoRules.maybeQuote("a,b"), is("'a,b'"));
+ assertThat(MongoRules.maybeQuote("a:b"), is("'a:b'"));
+ }
+
+ @Test void quoteEscapesBackslashAndApostrophe() {
+ // Inside the quoted BSON literal a backslash or an apostrophe
+ // would otherwise let the string terminate early and shift the
+ // document boundaries; both must be escaped.
+ assertThat(MongoRules.quote("a\\b"), is("'a\\\\b'"));
+ assertThat(MongoRules.quote("a'b"), is("'a\\'b'"));
+ assertThat(MongoRules.quote("a\\'b"), is("'a\\\\\\'b'"));
+ }
+
+ @Test void quoteTrailingBackslash() {
+ // A trailing backslash must be doubled or it would otherwise
+ // consume the closing apostrophe.
+ assertThat(MongoRules.quote("a\\"), is("'a\\\\'"));
+ }
+
+ @Test void quoteApostropheOnly() {
+ assertThat(MongoRules.quote("'"), is("'\\''"));
+ }
+}
diff --git a/site/_docs/history.md b/site/_docs/history.md
index cc4e1e9ebb..0b6d23450b 100644
--- a/site/_docs/history.md
+++ b/site/_docs/history.md
@@ -119,6 +119,15 @@ #### Breaking Changes
`-Dcalcite.enable.spark=true`. The `spark` connection property alone no longer
activates it; a connection using `spark=true` without the opt-in will fail.
+* [<a
href="https://issues.apache.org/jira/browse/CALCITE-7781";>CALCITE-7781</a>]
+The fallback `SqlDialect` for an unrecognized JDBC product name now doubles
+backslashes when quoting string literals, so a value cannot terminate the
+literal early on a backend that treats `\` as an in-string escape character
+(the MySQL family, among others). Deployments whose unrecognized backend
+follows SQL-standard literal lexing may restore the legacy behavior with
+`-Dcalcite.sql.dialect.unknown.escapes.backslash=false`; the robust fix is
+to add a `SqlDialect` implementation for the product.
+
* [<a
href="https://issues.apache.org/jira/browse/CALCITE-7787";>CALCITE-7787</a>]
The Geode schema operand `pdxSerializablePackagePath` (honored by
`GeodeSchemaFactory`
and `GeodeSimpleSchemaFactory`) now requires the operator-level opt-in system
property
diff --git
a/splunk/src/main/java/org/apache/calcite/adapter/splunk/SplunkPushDownRule.java
b/splunk/src/main/java/org/apache/calcite/adapter/splunk/SplunkPushDownRule.java
index 44400635eb..ec5a68437d 100644
---
a/splunk/src/main/java/org/apache/calcite/adapter/splunk/SplunkPushDownRule.java
+++
b/splunk/src/main/java/org/apache/calcite/adapter/splunk/SplunkPushDownRule.java
@@ -42,12 +42,14 @@
import com.google.common.collect.ImmutableSet;
+import org.checkerframework.checker.nullness.qual.Nullable;
import org.immutables.value.Value;
import org.slf4j.Logger;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
+import java.util.regex.Pattern;
/**
* Planner rule to push filters and projections to Splunk.
@@ -58,6 +60,25 @@ public class SplunkPushDownRule
private static final Logger LOGGER =
StringUtils.getClassTracer(SplunkPushDownRule.class);
+ /**
+ * Shape of field names that may be copied verbatim into the SPL search
+ * string. SPL has no generic identifier-quoting mechanism, so a name
+ * outside this set is never pushed down; the corresponding filter or
+ * projection is evaluated by Calcite instead. The set is deliberately
+ * broad enough to cover the common Splunk shapes (leading underscore as
+ * in {@code _raw}, dotted paths, {@code {}} for multivalued fields,
+ * {@code $} in Calcite-generated aliases such as {@code EXPR$0}, and
+ * {@code -} in host-style names) so pushdown is preserved for legitimate
+ * inputs.
+ */
+ private static final Pattern SAFE_FIELD_NAME =
+ Pattern.compile("[A-Za-z0-9_.:@#{}$\\-]+");
+
+ /** Returns whether a field name may be embedded verbatim in SPL text. */
+ static boolean isSafeFieldName(String name) {
+ return name != null && SAFE_FIELD_NAME.matcher(name).matches();
+ }
+
private static final Set<SqlKind> SUPPORTED_OPS =
ImmutableSet.of(
SqlKind.CAST,
@@ -193,10 +214,15 @@ protected SplunkPushDownRule(RelOptRuleOperand operand,
}
LOGGER.debug("pre transformTo fieldNames: {}", getFieldsString(topRow));
- call.transformTo(
+ final RelNode rel =
appendSearchString(
filterString, splunkRel, topProj, bottomProj,
- topRow, null));
+ topRow, null);
+ if (rel == null) {
+ // A field-rename side is not a safe SPL name; leave the plan in place.
+ return;
+ }
+ call.transformTo(rel);
}
/**
@@ -206,8 +232,12 @@ protected SplunkPushDownRule(RelOptRuleOperand operand,
* @param splunkRel Relational expression
* @param topProj Top projection
* @param bottomProj Bottom projection
+ *
+ * @return the rewritten scan, or null if a field rename involves a name
+ * that cannot be safely embedded in SPL text (see
+ * {@link #isSafeFieldName})
*/
- protected RelNode appendSearchString(
+ protected @Nullable RelNode appendSearchString(
String toAppend,
SplunkTableScan splunkRel,
LogicalProject topProj,
@@ -269,6 +299,16 @@ protected RelNode appendSearchString(
}
if (!renames.isEmpty()) {
+ // SPL provides no generic identifier-quoting mechanism, so refuse to
+ // push down a rename when either side would not be a safe SPL name
+ // (see isSafeFieldName). Returning null signals the caller to leave
+ // the original plan in place.
+ for (int r = 0; r < renames.size(); r++) {
+ if (!isSafeFieldName(renames.left(r))
+ || !isSafeFieldName(renames.right(r))) {
+ return null;
+ }
+ }
updateSearchStr.append("| rename ");
renames.forEach((left, right) ->
updateSearchStr.append(left).append(" AS ")
@@ -309,7 +349,7 @@ private static RelNode addProjectionRule(LogicalProject
proj, RelNode rel) {
// TODO: refactor this to use more tree like parsing, need to also
// make sure we use parens properly - currently precedence
// rules are simply left to right
- private static boolean getFilter(SqlOperator op, List<RexNode> operands,
+ static boolean getFilter(SqlOperator op, List<RexNode> operands,
StringBuilder s, List<String> fieldNames) {
if (!valid(op.getKind())) {
return false;
@@ -364,6 +404,13 @@ private static boolean asd(boolean like, List<RexNode>
operands, StringBuilder s
}
int fieldIndex = ((RexInputRef) operand).getIndex();
String name = fieldNames.get(fieldIndex);
+ // Refuse pushdown when the field name is not a safe SPL name (see
+ // isSafeFieldName): SPL provides no generic identifier-quoting
+ // mechanism, so anything else could shift the boundaries of the
+ // emitted command.
+ if (!isSafeFieldName(name)) {
+ return false;
+ }
s.append(name);
} else { // RexLiteral
String tmp = toString(like, (RexLiteral) operand);
diff --git
a/splunk/src/test/java/org/apache/calcite/adapter/splunk/SplunkPushDownRuleTest.java
b/splunk/src/test/java/org/apache/calcite/adapter/splunk/SplunkPushDownRuleTest.java
new file mode 100644
index 0000000000..c3129d714e
--- /dev/null
+++
b/splunk/src/test/java/org/apache/calcite/adapter/splunk/SplunkPushDownRuleTest.java
@@ -0,0 +1,107 @@
+/*
+ * 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.splunk;
+
+import org.apache.calcite.jdbc.JavaTypeFactoryImpl;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexInputRef;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.sql.type.SqlTypeName;
+
+import com.google.common.collect.ImmutableList;
+
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.nullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+/**
+ * Unit tests for the field-name allowlist inside
+ * {@link SplunkPushDownRule}. SPL has no generic identifier-quoting
+ * mechanism, so a field name that is copied verbatim into the generated
+ * search must match a strict allowlist; anything else could shift the
+ * boundaries of the emitted command, and the corresponding filter or
+ * rename is evaluated by Calcite instead.
+ *
+ * <p>The tests exercise the filter translation directly and do not
+ * require a running Splunk instance.
+ */
+class SplunkPushDownRuleTest {
+
+ private static final RelDataTypeFactory TYPE_FACTORY = new
JavaTypeFactoryImpl();
+ private static final RexBuilder REX_BUILDER = new RexBuilder(TYPE_FACTORY);
+
+ /** Translates {@code <field> = 'foo'} via the rule's (private)
+ * {@code getFilter}, returning the SPL string, or null if the rule
+ * refuses the push down. */
+ private static String filterString(String fieldName) throws Throwable {
+ RelDataType varchar = TYPE_FACTORY.createSqlType(SqlTypeName.VARCHAR);
+ RexInputRef ref = REX_BUILDER.makeInputRef(varchar, 0);
+ RexNode lit = REX_BUILDER.makeLiteral("foo");
+ StringBuilder buf = new StringBuilder();
+ final boolean ok =
+ SplunkPushDownRule.getFilter(SqlStdOperatorTable.EQUALS,
ImmutableList.of(ref, lit), buf,
+ ImmutableList.of(fieldName));
+ return ok ? buf.toString() : null;
+ }
+
+ @Test void plainFieldNamePushedDown() throws Throwable {
+ assertThat(filterString("source"), is("source = foo"));
+ assertThat(filterString("date_hour"), is("date_hour = foo"));
+ }
+
+ @Test void splunkStyleFieldNamesAccepted() {
+ // Real-world Splunk field-name shapes: the allowlist must not
+ // reject these, or legitimate pushdown would stop working.
+ assertThat(SplunkPushDownRule.isSafeFieldName("_raw"), is(true));
+ assertThat(SplunkPushDownRule.isSafeFieldName("props{}.value"), is(true));
+ assertThat(SplunkPushDownRule.isSafeFieldName("EXPR$0"), is(true));
+ assertThat(SplunkPushDownRule.isSafeFieldName("host-name"), is(true));
+ assertThat(SplunkPushDownRule.isSafeFieldName("a.b.c"), is(true));
+ assertThat(SplunkPushDownRule.isSafeFieldName("date_hour"), is(true));
+ }
+
+ @Test void aliasWithPipeNotPushedDown() throws Throwable {
+ // A pipe character starts a new SPL command; a rename containing
+ // one would chain unintended pipeline stages onto the emitted
+ // search. The rule must therefore refuse this push down.
+ assertThat(filterString("x | delete | search a"), nullValue());
+ }
+
+ @Test void namesWithSplMetacharactersRejected() {
+ // Characters that would let a rename fragment or field reference
+ // shift the boundaries of the emitted SPL command.
+ assertThat(SplunkPushDownRule.isSafeFieldName("a b"), is(false));
+ assertThat(SplunkPushDownRule.isSafeFieldName("a\"b"), is(false));
+ assertThat(SplunkPushDownRule.isSafeFieldName("a'b"), is(false));
+ assertThat(SplunkPushDownRule.isSafeFieldName("a=b"), is(false));
+ assertThat(SplunkPushDownRule.isSafeFieldName("a[b]"), is(false));
+ assertThat(SplunkPushDownRule.isSafeFieldName("a`b"), is(false));
+ assertThat(SplunkPushDownRule.isSafeFieldName("a\\b"), is(false));
+ assertThat(SplunkPushDownRule.isSafeFieldName("a,b"), is(false));
+ assertThat(SplunkPushDownRule.isSafeFieldName("(a)"), is(false));
+ }
+
+ @Test void nullAndEmptyRejected() {
+ assertThat(SplunkPushDownRule.isSafeFieldName(null), is(false));
+ assertThat(SplunkPushDownRule.isSafeFieldName(""), is(false));
+ }
+}
