Skip to content

Dart resource deprecation#18003

Merged
cryptoe merged 28 commits intoapache:masterfrom
adarshsanjeev:dart-resource-deprecation
Jun 2, 2025
Merged

Dart resource deprecation#18003
cryptoe merged 28 commits intoapache:masterfrom
adarshsanjeev:dart-resource-deprecation

Conversation

@adarshsanjeev
Copy link
Copy Markdown
Contributor

@adarshsanjeev adarshsanjeev commented May 15, 2025

Presently, DartResources handles DART queries, while SqlResource handles sql native queries. This includes some Dart specific code which needs to be run for creating and cancelling queries.

This PR aims to unify these interfaces by adding an "engine selector" to SqlResource to allow it to run native as well as Dart queries. This involves pushing any engine specific code into the SqlEngine and refactoring the Dart cancellation to be more generic.

Release Notes

  • Dart specific endpoints have been removed and folded into SqlResource.
  • Added a new engine QueryContext parameter. The value can be native or msq-dart. The value determines the engine used to run the query. The default value is native.

This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • a release note entry in the PR description.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

@github-actions github-actions Bot added Area - Batch Ingestion Area - Querying Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262 labels May 15, 2025
Copy link
Copy Markdown
Member

@kgyrtkirk kgyrtkirk left a comment

Choose a reason for hiding this comment

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

I know you've just opened to run CI; but I wonder if this should be the path or not

Comment thread processing/src/main/java/org/apache/druid/query/Engine.java Outdated
@adarshsanjeev adarshsanjeev added the Needs web console change Backend API changes that would benefit from frontend support in the web console label May 20, 2025
Comment thread sql/src/main/java/org/apache/druid/sql/http/SqlResource.java Fixed
Comment thread sql/src/test/java/org/apache/druid/sql/http/SqlResourceTest.java Fixed
Comment thread sql/src/test/java/org/apache/druid/sql/http/SqlResourceTest.java Fixed
@adarshsanjeev adarshsanjeev marked this pull request as ready for review May 20, 2025 10:52
Copy link
Copy Markdown
Member

@kgyrtkirk kgyrtkirk left a comment

Choose a reason for hiding this comment

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

looks good! left a bunch of comments but nothing really serious... mostly just questions/minor notices :)

Comment thread sql/src/main/java/org/apache/druid/sql/http/SqlResource.java Outdated
Comment thread sql/src/main/java/org/apache/druid/sql/calcite/run/NativeSqlEngine.java Outdated
Comment thread sql/src/main/java/org/apache/druid/sql/http/SqlResource.java Outdated
Comment thread sql/src/main/java/org/apache/druid/sql/calcite/run/SqlEngine.java Outdated
Comment thread sql/src/main/java/org/apache/druid/sql/calcite/run/SqlEngine.java Outdated
Comment thread sql/src/main/java/org/apache/druid/sql/http/QueryInfo.java Outdated
Comment thread sql/src/main/java/org/apache/druid/sql/http/SqlResource.java Outdated
Comment thread sql/src/test/java/org/apache/druid/sql/http/GetQueriesResponseTest.java Outdated
Comment thread sql/src/test/java/org/apache/druid/sql/http/GetQueriesResponseTest.java Outdated
Comment thread sql/src/main/java/org/apache/druid/sql/http/SqlResource.java Outdated
Comment thread sql/src/main/java/org/apache/druid/sql/calcite/run/SqlEngine.java Outdated
Comment thread sql/src/main/java/org/apache/druid/sql/calcite/run/SqlEngine.java Outdated
Comment thread sql/src/main/java/org/apache/druid/sql/http/SupportedEnginesResponse.java Outdated
@cryptoe cryptoe merged commit 69b2e80 into apache:master Jun 2, 2025
72 of 75 checks passed
@capistrant capistrant added this to the 34.0.0 milestone Jul 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area - Batch Ingestion Area - Dependencies Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262 Area - Querying Needs web console change Backend API changes that would benefit from frontend support in the web console

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants