Skip to content

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

Open
yashmayya wants to merge 1 commit into
apache:masterfrom
yashmayya:fix-explain-v2-flaky-time-mask
Open

Fix flaky testExplainPlanQueryV2 by masking scientific-notation rule timings#18650
yashmayya wants to merge 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 16 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 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.

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.

2 participants