-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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] Implemented table alias functionality in the semantic analysis #7629
Conversation
a68f4fa
to
4239473
Compare
4239473
to
fa60cbb
Compare
fa60cbb
to
c61f7e2
Compare
c61f7e2
to
8b6e836
Compare
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
619a11d
to
18e61f0
Compare
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
cfcb83a
to
481e317
Compare
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
keyspace, err := vschema.DefaultKeyspace() | ||
if err != nil { | ||
return nil, err | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will start failing for cases when there is no default database selected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmm... how come no tests are failing? do we need to add more tests to the planner?
you are probably absolutely correct about this comment. that being said - since this is not failing any tests, how about we take care of this in a separate PR? We should add more tests, and then also fix the issue
WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we are introducing this check here, so it be be better to have a test that covers the error case if existing tests do not fail.
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
b13224c
to
18036f5
Compare
CI tests are failing. |
Signed-off-by: Andres Taylor <andres@planetscale.com>
Description
This PR teaches the semantic analysis how to deal with table aliases.
Every table mentioned in the
FROM
will now either be explicitly qualified with the database name, or the current database is used to qualify the table.This PR also adds support for routed tables, which depend on table aliases being available.
Checklist
Nope
Lots of tests added
Not required
Deployment Notes
Impacted Areas in Vitess
Components that this PR will affect: