refactor(query): extract optional query paths into support crates#19644
Open
KKould wants to merge 28 commits intodatabendlabs:mainfrom
Open
refactor(query): extract optional query paths into support crates#19644KKould wants to merge 28 commits intodatabendlabs:mainfrom
KKould wants to merge 28 commits intodatabendlabs:mainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7345132e6c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
1dcabb6 to
bb4f71d
Compare
bb4f71d to
cecb3e4
Compare
Member
Author
|
@codex review |
Collaborator
|
Have you considered incremental compilation? The initial compilation time isn’t that critical; what matters is how long it takes to compile after making minor changes.It’s worth noting that the compilation outputs for different features are completely non-reusable. |
sundy-li
reviewed
Apr 3, 2026
sundy-li
reviewed
Apr 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/
Summary
task-support,script-udf, andstorage-stagequery paths into dedicated support cratesdatabend-queryfeature gates to those support crates so toggling a feature removes the corresponding dependency edge instead of relying on more fragile in-cratecfgwiringdatabend-querybehavior aligned withmainwhile making lean development builds cheaper to compileChanges
Compared with
main, this PR changes how a few heavier optional query paths are organized in the workspace.Instead of keeping the feature-specific implementations directly inside the main query service crate, this PR moves them into dedicated support crates and keeps
databend-queryresponsible only for the thin registration and bridge points. That makes the optional dependency boundaries much clearer, reduces the chance of breaking feature combinations with scatteredcfgblocks, and keeps the default build behavior unchanged.The concrete changes are:
src/query/task_supportfor task interpreters, task table functions, and task-related system tablessrc/query/script_udf_supportfor script UDF runtime, transform, and UDAF integrationssrc/query/storage_stage_supportas the feature-gated boundary for stage-storage integrationdatabend-queryanddatabend-query/serviceto consume those support crates through feature-gated dependencies instead of directly owning the full implementationsImplementation
src/query/task_support, leaving only the service-side context and registration bridge indatabend-query/service.src/query/script_udf_support, and re-export the required entry points back into the service layer.src/query/storage_stage_supportso the stage-storage path is isolated behind a narrower feature-gated crate boundary.main, while allowing smaller dependency graphs when those optional paths are not needed during development.Stable Compile Benchmark
Benchmark shape:
databend-querymake cleanbefore the benchmarktarget-dirper casecargorunscargo check -p databend-query --lib--no-default-features --features simdand add exactly one optional path--all-featuressimdbaseline twice, once at the beginning and once at the endBenchmark environment:
The two
simdbaselines were:801.90s793.52sSo I use their mean,
797.71s, as the reference point, and treat roughly+-4.19sas the low-confidence noise band for this run.Results:
simd_meansimd_baselinecargo check -p databend-query --lib --no-default-features --features simd801.90s+4.19s+0.53%script-udfcargo check -p databend-query --lib --no-default-features --features simd,script-udf860.24s+62.53s+7.84%task-supportcargo check -p databend-query --lib --no-default-features --features simd,task-support812.91s+15.20s+1.91%storage-stagecargo check -p databend-query --lib --no-default-features --features simd,storage-stage1206.29s+408.58s+51.22%defaultcargo check -p databend-query --lib1275.07s+477.36s+59.84%all-featurescargo check -p databend-query --lib --all-features1311.99s+514.28s+64.47%simd_repeatcargo check -p databend-query --lib --no-default-features --features simd793.52s-4.19s-0.53%Reviewer-facing takeaways:
storage-stageis still the dominant optional path by a wide margin:+408.58svs the mean baseline, about+51.22%storage-stage;defaultis only about68.78sabovestorage-stagealonescript-udfhas a clear standalone cost of+62.53s, about+7.84%task-supportis now a small but measurable standalone cost at+15.20s, about+1.91%, still far belowstorage-stageandscript-udf--all-featuresis heavier than the default path by about36.92s, so most of the compile weight is already present in the default feature setTests
Type of change
Risks
This change is