Optimizer: Fix Merge In and Not Equal ranges #39676 #42229

ghazalfamilyusa · 2023-03-14T20:33:21Z

What problem does this PR solve?

Issue Number: ref #39676

Problem Summary:

Background

TiDB optimizer has limited predicate simplifications and does not support many cases. One example is filtering redundant conditions across <> and In for same column. For example, a <> 1 and a in (1,2,3) can be simplified to a in (2,3). Also, a <> 5 and a in (1,2,3) can be simplified to a in (1,2,3)

Solution

We added a new place holder logical rewrite optimization for predicate simplification. The general solution is
1- Process all predicates as a set of ranges for all columns
2- build equivalence classes for columns based col = col
3- Merge all ranges of each equivalence class
4- Build range predicates for each column in each equivalence class
This solution is on the optimizer roadmap and similar to what Teradata did https://docs.teradata.com/r/8mHBBLGP88~HK9Auie2QvQ/dinTQRLUmuWh_KUBr7ffdg

In this PR, we only do <> and In list merge to address the customer issue. The algorithm is simple and applied only to table scan predicates. The main idea is for each pair of conditions of a <> constant_0 and a in (constant_1, ... constant_n) we

Remove the a <> constant_0 since it is always reduntant
Remove constant_0 from the in list if any. We keep at least one element in the in list and postpone the (no table access) for later on.

Tests

Unit test

Side effects
N/A

Documentation

N/A

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

ti-chi-bot · 2023-03-14T20:33:23Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

fixdb
winoros

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

ghazalfamilyusa · 2023-03-15T00:37:15Z

/retest

fixdb

Other than the comments, looks good to me.

fixdb · 2023-03-15T21:33:08Z

planner/core/rule_predicate_simplification.go

+			if _, ok := args[1].(*expression.Constant); !ok {
+				return nil, otherPredicate
+			}
+			return col, notEqualPredicate


We don't need these predicateType enum, instead we can just use v.FuncName.L as the return value

The check is for more than the function name like col in (list of constants) and col <> constant.

fixdb · 2023-03-16T00:21:41Z

planner/core/rule_predicate_simplification.go

+	return newPred
+}
+
+func indexInSlice(index int, list []int) bool {


You can use slices.Contains by importing golang.org/x/exp/slices.

Sounds good. Will be done in next round.

winoros · 2023-03-16T02:00:32Z

planner/core/rule_predicate_simplification.go

+	inListPredicate   predicateType = 0x00
+	notEqualPredicate predicateType = 0x01
+	otherPredicate    predicateType = 0x02


Suggested change

inListPredicate predicateType = 0x00

notEqualPredicate predicateType = 0x01

otherPredicate predicateType = 0x02

inListPredicate predicateType = iota

notEqualPredicate

otherPredicate

In golang, this syntactic sugar should work

Sounds good. Will be done in next round.

ghazalfamilyusa · 2023-03-16T04:06:51Z

/retest

fixdb

+1

planner/core/rule_predicate_simplification.go

winoros · 2023-03-20T16:16:45Z

planner/core/casetest/testdata/predicate_simplification_out.json

+        "SQL": "select f from t use index() where f <> 3 and f in (1,2,3) and f <> 1 and f <> 2 -- Multiple <> values and cover whole inlist. We keep at least one in inlist",
+        "Plan": [
+          "TableReader 10.00 root  data:Selection",
+          "└─Selection 10.00 cop[tikv]  in(test.t.f, 2)",
+          "  └─TableFullScan 10000.00 cop[tikv] table:t keep order:false, stats:pseudo"
+        ]
+      },


We get the wrong result in this case? If we keep f in (2), we should also keep f != 2.

You are right and we need to keep both. Good catch. Fixed.

ghazalfamilyusa · 2023-03-20T18:05:36Z

/retest

winoros · 2023-03-21T04:40:50Z

/merge

ti-chi-bot · 2023-03-21T04:40:53Z

This pull request has been accepted and is ready to merge.

Commit hash: efc79eb

ti-chi-bot added do-not-merge/invalid-title do-not-merge/needs-linked-issue release-note-none size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 14, 2023

ghazalfamilyusa changed the title ~~Optimize:r Fix Merge In and Not Equal ranges #39676~~ Optimizer Fix Merge In and Not Equal ranges #39676 Mar 14, 2023

ghazalfamilyusa changed the title ~~Optimizer Fix Merge In and Not Equal ranges #39676~~ Optimizer: Fix Merge In and Not Equal ranges #39676 Mar 14, 2023

ti-chi-bot removed do-not-merge/invalid-title do-not-merge/needs-linked-issue labels Mar 14, 2023

ghazalfamilyusa marked this pull request as draft March 14, 2023 20:40

ti-chi-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 14, 2023

Add new file to bazal

b721a9f

ghazalfamilyusa force-pushed the predicate_simplification branch from 06c368c to b721a9f Compare March 14, 2023 21:00

ghazalfamilyusa marked this pull request as ready for review March 14, 2023 21:06

ti-chi-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 14, 2023

ghazalfamilyusa force-pushed the predicate_simplification branch 2 times, most recently from 6f52795 to f780f94 Compare March 14, 2023 21:39

fixdb reviewed Mar 16, 2023

View reviewed changes

winoros reviewed Mar 16, 2023

View reviewed changes

ghazalfamilyusa force-pushed the predicate_simplification branch from f780f94 to 70f3f94 Compare March 16, 2023 02:32

fixdb approved these changes Mar 16, 2023

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 16, 2023

winoros reviewed Mar 20, 2023

View reviewed changes

ghazalfamilyusa force-pushed the predicate_simplification branch from 70f3f94 to 9a968d1 Compare March 20, 2023 16:45

code review comment from Yiding

efc79eb

ghazalfamilyusa force-pushed the predicate_simplification branch from 9a968d1 to efc79eb Compare March 20, 2023 17:24

ti-chi-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 20, 2023

winoros approved these changes Mar 21, 2023

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Mar 21, 2023

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 21, 2023

ti-chi-bot merged commit b96c55d into pingcap:master Mar 21, 2023

qw4990 mentioned this pull request Apr 25, 2023

wrong result with plan cache #43405

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimizer: Fix Merge In and Not Equal ranges #39676 #42229

Optimizer: Fix Merge In and Not Equal ranges #39676 #42229

ghazalfamilyusa commented Mar 14, 2023 •

edited

ti-chi-bot commented Mar 14, 2023 •

edited

ghazalfamilyusa commented Mar 15, 2023

fixdb left a comment

fixdb Mar 15, 2023

ghazalfamilyusa Mar 16, 2023

fixdb Mar 16, 2023

ghazalfamilyusa Mar 16, 2023

winoros Mar 16, 2023

ghazalfamilyusa Mar 16, 2023

ghazalfamilyusa commented Mar 16, 2023

fixdb left a comment

winoros Mar 20, 2023

ghazalfamilyusa Mar 20, 2023

ghazalfamilyusa commented Mar 20, 2023

winoros commented Mar 21, 2023

ti-chi-bot commented Mar 21, 2023

Optimizer: Fix Merge In and Not Equal ranges #39676 #42229

Optimizer: Fix Merge In and Not Equal ranges #39676 #42229

Conversation

ghazalfamilyusa commented Mar 14, 2023 • edited

What problem does this PR solve?

Background

Solution

Release note

ti-chi-bot commented Mar 14, 2023 • edited

ghazalfamilyusa commented Mar 15, 2023

fixdb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghazalfamilyusa commented Mar 16, 2023

fixdb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghazalfamilyusa commented Mar 20, 2023

winoros commented Mar 21, 2023

ti-chi-bot commented Mar 21, 2023

ghazalfamilyusa commented Mar 14, 2023 •

edited

ti-chi-bot commented Mar 14, 2023 •

edited