Skip to content

Commit 8113f69

Browse files
authored
Fix analytics engine routing for SQL after V2 parser change (opensearch-project#5456)
The switch from CalciteSqlQueryParser to SqlV2QueryParser (opensearch-project#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 6926de1 commit 8113f69

2 files changed

Lines changed: 67 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;
@@ -215,12 +209,8 @@ private void forwardClusterSetting(
215209
*/
216210
private static Optional<String> extractIndexName(
217211
String query, QueryType queryType, UnifiedQueryContext context) {
218-
if (queryType == QueryType.PPL) {
219-
UnresolvedPlan unresolvedPlan = (UnresolvedPlan) context.getParser().parse(query);
220-
return Optional.ofNullable(unresolvedPlan.accept(new IndexNameExtractor(), null));
221-
}
222-
SqlNode sqlNode = (SqlNode) context.getParser().parse(query);
223-
return Optional.ofNullable(extractTableNameFromSqlNode(sqlNode));
212+
UnresolvedPlan unresolvedPlan = (UnresolvedPlan) context.getParser().parse(query);
213+
return Optional.ofNullable(unresolvedPlan.accept(new IndexNameExtractor(), null));
224214
}
225215

226216
/** AST visitor that extracts the source index name from a Relation node (PPL path). */
@@ -231,29 +221,6 @@ public String visitRelation(Relation node, Void context) {
231221
}
232222
}
233223

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

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

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,10 +89,75 @@ 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));
137+
}
138+
139+
@Test
140+
public void showStatementRoutesToLucene() {
141+
registerIndex(
142+
"parquet_logs",
143+
Settings.builder()
144+
.put(IndexSettings.PLUGGABLE_DATAFORMAT_ENABLED_SETTING.getKey(), true)
145+
.put(IndexSettings.PLUGGABLE_DATAFORMAT_VALUE_SETTING.getKey(), "composite")
146+
.build());
147+
148+
assertFalse(action.isAnalyticsIndex("SHOW TABLES LIKE 'parquet_logs'", QueryType.SQL));
149+
}
150+
151+
@Test
152+
public void describeStatementRoutesToLucene() {
153+
registerIndex(
154+
"parquet_logs",
155+
Settings.builder()
156+
.put(IndexSettings.PLUGGABLE_DATAFORMAT_ENABLED_SETTING.getKey(), true)
157+
.put(IndexSettings.PLUGGABLE_DATAFORMAT_VALUE_SETTING.getKey(), "composite")
158+
.build());
159+
160+
assertFalse(action.isAnalyticsIndex("DESCRIBE TABLES LIKE 'parquet_logs'", QueryType.SQL));
96161
}
97162

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

0 commit comments

Comments
 (0)