Gen4: Filter primitive and planning #9017

frouioui · 2021-10-19T06:42:12Z

Description

This pull request adds the Filter engine primitive along with the filter logical plan. A basic version of the ComparisonExpr (implementation of EvalEngine's Expr) is added through this pull request too. ComparisonExpr supports basic operations (=, !=, <, <=, >, >=) on values that can be transformed to numerical values, i.e support for type coercion will come in the following pull request.

Related Issue(s)

Gen4 Tracking #7280

Checklist

Tests were added or are not required
Documentation was added or is not required

go/vt/vtgate/engine/filter.go

go/vt/vtgate/planbuilder/filter.go

go/vt/vtgate/planbuilder/rewrite.go

go/vt/sqlparser/expression_converter.go

go/test/endtoend/vtgate/utils/utils.go

go/vt/sqlparser/expression_converter.go

go/vt/vtgate/planbuilder/filter.go

go/vt/vtgate/planbuilder/horizon_planning.go

go/vt/vtgate/evalengine/arithmetic.go

systay

Nice stuff

go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go

go/vt/vtgate/planbuilder/system_tables.go

go/vt/vtgate/planbuilder/rewrite_test.go

go/vt/vtgate/planbuilder/filter.go

go/vt/vtgate/evalengine/arithmetic.go

go/vt/vtgate/evalengine/comparisons.go

go/vt/vtgate/engine/filter.go

systay · 2021-10-25T16:28:51Z

go/vt/vtgate/evalengine/comparisons.go

+		return 0, vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "cannot compare a date with a numeric value")
+
+	default:
+		// Quoting MySQL Docs:


go/vt/vtgate/evalengine/comparisons_test.go

…rther error analisis Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

…mports Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

…nbuilder package instead of ints Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

…on to an evalengine.expression Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

go/vt/vtgate/engine/route_test.go

go/vt/vtgate/planbuilder/filter.go

systay

Looks great. Just two minor comments that we could address in a future PR as well

frouioui · 2021-11-03T15:00:38Z

Looks great. Just two minor comments that we could address in a future PR as well

I will address the two comments via #9097

harshit-gangal · 2021-11-08T04:51:14Z

go/vt/vtgate/engine/ordered_aggregate_test.go

-		"d|null|1",
+		"d|1|2",


I see that you added a test row d|1|1 due to which you changed the test expectation from d|null|1 to d|1|2, any reason for doing this change in the test?

we wanted to test more cases

I agree with @harshit-gangal, naming is now confusing, in the test input we are saying // Single null even though we have two rows with only one of them being null. I'll fix the naming and add another row to test single null behavior.

harshit-gangal · 2021-11-08T05:05:17Z

go/vt/vtgate/evalengine/evalengine.go

+			now := time.Now()
+			// setting the date to today's date, because we use AddDate on t
+			// which is "0000-01-01 xx:xx:xx", we do minus one on the month
+			// and day to take into account the 01 in both month and day of t
+			t = t.AddDate(now.Year(), int(now.Month()-1), now.Day()-1)
+		}


do you have a test case for this? looks like there can be corner cases with timezone (set on MySQL vs VTGate instance)

what type of tests do you feel would be helpful here?

this is just an internal behavior of Golang where time.Times with no date have their dates set by default to 0000-01-01

filtering on column with time column on left and date column on right.
not sure if evalengine supports that, that would lead the test to come to this code path.

the EvalEngine supports it thanks to this code snippet you highlighted. we also have tests for it:

vitess/go/vt/vtgate/evalengine/comparisons_test.go

Lines 804 to 815 in 1152ecc

{

name: "date equal time",

v1: NewColumn(0, defaultCollation), v2: NewColumn(1, defaultCollation),

out: &T, op: &EqualOp{},

row: []sqltypes.Value{sqltypes.NewDate(time.Now().Format("2006-01-02")), sqltypes.NewTime("00:00:00")},

},

{

name: "date not equal time",

v1: NewColumn(0, defaultCollation), v2: NewColumn(1, defaultCollation),

out: &T, op: &NotEqualOp{},

row: []sqltypes.Value{sqltypes.NewDate(time.Now().Format("2006-01-02")), sqltypes.NewTime("12:00:00")},

},

frouioui added Type: Enhancement Logical improvement (somewhere between a bug and feature) Component: Query Serving release notes labels Oct 19, 2021

frouioui force-pushed the gen4-filter-planning branch from c33f548 to 40b6e2b Compare October 19, 2021 07:20

frouioui self-assigned this Oct 20, 2021

systay reviewed Oct 20, 2021

View reviewed changes

go/vt/vtgate/engine/filter.go Outdated Show resolved Hide resolved

systay reviewed Oct 20, 2021

View reviewed changes

go/vt/vtgate/planbuilder/filter.go Outdated Show resolved Hide resolved

systay reviewed Oct 20, 2021

View reviewed changes

go/vt/vtgate/planbuilder/rewrite.go Outdated Show resolved Hide resolved

frouioui marked this pull request as ready for review October 20, 2021 09:11

frouioui requested review from GuptaManan100 and harshit-gangal as code owners October 20, 2021 09:11

frouioui marked this pull request as draft October 20, 2021 09:15

frouioui marked this pull request as ready for review October 20, 2021 09:21

frouioui requested a review from systay October 20, 2021 09:21

systay reviewed Oct 20, 2021

View reviewed changes

go/vt/sqlparser/expression_converter.go Outdated Show resolved Hide resolved

GuptaManan100 approved these changes Oct 20, 2021

View reviewed changes

go/test/endtoend/vtgate/utils/utils.go Outdated Show resolved Hide resolved

go/vt/sqlparser/expression_converter.go Outdated Show resolved Hide resolved

go/vt/vtgate/planbuilder/filter.go Outdated Show resolved Hide resolved

go/vt/vtgate/planbuilder/horizon_planning.go Show resolved Hide resolved

systay reviewed Oct 20, 2021

View reviewed changes

go/vt/vtgate/evalengine/arithmetic.go Outdated Show resolved Hide resolved

systay approved these changes Oct 20, 2021

View reviewed changes

frouioui marked this pull request as draft October 21, 2021 08:28

harshit-gangal reviewed Oct 21, 2021

View reviewed changes

frouioui force-pushed the gen4-filter-planning branch 4 times, most recently from 1309eb0 to af16294 Compare October 25, 2021 15:51

systay reviewed Oct 25, 2021

View reviewed changes

go/vt/vtgate/evalengine/comparisons_test.go Outdated Show resolved Hide resolved

systay reviewed Oct 25, 2021

View reviewed changes

go/vt/vtgate/evalengine/comparisons_test.go Outdated Show resolved Hide resolved

systay reviewed Oct 25, 2021

View reviewed changes

go/vt/vtgate/evalengine/comparisons_test.go Show resolved Hide resolved

frouioui force-pushed the gen4-filter-planning branch from 094e9a1 to e5ef8a3 Compare October 26, 2021 08:00

frouioui force-pushed the gen4-filter-planning branch from 00ebb2c to ec94419 Compare November 3, 2021 07:06

frouioui added 21 commits November 3, 2021 12:21

Use of ErrConvertExprNotSupported string in Convert error to allow fu…

f643574

…rther error analisis Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

Addition of executeComparison to respect MySQL type conversion

5ae4bc3

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

initial addition of all the different comparison cases

4cac02f

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

support for decimal comparisons

d96ddd1

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

addition of test matrix for the decimal case

07ab21f

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

addition of test matrices for numerical comparisons

b322206

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

initial addition of date comparisons

321c03f

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

Error cases for dates

bd4e3cc

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

Compare string with dates

2c26e26

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

E2E and unit tests for bind variables comparisons with dates

6af387f

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

failing comparisons with hexadecimals

9e26f89

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

store collation informations in EvalEngine exprs

830c37a

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

Addition of FakeConverter struct where needed

665044b

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

Compare strings using collation in the evalengine and remove cyclic i…

f0f59d2

…mports Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

Use of collations.ID and collations.Unknown in the evalengine and pla…

e95f823

…nbuilder package instead of ints Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

Add better collation error check when converting a sqlparser.expressi…

f464dca

…on to an evalengine.expression Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

allow string convertion without a collation

40b27df

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

applied harshit's review suggestions

2dd4be0

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

fixed sum arithmetic with null/decimal values

d0c2ee2

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

refactor of the data out of range errors in arithmetic

f549608

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

simplified OA plan order by switch case

1268684

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

frouioui force-pushed the gen4-filter-planning branch from 248aa2f to 1268684 Compare November 3, 2021 11:21

systay marked this pull request as ready for review November 3, 2021 12:33

systay reviewed Nov 3, 2021

View reviewed changes

go/vt/vtgate/engine/route_test.go Show resolved Hide resolved

systay reviewed Nov 3, 2021

View reviewed changes

go/vt/vtgate/planbuilder/filter.go Show resolved Hide resolved

systay approved these changes Nov 3, 2021

View reviewed changes

frouioui merged commit 1b52453 into vitessio:main Nov 3, 2021

frouioui deleted the gen4-filter-planning branch November 3, 2021 15:01

harshit-gangal reviewed Nov 8, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gen4: Filter primitive and planning #9017

Gen4: Filter primitive and planning #9017

frouioui commented Oct 19, 2021 •

edited

Loading

systay left a comment

systay Oct 25, 2021

systay left a comment

frouioui commented Nov 3, 2021

harshit-gangal Nov 8, 2021

systay Nov 8, 2021

frouioui Nov 8, 2021

harshit-gangal Nov 8, 2021

systay Nov 8, 2021

frouioui Nov 8, 2021

harshit-gangal Nov 9, 2021

frouioui Nov 9, 2021

	{
	name: "date equal time",
	v1: NewColumn(0, defaultCollation), v2: NewColumn(1, defaultCollation),
	out: &T, op: &EqualOp{},
	row: []sqltypes.Value{sqltypes.NewDate(time.Now().Format("2006-01-02")), sqltypes.NewTime("00:00:00")},
	},
	{
	name: "date not equal time",
	v1: NewColumn(0, defaultCollation), v2: NewColumn(1, defaultCollation),
	out: &T, op: &NotEqualOp{},
	row: []sqltypes.Value{sqltypes.NewDate(time.Now().Format("2006-01-02")), sqltypes.NewTime("12:00:00")},
	},

Gen4: Filter primitive and planning #9017

Gen4: Filter primitive and planning #9017

Conversation

frouioui commented Oct 19, 2021 • edited Loading

Description

Related Issue(s)

Checklist

systay left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

systay left a comment

Choose a reason for hiding this comment

frouioui commented Nov 3, 2021

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

Choose a reason for hiding this comment

frouioui commented Oct 19, 2021 •

edited

Loading