[planner] Schema information on the information_schema views by systay · Pull Request #11941 · vitessio/vitess

systay · 2022-12-12T10:15:12Z

Description

This makes it so that Vitess knows the columns and types of all information_schema views, which can then be used during semantic analysis.

While working on this I also found and fixed an issue around resolving of derived table columns with different cases.

Related Issue(s)

Fixes #11755
Fixes #11942

Checklist

"Backport to:" labels have been added if this change should be back-ported
Tests were added or are not required
Documentation was added or is not required

vitess-bot · 2022-12-12T10:15:15Z

Signed-off-by: Andres Taylor <andres@planetscale.com>

systay · 2022-12-12T14:52:45Z

go/vt/vtgate/planbuilder/testdata/unsupported_cases.json

this query didn't make much sense to me, so I removed it. The problem is with in (select * - IN works against a single column. I think we acceptable coverage of star expansion on system tables even without this query

Signed-off-by: Andres Taylor <andres@planetscale.com>

arthurschreiber · 2022-12-12T15:15:01Z

go/vt/vtgate/planbuilder/testdata/systemtables_cases.json

@@ -208,6 +190,30 @@
      "Original": "select * from information_schema.tables where table_schema = 'user' union select * from information_schema.tables where table_schema = 'main'",


This reminds me that we could look into optimizing UNION queries to not always split them up (e.g. in cases we will end up sending them to the same shard anyway).

yeah, we need to move all horizon planning over to the operators.
I have to finish this: #11698

go/vt/vtgate/planbuilder/testdata/systemtables_cases.json

Signed-off-by: Andres Taylor <andres@planetscale.com>

go/vt/vtgate/semantics/info_schema.go

frouioui · 2022-12-12T15:28:32Z

go/vt/vtgate/semantics/info_schema.go

+func getInfoSchema() map[string]map[string]query.Type {
+	infSchema := map[string]map[string]query.Type{}


nit: maybe adding a link to the doc could help to know what each table/column is used for: https://dev.mysql.com/doc/refman/8.0/en/information-schema-table-reference.html

go/vt/vtgate/semantics/info_schema_gen_test.go

systay · 2022-12-12T15:35:25Z

go/vt/vtgate/planbuilder/testdata/union_cases.json

    }
+  },
+  {
+    "comment": "systable union query in derived table with constraint on outside (star projection)",


this query was moved here from unsupported_cases.json

go/vt/vtgate/semantics/info_schema.go

go/vt/vtgate/semantics/info_schema_gen_test.go

go/vt/vtgate/semantics/info_schema.go

Signed-off-by: Andres Taylor <andres@planetscale.com>

systay · 2022-12-13T06:45:43Z

go/vt/vtgate/planbuilder/testdata/systemtables_cases57.json

+    }
+  },
+  {
+    "comment": "rails_query 6",


here we should have rails query 5, but it uses an info_schema view that did not exist in 5.7 information_schema.check_constraints. There were two more queries in this file that I had to delete because they used this view

arthurschreiber · 2022-12-13T08:23:38Z

go/vt/vtgate/semantics/derived_table.go

 	directDeps := org.tableSetFor(dt.ASTNode)
 	for i, name := range dt.columnNames {
-		if name != colName {
+		if !strings.EqualFold(name, colName) {


Is this fixing a potential bug / does this need to be backported? 🤔

It fixes #11942

Yeah, we could backport just this change. We already have a test that exposes the problem in the plan tests. That's how I found it

Signed-off-by: Andres Taylor <andres@planetscale.com>

harshit-gangal · 2022-12-14T18:24:48Z

go/vt/vtgate/semantics/table_collector.go

+		var err error
+		tbl, vindex, _, _, _, err = tc.si.FindTableOrVindex(t)
+		if err != nil {
+			return err
 		}
+		if tbl == nil && vindex != nil {
+			tbl = newVindexTable(t.Name)
+		}
+


this looks like a logic change, where we do FindTableOrVindex even if isInfSchema is true

yeah, this is by design. tc.si now knows about information_schema views, so we want to ask it about these

I see now, Thank you

…o#11941) * add info_schema information Signed-off-by: Andres Taylor <andres@planetscale.com> * add SchemaInformation handling for info_schema tables Signed-off-by: Andres Taylor <andres@planetscale.com> * fix bad test query Signed-off-by: Andres Taylor <andres@planetscale.com> * add support for information_schema on mysql 5.7 Signed-off-by: Andres Taylor <andres@planetscale.com> * columns sorted just like mysql, and tests for 5.7 Signed-off-by: Andres Taylor <andres@planetscale.com> * test: skip test that should not run Signed-off-by: Andres Taylor <andres@planetscale.com> Signed-off-by: Andres Taylor <andres@planetscale.com>

* [planner] Schema information on the information_schema views (#11941) * add info_schema information Signed-off-by: Andres Taylor <andres@planetscale.com> * add SchemaInformation handling for info_schema tables Signed-off-by: Andres Taylor <andres@planetscale.com> * fix bad test query Signed-off-by: Andres Taylor <andres@planetscale.com> * add support for information_schema on mysql 5.7 Signed-off-by: Andres Taylor <andres@planetscale.com> * columns sorted just like mysql, and tests for 5.7 Signed-off-by: Andres Taylor <andres@planetscale.com> * test: skip test that should not run Signed-off-by: Andres Taylor <andres@planetscale.com> Signed-off-by: Andres Taylor <andres@planetscale.com> * Fix for USING when column names not lower cased (#12379) Signed-off-by: Andres Taylor <andres@planetscale.com> --------- Signed-off-by: Andres Taylor <andres@planetscale.com>

systay added Type: Enhancement Logical improvement (somewhere between a bug and feature) Component: Query Serving labels Dec 12, 2022

add info_schema information

051d47c

Signed-off-by: Andres Taylor <andres@planetscale.com>

systay force-pushed the info_schema_tracking branch from 48dc452 to 027f00e Compare December 12, 2022 14:48

systay commented Dec 12, 2022

View reviewed changes

systay marked this pull request as ready for review December 12, 2022 14:54

systay requested review from GuptaManan100, arthurschreiber, frouioui and harshit-gangal as code owners December 12, 2022 14:54

add SchemaInformation handling for info_schema tables

fed492d

Signed-off-by: Andres Taylor <andres@planetscale.com>

systay force-pushed the info_schema_tracking branch from 027f00e to fed492d Compare December 12, 2022 14:56

arthurschreiber reviewed Dec 12, 2022

View reviewed changes

frouioui reviewed Dec 12, 2022

View reviewed changes

go/vt/vtgate/planbuilder/testdata/systemtables_cases.json Show resolved Hide resolved

fix bad test query

f955b95

Signed-off-by: Andres Taylor <andres@planetscale.com>

frouioui reviewed Dec 12, 2022

View reviewed changes

go/vt/vtgate/semantics/info_schema.go Outdated Show resolved Hide resolved

frouioui reviewed Dec 12, 2022

View reviewed changes

go/vt/vtgate/semantics/info_schema_gen_test.go Show resolved Hide resolved

systay commented Dec 12, 2022

View reviewed changes

frouioui reviewed Dec 12, 2022

View reviewed changes

go/vt/vtgate/semantics/info_schema.go Show resolved Hide resolved

arthurschreiber reviewed Dec 12, 2022

View reviewed changes

go/vt/vtgate/semantics/info_schema_gen_test.go Outdated Show resolved Hide resolved

frouioui reviewed Dec 12, 2022

View reviewed changes

go/vt/vtgate/semantics/info_schema.go Outdated Show resolved Hide resolved

systay added 2 commits December 12, 2022 17:01

add support for information_schema on mysql 5.7

d2e3228

Signed-off-by: Andres Taylor <andres@planetscale.com>

columns sorted just like mysql, and tests for 5.7

ffd5000

Signed-off-by: Andres Taylor <andres@planetscale.com>

systay commented Dec 13, 2022

View reviewed changes

frouioui approved these changes Dec 13, 2022

View reviewed changes

arthurschreiber approved these changes Dec 13, 2022

View reviewed changes

test: skip test that should not run

564cfb1

Signed-off-by: Andres Taylor <andres@planetscale.com>

systay merged commit 397fbe2 into vitessio:main Dec 14, 2022

systay deleted the info_schema_tracking branch December 14, 2022 07:15

harshit-gangal reviewed Dec 14, 2022

View reviewed changes

deepthi mentioned this pull request Dec 16, 2022

Table GC: rely on tm state to determine operation mode #11972

Merged

3 tasks

systay mentioned this pull request Jan 27, 2023

handle system databases other that information_schema correctly #12175

Merged

3 tasks

systay mentioned this pull request Mar 2, 2023

[bug fix] USING planning on information_schema #12542

Merged

		@@ -208,6 +190,30 @@
		"Original": "select * from information_schema.tables where table_schema = 'user' union select * from information_schema.tables where table_schema = 'main'",

		func getInfoSchema() map[string]map[string]query.Type {
		infSchema := map[string]map[string]query.Type{}

Conversation

systay commented Dec 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue(s)

Checklist

Uh oh!

vitess-bot bot commented Dec 12, 2022

Review Checklist

General

If a new flag is being introduced:

If a workflow is added or modified:

Bug fixes

Non-trivial changes

New/Existing features

Backward compatibility

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

systay Dec 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

systay commented Dec 12, 2022 •

edited

Loading

systay Dec 12, 2022 •

edited

Loading