Skip to content

Avoid a case where the planner was erroneously dropping predicates#3965

Open
alecgrieser wants to merge 4 commits intoFoundationDB:mainfrom
alecgrieser:03950-keep-range-constraints
Open

Avoid a case where the planner was erroneously dropping predicates#3965
alecgrieser wants to merge 4 commits intoFoundationDB:mainfrom
alecgrieser:03950-keep-range-constraints

Conversation

@alecgrieser
Copy link
Copy Markdown
Collaborator

There was a case in RangeConstraints.asComparisonRange where the process of merging different comparison ranges could result in predicates being dropped. The fundamental error was that there is a function, merge, which merges a comparison into a comparison range. However, if the comparison can't be successfully merged, then it will be added to a residual comparison list. But the asComparisonRange function was ignoring those predicates entirely.

This modifies the method so that it collects the ranges rather than dropping them. It then also modifies the compensation function so that if there are residual predicates, we are sure to generate a predicate for it.

This fixes #3950.

@alecgrieser alecgrieser added the bug fix Change that fixes a bug label Feb 20, 2026
@alecgrieser alecgrieser force-pushed the 03950-keep-range-constraints branch from 79c4b37 to 3911001 Compare February 20, 2026 18:21
@alecgrieser alecgrieser marked this pull request as ready for review February 23, 2026 12:23
@alecgrieser alecgrieser force-pushed the 03950-keep-range-constraints branch 3 times, most recently from 97d21e5 to ddd955b Compare February 23, 2026 14:05
@alecgrieser alecgrieser requested a review from hatyo February 23, 2026 14:12
… in the compaensation as needed

Validate behavior in new query test, as well as in more purpose built unit tests.
@alecgrieser alecgrieser force-pushed the 03950-keep-range-constraints branch from cbb9a3e to 5ade72f Compare March 2, 2026 12:05
@alecgrieser alecgrieser added the Run mixed-mode Label to add to Pull Requests to have it run mixed mode tests label Apr 10, 2026
@github-actions
Copy link
Copy Markdown

📊 Metrics Diff Analysis Report

Summary

  • New queries: 19
  • Dropped queries: 13
  • Plan changed + metrics changed: 16
  • Plan unchanged + metrics changed: 0
ℹ️ About this analysis

This automated analysis compares query planner metrics between the base branch and this PR. It categorizes changes into:

  • New queries: Queries added in this PR
  • Dropped queries: Queries removed in this PR. These should be reviewed to ensure we are not losing coverage.
  • Plan changed + metrics changed: The query plan has changed along with planner metrics.
  • Metrics only changed: Same plan but different metrics

The last category in particular may indicate planner regressions that should be investigated.

New Queries

Count of new queries by file:

  • yaml-tests/src/test/resources/join-tests.metrics.yaml: 18
  • yaml-tests/src/test/resources/sql-functions.metrics.yaml: 1

Dropped Queries

The following queries with metrics were removed:

The reviewer should double check that these queries were removed intentionally to avoid a loss of coverage.

Plan and Metrics Changed

These queries experienced both plan and metrics changes. This generally indicates that there was some planner change
that means the planning for this query may be substantially different. Some amount of query plan metrics change is expected,
but the reviewer should still validate that these changes are not excessive.

Total: 16 queries

Statistical Summary (Plan and Metrics Changed)

task_count:

  • Average change: +935.8
  • Average regression: +935.8
  • Median change: +1126
  • Median regression: +1126
  • Standard deviation: 927.8
  • Standard deviation of regressions: 927.8
  • Range: +20 to +3865
  • Range of regressions: +20 to +3865
  • Queries changed: 16
  • Queries regressed: 16

transform_count:

  • Average change: +184.4
  • Average regression: +184.4
  • Median change: +225
  • Median regression: +225
  • Standard deviation: 180.6
  • Standard deviation of regressions: 180.6
  • Range: +4 to +746
  • Range of regressions: +4 to +746
  • Queries changed: 16
  • Queries regressed: 16

transform_yield_count:

  • Average change: +67.6
  • Average regression: +67.6
  • Median change: +88
  • Median regression: +88
  • Standard deviation: 61.2
  • Standard deviation of regressions: 61.2
  • Range: +1 to +238
  • Range of regressions: +1 to +238
  • Queries changed: 16
  • Queries regressed: 16

insert_new_count:

  • Average change: +189.4
  • Average regression: +189.4
  • Median change: +220
  • Median regression: +220
  • Standard deviation: 208.4
  • Standard deviation of regressions: 208.4
  • Range: +3 to +884
  • Range of regressions: +3 to +884
  • Queries changed: 16
  • Queries regressed: 16

insert_reused_count:

  • Average change: +2.0
  • Average regression: +2.3
  • Median change: +2
  • Median regression: +2
  • Standard deviation: 2.1
  • Standard deviation of regressions: 2.0
  • Range: -1 to +8
  • Range of regressions: +1 to +8
  • Queries changed: 11
  • Queries regressed: 10

Significant Regressions (Plan and Metrics Changed)

There was 1 outlier detected. Outlier queries have a significant regression in at least one field. Statistically, this represents either an increase of more than two standard deviations above the mean or a large absolute increase (e.g., 100).

  • yaml-tests/src/test/resources/join-tests.metrics.yaml:428: EXPLAIN select dept.id, dept.name, project.name from emp, dept, project where emp.dept_id = dept.id and project.emp_id = emp.id and dept.id in (1, 2)
    • old explain: [IN arrayDistinct(promote(@c40 AS ARRAY(LONG)))] | INJOIN q0 -> { SCAN([IS DEPT, EQUALS q0]) } | FLATMAP q1 -> { SCAN([IS PROJECT]) | FLATMAP q2 -> { SCAN([IS EMP]) | FILTER _.DEPT_ID EQUALS q1.ID AND q2.EMP_ID EQUALS _.ID AS q3 RETURN q2 } AS q2 RETURN (q1.ID AS ID, q1.NAME AS _1, q2.NAME AS _2) }
    • new explain: [IN arrayDistinct(promote(@c40 AS ARRAY(LONG)))] | INJOIN q0 -> { SCAN([IS DEPT, EQUALS q0]) } | FLATMAP q1 -> { ISCAN(EMPDEPT [EQUALS q1.ID]) | FLATMAP q2 -> { SCAN([IS PROJECT]) | FILTER _.EMP_ID EQUALS q2.ID AS q3 RETURN q3 } AS q3 RETURN (q1.ID AS ID, q1.NAME AS _1, q3.NAME AS _2) }
    • task_count: 4963 -> 8828 (+3865)
    • transform_count: 1903 -> 2649 (+746)
    • transform_yield_count: 287 -> 525 (+238)
    • insert_new_count: 746 -> 1630 (+884)
    • insert_reused_count: 102 -> 110 (+8)

Minor Changes (Plan and Metrics Changed)

In addition, there were 15 queries with minor changes.

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

Labels

bug fix Change that fixes a bug Run mixed-mode Label to add to Pull Requests to have it run mixed mode tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RangeConstraints.asComparisonRange can result in predicates being lost

1 participant