Skip to content

Commit a9ad0bf

Browse files
committed
Fix analytics engine routing for SQL after V2 parser change
The switch from CalciteSqlQueryParser to SqlV2QueryParser (#5438) changed the parse return type from SqlNode to UnresolvedPlan. This caused a ClassCastException in extractIndexName(), which was silently caught and returned false, routing all SQL queries to V2 engine instead of analytics. Fix: unify SQL and PPL paths to both use UnresolvedPlan + IndexNameExtractor. Remove dead SqlNode-based extraction code (SqlTableNameExtractor). Signed-off-by: Chen Dai <daichen@amazon.com>
1 parent 5851bdb commit a9ad0bf

2 files changed

Lines changed: 43 additions & 35 deletions

File tree

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

Lines changed: 2 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,6 @@
1313
import java.util.Map;
1414
import java.util.Optional;
1515
import org.apache.calcite.rel.RelNode;
16-
import org.apache.calcite.sql.SqlCall;
17-
import org.apache.calcite.sql.SqlIdentifier;
18-
import org.apache.calcite.sql.SqlJoin;
19-
import org.apache.calcite.sql.SqlNode;
20-
import org.apache.calcite.sql.SqlSelect;
21-
import org.apache.calcite.sql.util.SqlBasicVisitor;
2216
import org.apache.logging.log4j.LogManager;
2317
import org.apache.logging.log4j.Logger;
2418
import org.apache.logging.log4j.ThreadContext;
@@ -213,12 +207,8 @@ private UnifiedQueryContext.Builder applyClusterOverrides(UnifiedQueryContext.Bu
213207
*/
214208
private static Optional<String> extractIndexName(
215209
String query, QueryType queryType, UnifiedQueryContext context) {
216-
if (queryType == QueryType.PPL) {
217-
UnresolvedPlan unresolvedPlan = (UnresolvedPlan) context.getParser().parse(query);
218-
return Optional.ofNullable(unresolvedPlan.accept(new IndexNameExtractor(), null));
219-
}
220-
SqlNode sqlNode = (SqlNode) context.getParser().parse(query);
221-
return Optional.ofNullable(extractTableNameFromSqlNode(sqlNode));
210+
UnresolvedPlan unresolvedPlan = (UnresolvedPlan) context.getParser().parse(query);
211+
return Optional.ofNullable(unresolvedPlan.accept(new IndexNameExtractor(), null));
222212
}
223213

224214
/** AST visitor that extracts the source index name from a Relation node (PPL path). */
@@ -229,29 +219,6 @@ public String visitRelation(Relation node, Void context) {
229219
}
230220
}
231221

232-
/** SqlNode visitor that extracts the source table name from a SQL parse tree. */
233-
private static class SqlTableNameExtractor extends SqlBasicVisitor<String> {
234-
@Override
235-
public String visit(SqlCall call) {
236-
if (call instanceof SqlSelect select) {
237-
return select.getFrom().accept(this);
238-
}
239-
if (call instanceof SqlJoin join) {
240-
return join.getLeft().accept(this);
241-
}
242-
return null;
243-
}
244-
245-
@Override
246-
public String visit(SqlIdentifier id) {
247-
return id.toString();
248-
}
249-
}
250-
251-
private static String extractTableNameFromSqlNode(SqlNode sqlNode) {
252-
return sqlNode.accept(new SqlTableNameExtractor());
253-
}
254-
255222
private static RelNode addQuerySizeLimit(RelNode plan, CalcitePlanContext context) {
256223
return LogicalSystemLimit.create(
257224
LogicalSystemLimit.SystemLimitType.QUERY_SIZE_LIMIT,

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,10 +89,51 @@ public void missingIndexRoutesToLucene() {
8989
assertFalse(action.isAnalyticsIndex("source = does_not_exist | fields ts", QueryType.PPL));
9090
}
9191

92+
@Test
93+
public void sqlQueryRoutesToAnalyticsForPluggableIndex() {
94+
registerIndex(
95+
"parquet_logs",
96+
Settings.builder()
97+
.put(IndexSettings.PLUGGABLE_DATAFORMAT_ENABLED_SETTING.getKey(), true)
98+
.put(IndexSettings.PLUGGABLE_DATAFORMAT_VALUE_SETTING.getKey(), "composite")
99+
.build());
100+
101+
assertTrue(action.isAnalyticsIndex("SELECT * FROM parquet_logs", QueryType.SQL));
102+
assertTrue(
103+
action.isAnalyticsIndex(
104+
"SELECT ts, level FROM parquet_logs WHERE level = 'ERROR'", QueryType.SQL));
105+
}
106+
107+
@Test
108+
public void sqlQueryWithSchemaRoutesToAnalytics() {
109+
registerIndex(
110+
"parquet_logs",
111+
Settings.builder()
112+
.put(IndexSettings.PLUGGABLE_DATAFORMAT_ENABLED_SETTING.getKey(), true)
113+
.put(IndexSettings.PLUGGABLE_DATAFORMAT_VALUE_SETTING.getKey(), "composite")
114+
.build());
115+
116+
assertTrue(action.isAnalyticsIndex("SELECT * FROM opensearch.parquet_logs", QueryType.SQL));
117+
}
118+
119+
@Test
120+
public void sqlQueryRoutesToLuceneForNonPluggableIndex() {
121+
registerIndex("plain_logs", Settings.EMPTY);
122+
123+
assertFalse(action.isAnalyticsIndex("SELECT * FROM plain_logs", QueryType.SQL));
124+
}
125+
126+
@Test
127+
public void sqlQueryRoutesToLuceneForMissingIndex() {
128+
assertFalse(action.isAnalyticsIndex("SELECT * FROM does_not_exist", QueryType.SQL));
129+
}
130+
92131
@Test
93132
public void nullAndEmptyQueriesRouteToLucene() {
94133
assertFalse(action.isAnalyticsIndex(null, QueryType.PPL));
95134
assertFalse(action.isAnalyticsIndex("", QueryType.PPL));
135+
assertFalse(action.isAnalyticsIndex(null, QueryType.SQL));
136+
assertFalse(action.isAnalyticsIndex("", QueryType.SQL));
96137
}
97138

98139
private void registerIndex(String name, Settings settings) {

0 commit comments

Comments
 (0)