Skip to content

Fix flaky testExplainPlanQueryV2 by masking scientific-notation rule timings#18650

Merged
xiangfu0 merged 1 commit into
apache:masterfrom
yashmayya:fix-explain-v2-flaky-time-mask
Jun 6, 2026
Merged

Fix flaky testExplainPlanQueryV2 by masking scientific-notation rule timings#18650
xiangfu0 merged 1 commit into
apache:masterfrom
yashmayya:fix-explain-v2-flaky-time-mask

Conversation

@yashmayya
Copy link
Copy Markdown
Contributor

Problem

OfflineClusterIntegrationTest#testExplainPlanQueryV2 is intermittently flaky. It compares the EXPLAIN output's "Rule Execution Times" section after masking each per-rule time with *, using the regex Time: \d+\.\d+.

Rule timings are rendered as durationNanos / 1_000_000.0 (a double) via Double.toString() in RuleTimingPlannerListener. When a rule runs fast enough (sub-microsecond, i.e. < 0.001 ms), Double.toString() switches to scientific notation such as 1.5E-4. The old regex matches only the 1.5 mantissa and leaves the exponent behind, producing Time:*E-4, which no longer matches the expected Time:*:

expected: Rule: AggregateProjectPullUpConstants -> Time:*
found:    Rule: AggregateProjectPullUpConstants -> Time:*E-4

The rule list itself is correct/unchanged — only the time-masking is wrong. This is a pre-existing flake, unrelated to any Calcite version.

Fix

Extend the masking regex to also strip an optional scientific-notation exponent, so both 1.5 and 1.5E-4 collapse to *:

Time: \d+\.\d+   ->   Time: \d+\.\d+(?:[eE][-+]?\d+)?

Applied to both occurrences in the test (the two responses asserted in the method).

Testing

./mvnw -pl pinot-integration-tests test -Dtest=OfflineClusterIntegrationTest#testExplainPlanQueryV2 -Dsurefire.failIfNoSpecifiedTests=false passes.

@yashmayya yashmayya added the flaky-test Tracks a test that intermittently fails label Jun 2, 2026
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.40%. Comparing base (f6b930b) to head (c627060).
⚠️ Report is 49 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #18650      +/-   ##
============================================
- Coverage     64.45%   64.40%   -0.05%     
- Complexity     1282     1291       +9     
============================================
  Files          3352     3365      +13     
  Lines        207171   208066     +895     
  Branches      32348    32482     +134     
============================================
+ Hits         133534   134007     +473     
- Misses        62910    63283     +373     
- Partials      10727    10776      +49     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-21 64.40% <ø> (-0.05%) ⬇️
temurin 64.40% <ø> (-0.05%) ⬇️
unittests 64.40% <ø> (-0.05%) ⬇️
unittests1 56.81% <ø> (+<0.01%) ⬆️
unittests2 37.13% <ø> (-0.03%) ⬇️

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

☔ View full report in Codecov by Harness.
📢 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 merged commit efb8b80 into apache:master Jun 6, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flaky-test Tracks a test that intermittently fails

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants