Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gen4: extracting routing information from predicates for system tables #8480

Merged
merged 9 commits into from
Jul 20, 2021

Conversation

harshit-gangal
Copy link
Member

@harshit-gangal harshit-gangal commented Jul 15, 2021

Description

Add support for information_schema queries in Gen4. This also works if CNF rewriting is required.

Related Issue(s)

#7280

Checklist

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

Deployment Notes

@harshit-gangal harshit-gangal added Component: Query Serving Type: Enhancement Logical improvement (somewhere between a bug and feature) labels Jul 15, 2021
…ed all system tables plan tests into its own file

Signed-off-by: Harshit Gangal <[email protected]>
…e them to validate that all the tables points to same keyspace routing

Signed-off-by: Harshit Gangal <[email protected]>
…ault keyspace is not set

Signed-off-by: Harshit Gangal <[email protected]>
@harshit-gangal harshit-gangal marked this pull request as ready for review July 19, 2021 10:19
@harshit-gangal harshit-gangal requested a review from systay as a code owner July 19, 2021 10:19
Signed-off-by: Harshit Gangal <[email protected]>
Copy link
Member

@frouioui frouioui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a nit. Let's double-approve with @systay though

Comment on lines +472 to +475
ksName := ""
if vschema.keyspace != nil {
ksName = vschema.keyspace.Name
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably extract this. We use the same snippet in three different places

…re not information_schema queries

Signed-off-by: Harshit Gangal <[email protected]>
Signed-off-by: Harshit Gangal <[email protected]>
@@ -95,6 +97,10 @@ type (

// columns needed to feed other plans`
columns []*sqlparser.ColName

// The following two fields are used when routing information_schema queries
SysTableTableSchema []evalengine.Expr
Copy link
Collaborator

@systay systay Jul 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm... we are using sqlparser.Expr for predicates here. feels icky to mix both parser stuff and evalengine stuff in the same struct. would it make sense to keep this as AST elements and turn them into evalengine thingies in the jointree_transformers?

@harshit-gangal
Copy link
Member Author

Will address review comments in following PR

@harshit-gangal harshit-gangal merged commit b472ac6 into vitessio:main Jul 20, 2021
@harshit-gangal harshit-gangal deleted the gen4-dba-tables branch July 20, 2021 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Query Serving Type: Enhancement Logical improvement (somewhere between a bug and feature)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants