Skip to content

Commit f006e29

Browse files
Route analytics queries by index setting, not table-name prefix (#5429)
1 parent 0d5e553 commit f006e29

2 files changed

Lines changed: 81 additions & 15 deletions

File tree

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

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.opensearch.cluster.service.ClusterService;
2828
import org.opensearch.common.unit.TimeValue;
2929
import org.opensearch.core.action.ActionListener;
30+
import org.opensearch.index.IndexSettings;
3031
import org.opensearch.sql.api.UnifiedQueryContext;
3132
import org.opensearch.sql.api.UnifiedQueryPlanner;
3233
import org.opensearch.sql.ast.AbstractNodeVisitor;
@@ -73,8 +74,10 @@ public RestUnifiedQueryAction(
7374
}
7475

7576
/**
76-
* Check if the query targets an analytics engine index (e.g., Parquet-backed). Uses the context's
77-
* parser for index name extraction, supporting both PPL and SQL.
77+
* Returns true iff the target index has {@link
78+
* IndexSettings#PLUGGABLE_DATAFORMAT_ENABLED_SETTING} set and {@link
79+
* IndexSettings#PLUGGABLE_DATAFORMAT_VALUE_SETTING} is {@code "parquet"}, routing it to
80+
* DataFusion instead of the Calcite→DSL path.
7881
*
7982
* <p>Note: This creates a separate UnifiedQueryContext for parsing. The context cannot be shared
8083
* with doExecute/doExplain because UnifiedQueryContext holds a Calcite JDBC connection that fails
@@ -87,18 +90,29 @@ public boolean isAnalyticsIndex(String query, QueryType queryType) {
8790
}
8891
try (UnifiedQueryContext context = buildParsingContext(queryType)) {
8992
return extractIndexName(query, queryType, context)
90-
.map(
91-
indexName -> {
92-
int lastDot = indexName.lastIndexOf('.');
93-
return lastDot >= 0 ? indexName.substring(lastDot + 1) : indexName;
94-
})
95-
.map(tableName -> tableName.startsWith("parquet_"))
93+
.map(this::stripSchemaPrefix)
94+
.map(this::isPluggableDataformatIndex)
9695
.orElse(false);
9796
} catch (Exception e) {
9897
return false;
9998
}
10099
}
101100

101+
private String stripSchemaPrefix(String indexName) {
102+
int lastDot = indexName.lastIndexOf('.');
103+
return lastDot >= 0 ? indexName.substring(lastDot + 1) : indexName;
104+
}
105+
106+
private boolean isPluggableDataformatIndex(String indexName) {
107+
var indexMetadata = clusterService.state().metadata().index(indexName);
108+
if (indexMetadata == null) {
109+
return false;
110+
}
111+
var settings = indexMetadata.getSettings();
112+
return IndexSettings.PLUGGABLE_DATAFORMAT_ENABLED_SETTING.get(settings)
113+
&& "parquet".equals(IndexSettings.PLUGGABLE_DATAFORMAT_VALUE_SETTING.get(settings));
114+
}
115+
102116
/** Execute a query through the unified query pipeline on the sql-worker thread pool. */
103117
public void execute(
104118
String query,

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

Lines changed: 59 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,44 +8,96 @@
88
import static org.junit.Assert.assertFalse;
99
import static org.junit.Assert.assertTrue;
1010
import static org.mockito.Mockito.mock;
11+
import static org.mockito.Mockito.when;
1112

1213
import org.apache.calcite.rel.RelNode;
1314
import org.junit.Before;
1415
import org.junit.Test;
1516
import org.opensearch.analytics.exec.QueryPlanExecutor;
17+
import org.opensearch.cluster.ClusterState;
18+
import org.opensearch.cluster.metadata.IndexMetadata;
19+
import org.opensearch.cluster.metadata.Metadata;
1620
import org.opensearch.cluster.service.ClusterService;
17-
import org.opensearch.sql.common.setting.Settings;
21+
import org.opensearch.common.settings.Settings;
22+
import org.opensearch.index.IndexSettings;
1823
import org.opensearch.sql.executor.QueryType;
1924
import org.opensearch.transport.client.node.NodeClient;
2025

2126
/**
22-
* Tests for analytics index routing in RestUnifiedQueryAction. Uses context parser for AST-based
23-
* index name extraction.
27+
* Tests for analytics index routing in RestUnifiedQueryAction. Routing requires both {@code
28+
* index.pluggable.dataformat.enabled=true} and {@code index.pluggable.dataformat=parquet}.
2429
*/
2530
public class RestUnifiedQueryActionTest {
2631

32+
private ClusterService clusterService;
33+
private Metadata metadata;
2734
private RestUnifiedQueryAction action;
2835

2936
@Before
3037
public void setUp() {
38+
clusterService = mock(ClusterService.class);
39+
ClusterState clusterState = mock(ClusterState.class);
40+
metadata = mock(Metadata.class);
41+
when(clusterService.state()).thenReturn(clusterState);
42+
when(clusterState.metadata()).thenReturn(metadata);
43+
3144
@SuppressWarnings("unchecked")
3245
QueryPlanExecutor<RelNode, Iterable<Object[]>> executor = mock(QueryPlanExecutor.class);
3346
action =
3447
new RestUnifiedQueryAction(
35-
mock(NodeClient.class), mock(ClusterService.class), executor, mock(Settings.class));
48+
mock(NodeClient.class),
49+
clusterService,
50+
executor,
51+
mock(org.opensearch.sql.common.setting.Settings.class));
3652
}
3753

3854
@Test
39-
public void parquetIndexRoutesToAnalytics() {
55+
public void pluggableDataformatIndexRoutesToAnalytics() {
56+
registerIndex(
57+
"parquet_logs",
58+
Settings.builder()
59+
.put(IndexSettings.PLUGGABLE_DATAFORMAT_ENABLED_SETTING.getKey(), true)
60+
.put(IndexSettings.PLUGGABLE_DATAFORMAT_VALUE_SETTING.getKey(), "parquet")
61+
.build());
62+
4063
assertTrue(action.isAnalyticsIndex("source = parquet_logs | fields ts", QueryType.PPL));
4164
assertTrue(
4265
action.isAnalyticsIndex("source = opensearch.parquet_logs | fields ts", QueryType.PPL));
4366
}
4467

4568
@Test
46-
public void nonParquetIndexRoutesToLucene() {
47-
assertFalse(action.isAnalyticsIndex("source = my_logs | fields ts", QueryType.PPL));
69+
public void pluggableEnabledButLuceneFormatRoutesToLucene() {
70+
registerIndex(
71+
"lucene_logs",
72+
Settings.builder()
73+
.put(IndexSettings.PLUGGABLE_DATAFORMAT_ENABLED_SETTING.getKey(), true)
74+
.put(IndexSettings.PLUGGABLE_DATAFORMAT_VALUE_SETTING.getKey(), "lucene")
75+
.build());
76+
77+
assertFalse(action.isAnalyticsIndex("source = lucene_logs | fields ts", QueryType.PPL));
78+
}
79+
80+
@Test
81+
public void indexWithoutSettingRoutesToLucene() {
82+
registerIndex("plain_logs", Settings.EMPTY);
83+
84+
assertFalse(action.isAnalyticsIndex("source = plain_logs | fields ts", QueryType.PPL));
85+
}
86+
87+
@Test
88+
public void missingIndexRoutesToLucene() {
89+
assertFalse(action.isAnalyticsIndex("source = does_not_exist | fields ts", QueryType.PPL));
90+
}
91+
92+
@Test
93+
public void nullAndEmptyQueriesRouteToLucene() {
4894
assertFalse(action.isAnalyticsIndex(null, QueryType.PPL));
4995
assertFalse(action.isAnalyticsIndex("", QueryType.PPL));
5096
}
97+
98+
private void registerIndex(String name, Settings settings) {
99+
IndexMetadata indexMetadata = mock(IndexMetadata.class);
100+
when(indexMetadata.getSettings()).thenReturn(settings);
101+
when(metadata.index(name)).thenReturn(indexMetadata);
102+
}
51103
}

0 commit comments

Comments
 (0)