⚠ This page is served via a proxy. Original site: https://github.com
This service does not collect credentials or authentication data.
Skip to content

Conversation

@xiangfu0
Copy link
Contributor

Summary

Add server‑side gapfill support for the v2 engine, including query‑option plumbing, server response handling, and multi‑stage plan/runtime integration.

Key Changes

  • Introduce serverSideGapfill and serverSideGapfillQuery query options, plus serialization/deserialization of the original PinotQuery for gapfill evaluation.

  • Broker handlers (single‑stage and multi‑stage) detect eligible gapfill queries and inject server‑side gapfill options with routing/partition checks.

  • Server‑side execution uses new GapfillInstanceResponseOperator and GapfillInstanceResponsePlanNode; v2 plan maker wires them in.

  • Runtime/leaf planning strips GAPFILL where needed and applies gapfill in TransformOperator for MSE.

  • Gapfill reducers and DateTimeConvert updated to align server‑side gapfill behavior; add GapfillIntegrationTest coverage.

@xiangfu0 xiangfu0 marked this pull request as draft January 19, 2026 14:35
@xiangfu0 xiangfu0 force-pushed the gapfill-v2-gapfill-test branch from 22f4325 to 8d58b42 Compare January 19, 2026 14:43
@codecov-commenter
Copy link

codecov-commenter commented Jan 19, 2026

Codecov Report

❌ Patch coverage is 10.26316% with 682 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.02%. Comparing base (c899956) to head (6accd39).

Files with missing lines Patch % Lines
...sthandler/BaseSingleStageBrokerRequestHandler.java 2.53% 152 Missing and 2 partials ⚠️
.../pinot/query/runtime/operator/GapfillOperator.java 0.00% 148 Missing ⚠️
...requesthandler/MultiStageBrokerRequestHandler.java 0.00% 141 Missing ⚠️
...ache/pinot/core/query/reduce/GapfillProcessor.java 23.23% 61 Missing and 15 partials ⚠️
.../pinot/core/query/reduce/BaseGapfillProcessor.java 7.14% 37 Missing and 2 partials ⚠️
...ry/runtime/plan/server/ServerPlanRequestUtils.java 23.91% 28 Missing and 7 partials ⚠️
...he/pinot/query/runtime/plan/PlanNodeToOpChain.java 26.82% 25 Missing and 5 partials ⚠️
.../runtime/plan/server/ServerPlanRequestVisitor.java 28.94% 21 Missing and 6 partials ⚠️
...e/pinot/core/query/reduce/BrokerReduceService.java 16.66% 12 Missing and 3 partials ⚠️
...not/core/query/reduce/GapfillProcessorFactory.java 0.00% 8 Missing ⚠️
... and 4 more
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #17533      +/-   ##
============================================
- Coverage     63.21%   63.02%   -0.19%     
+ Complexity     1476     1471       -5     
============================================
  Files          3170     3171       +1     
  Lines        189508   190249     +741     
  Branches      28997    29209     +212     
============================================
+ Hits         119789   119900     +111     
- Misses        60417    61009     +592     
- Partials       9302     9340      +38     
Flag Coverage Δ
custom-integration1 ?
integration ?
integration1 ?
integration2 ?
java-11 62.98% <10.26%> (-0.21%) ⬇️
java-21 62.96% <10.26%> (+7.44%) ⬆️
temurin 63.02% <10.26%> (-0.19%) ⬇️
unittests 63.02% <10.26%> (-0.19%) ⬇️
unittests1 55.45% <16.05%> (-0.11%) ⬇️
unittests2 33.91% <0.92%> (-0.12%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@xiangfu0 xiangfu0 force-pushed the gapfill-v2-gapfill-test branch 2 times, most recently from a2424d6 to 1e297c3 Compare January 20, 2026 02:57
@xiangfu0 xiangfu0 force-pushed the gapfill-v2-gapfill-test branch from 1e297c3 to 6accd39 Compare January 20, 2026 05:11
@Jackie-Jiang
Copy link
Contributor

Can you add a section in the description about the motivation of this PR?
Do we need MSE support for GAP_FILL? Should it be modeled as window function?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants