Skip to content

Adding dsl-query-executor plugin to sandbox#20969

Merged
mch2 merged 4 commits into
opensearch-project:feature/datafusionfrom
vinaykpud:feature/datafusion-dsl-plugin
Mar 25, 2026
Merged

Adding dsl-query-executor plugin to sandbox#20969
mch2 merged 4 commits into
opensearch-project:feature/datafusionfrom
vinaykpud:feature/datafusion-dsl-plugin

Conversation

@vinaykpud
Copy link
Copy Markdown
Contributor

@vinaykpud vinaykpud commented Mar 23, 2026

Description

[Describe what this change achieves]

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@vinaykpud
Add dsl-query-executor sandbox plugin for DSL-to-Calcite query execution
d43d715 
Introduces a new sandbox plugin that intercepts _search requests and
routes them through a execution pipeline via the analytics engine.

Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>
@vinaykpud
Add query filter, project, and sort converters
ae00298 
Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>
@vinaykpud
Added Bucket and Metric aggregation translators
cc4b546 
Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>
@vinaykpud
resolve conflicts in the build.gradle
548f2d8 
Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>
@vinaykpud vinaykpud marked this pull request as ready for review March 23, 2026 17:32
@vinaykpud vinaykpud requested a review from a team as a code owner March 23, 2026 17:32
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 23, 2026

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit 548f2d8.

PathLineSeverityDescription
sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/action/SearchActionFilter.java44mediumSearchActionFilter sets order() to Integer.MIN_VALUE, ensuring it executes before ALL other action filters including security/authorization filters. When a SearchAction is intercepted, the code bypasses chain.proceed() and dispatches directly to an internal 'cluster:internal/dsl/execute' action. Depending on security plugin filter ordering, this could redirect search requests before authorization checks complete. The TODO comment acknowledges the missing index-level gating check, confirming this is incomplete rather than fully vetted behavior.
sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/action/SearchActionFilter.java53mediumThe filter unconditionally intercepts every _search request cluster-wide and reroutes it through the experimental Calcite pipeline regardless of index or cluster configuration. There is no opt-in mechanism, feature flag, or index-level setting check. A comment in the test class explicitly flags this as missing: 'add tests to verify reroute only happens when the target index has the setting enabled'. This universal interception could silently alter query semantics for all tenants on a shared cluster.
sandbox/libs/analytics-framework/build.gradle33lowAddition of 'commons-io:commons-io' as a runtimeOnly dependency in the analytics-framework library. While commons-io is a well-known Apache library, its addition is unrelated to the stated PR scope (DSL query executor plugin) and is added to a shared framework library rather than the plugin itself. The specific version is not pinned inline but relies on a versions map — the justification for this addition is absent from commit context.

The table above displays the top 10 most important findings.

Total: 3 | Critical: 0 | High: 0 | Medium: 2 | Low: 1

Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.

⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

nssuresh2007
nssuresh2007 reviewed Mar 24, 2026
View reviewed changes
Comment thread ...dsl-query-executor/src/main/java/org/opensearch/dsl/aggregation/metric/MetricTranslator.java
* Translates a metric aggregation (AVG, SUM, MIN, MAX, etc.) to a Calcite AggregateCall,
* and converts raw result values back to OpenSearch InternalAggregation for response building.
*/
public interface MetricTranslator<T extends AggregationBuilder> extends AggregationType<T> {
Copy link
Copy Markdown
Contributor

@nssuresh2007 nssuresh2007 Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we rename this as SingleValueMetricTranslator since this returns single aggregate call, whereas for multi-value metric aggregations (like stats) we need multiple aggregate calls to be returned and that would need a different base class?

Copy link
Copy Markdown
Contributor Author

@vinaykpud vinaykpud Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For that i thought we can have like this

  • MetricTranslator as top level interface (we may still need this)
  • SingleMetricTranslator and all single value metrics will extend this
  • MultiValueMetricTranslator, all multi value metrics will extend this

@mch2 mch2 merged commit e4a1193 into opensearch-project:feature/datafusion Mar 25, 2026
10 of 35 checks passed
Bukhtawar
Bukhtawar reviewed Mar 26, 2026
View reviewed changes
Comment thread ...ns/dsl-query-executor/src/main/java/org/opensearch/dsl/action/TransportDslExecuteAction.java
protected void doExecute(Task task, SearchRequest request, ActionListener<SearchResponse> listener) {
try {
String indexName = resolveToSingleIndex(request);
long startTime = System.currentTimeMillis();
Copy link
Copy Markdown
Contributor

@Bukhtawar Bukhtawar Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets use nanoTime as wall clock time for measuring elapsed time

Copy link
Copy Markdown
Contributor Author

@vinaykpud vinaykpud Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will update this in #20916

himshikhagupta pushed a commit to himshikhagupta/OpenSearch that referenced this pull request Apr 8, 2026
@vinaykpud @himshikhagupta
Adding dsl-query-executor plugin to sandbox (opensearch-project#20969)
4fa7bd1 
* Add dsl-query-executor sandbox plugin for DSL-to-Calcite query execution

Introduces a new sandbox plugin that intercepts _search requests and
routes them through a execution pipeline via the analytics engine.

Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>

* Add query filter, project, and sort converters

Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>

* Added Bucket and Metric aggregation translators

Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>

* resolve conflicts in the build.gradle

Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>

---------

Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>
Signed-off-by: Himshikha Gupta <himshikh@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Bukhtawar Bukhtawar Bukhtawar left review comments

+1 more reviewer

@nssuresh2007 nssuresh2007 nssuresh2007 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vinaykpud @Bukhtawar @nssuresh2007 @mch2