Skip to content

Commit e5c29fd

Browse files
Fix SQL query routing to analytics engine for composite-dataformat indices
The SQL path in RestUnifiedQueryAction.isAnalyticsIndex() was failing to route queries to the analytics engine because: 1. extractIndexName() used context.getParser() which returns SqlV2QueryParser (producing UnresolvedPlan), but the SQL branch expected a Calcite SqlNode. The V2 parser also treats 'score' as a reserved keyword, causing parse failures on common column names. 2. SqlTableNameExtractor did not handle SqlOrderBy, which Calcite wraps around SELECT...LIMIT queries. 3. Calcite's default parser uppercases unquoted identifiers, but index names in OpenSearch are lowercase. Fix: - Use SqlParser.create(query).parseQuery() (Calcite parser) for the SQL branch, which correctly returns SqlNode and handles standard SQL without reserved-word conflicts. - Add SqlOrderBy handling in SqlTableNameExtractor to unwrap LIMIT queries. - Lowercase the extracted table name before cluster state lookup. Signed-off-by: Finn Carroll <carrofin@amazon.com>
1 parent 817f7f3 commit e5c29fd

2 files changed

Lines changed: 33 additions & 3 deletions

File tree

plugin/src/main/java/org/opensearch/sql/plugin/rest/RestUnifiedQueryAction.java

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,17 @@
1010
import static org.opensearch.sql.opensearch.executor.OpenSearchQueryManager.SQL_WORKER_THREAD_POOL_NAME;
1111
import static org.opensearch.sql.protocol.response.format.JsonResponseFormatter.Style.PRETTY;
1212

13+
import java.util.Locale;
1314
import java.util.Map;
1415
import java.util.Optional;
1516
import org.apache.calcite.rel.RelNode;
1617
import org.apache.calcite.sql.SqlCall;
1718
import org.apache.calcite.sql.SqlIdentifier;
1819
import org.apache.calcite.sql.SqlJoin;
1920
import org.apache.calcite.sql.SqlNode;
21+
import org.apache.calcite.sql.SqlOrderBy;
2022
import org.apache.calcite.sql.SqlSelect;
23+
import org.apache.calcite.sql.parser.SqlParser;
2124
import org.apache.calcite.sql.util.SqlBasicVisitor;
2225
import org.apache.logging.log4j.LogManager;
2326
import org.apache.logging.log4j.Logger;
@@ -214,13 +217,14 @@ private void forwardClusterSetting(
214217
* node. Uses the context's parser which supports both PPL and SQL.
215218
*/
216219
private static Optional<String> extractIndexName(
217-
String query, QueryType queryType, UnifiedQueryContext context) {
220+
String query, QueryType queryType, UnifiedQueryContext context) throws Exception {
218221
if (queryType == QueryType.PPL) {
219222
UnresolvedPlan unresolvedPlan = (UnresolvedPlan) context.getParser().parse(query);
220223
return Optional.ofNullable(unresolvedPlan.accept(new IndexNameExtractor(), null));
221224
}
222-
SqlNode sqlNode = (SqlNode) context.getParser().parse(query);
223-
return Optional.ofNullable(extractTableNameFromSqlNode(sqlNode));
225+
SqlNode sqlNode = SqlParser.create(query).parseQuery();
226+
String tableName = extractTableNameFromSqlNode(sqlNode);
227+
return Optional.ofNullable(tableName != null ? tableName.toLowerCase(Locale.ROOT) : null);
224228
}
225229

226230
/** AST visitor that extracts the source index name from a Relation node (PPL path). */
@@ -235,6 +239,9 @@ public String visitRelation(Relation node, Void context) {
235239
private static class SqlTableNameExtractor extends SqlBasicVisitor<String> {
236240
@Override
237241
public String visit(SqlCall call) {
242+
if (call instanceof SqlOrderBy orderBy) {
243+
return orderBy.query.accept(this);
244+
}
238245
if (call instanceof SqlSelect select) {
239246
return select.getFrom().accept(this);
240247
}

plugin/src/test/java/org/opensearch/sql/plugin/rest/RestUnifiedQueryActionTest.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,29 @@ public void pluggableDataformatIndexRoutesToAnalytics() {
6565
action.isAnalyticsIndex("source = opensearch.parquet_logs | fields ts", QueryType.PPL));
6666
}
6767

68+
@Test
69+
public void sqlQueryRoutesToAnalyticsForCompositeIndex() {
70+
registerIndex(
71+
"parquet_logs",
72+
Settings.builder()
73+
.put(IndexSettings.PLUGGABLE_DATAFORMAT_ENABLED_SETTING.getKey(), true)
74+
.put(IndexSettings.PLUGGABLE_DATAFORMAT_VALUE_SETTING.getKey(), "composite")
75+
.build());
76+
77+
assertTrue(action.isAnalyticsIndex("SELECT ts, msg FROM parquet_logs", QueryType.SQL));
78+
assertTrue(action.isAnalyticsIndex("SELECT ts FROM parquet_logs LIMIT 10", QueryType.SQL));
79+
assertTrue(
80+
action.isAnalyticsIndex(
81+
"SELECT score, name FROM parquet_logs WHERE score > 90", QueryType.SQL));
82+
}
83+
84+
@Test
85+
public void sqlQueryRoutesToLuceneForNonCompositeIndex() {
86+
registerIndex("plain_logs", Settings.EMPTY);
87+
88+
assertFalse(action.isAnalyticsIndex("SELECT ts FROM plain_logs", QueryType.SQL));
89+
}
90+
6891
@Test
6992
public void pluggableEnabledButLuceneFormatRoutesToLucene() {
7093
registerIndex(

0 commit comments

Comments
 (0)