Skip to content

mysql: handle db name correctly#5398

Merged
sougou merged 1 commit intovitessio:masterfrom
planetscale:ss-use
Nov 3, 2019
Merged

mysql: handle db name correctly#5398
sougou merged 1 commit intovitessio:masterfrom
planetscale:ss-use

Conversation

@sougou
Copy link
Contributor

@sougou sougou commented Nov 3, 2019

Fixes #5206

The conn schemaname var can get set through a new connection
or a ComInitDB packet. However, a use statement bypasses this
mechanism and gets handled at the vtgate level. This causes
problems because schemaName in the connection disagrees with
TargetString in vtgate.

To fix this, the new scheme calls a new handler function: ComInitDB
every time schemaName is initialized or changed in the connection.
This way, the TargetString remains the authoritative source
for the current db name.

I couldn't write a test for this specific behavior change because
the mysql go client does not implement support for ComInitDB.
But I've verified the things generally work as expected.

Signed-off-by: Sugu Sougoumarane ssougou@gmail.com

@sougou sougou requested review from deepthi, morgo and tirsen November 3, 2019 10:36
Fixes vitessio#5206

The conn schemaname var can get set through a new connection
or a ComInitDB packet. However, a use statement bypasses this
mechanism and gets handled at the vtgate level. This causes
problems because schemaName in the connection disagrees with
TargetString in vtgate.

To fix this, the new scheme calls a new handler function: ComInitDB
every time schemaName is initialized or changed in the connection.
This way, the TargetString remains the authoritative source
for the current db name.

I couldn't write a test for this specific behavior change because
the mysql go client does not implement support for ComInitDB.
But I've verified the things generally work as expected.

Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@morgo
Copy link
Contributor

morgo commented Nov 3, 2019

I managed to manually verify this as fixed, by using local example. Here is the previous behavior:

morgo@morgox1:~/vitess/src/vitess.io/vitess$ mysql
Welcome to the MySQL monitor.  Commands end with ; or \g.
Your MySQL connection id is 27
Server version: 5.5.10-Vitess (Ubuntu)

Copyright (c) 2000, 2019, Oracle and/or its affiliates. All rights reserved.

Oracle is a registered trademark of Oracle Corporation and/or its
affiliates. Other names may be trademarks of their respective
owners.

Type 'help;' or '\h' for help. Type '\c' to clear the current input statement.

mysql> show databases;
+-----------+
| Databases |
+-----------+
| commerce  |
| customer  |
+-----------+
2 rows in set (0.00 sec)

mysql> use commerce;
Reading table information for completion of table and column names
You can turn off this feature to get a quicker startup with -A

Database changed
mysql> show tables;
+-----------------------+
| Tables_in_vt_commerce |
+-----------------------+
| customer_seq          |
| order_seq             |
| product               |
+-----------------------+
3 rows in set (0.01 sec)

mysql> use customer;
Reading table information for completion of table and column names
You can turn off this feature to get a quicker startup with -A

Database changed
mysql> show tables;
+-----------------------+
| Tables_in_vt_commerce |
+-----------------------+
| customer_seq          |
| order_seq             |
| product               |
+-----------------------+
3 rows in set (0.00 sec)

mysql> use customer;
Reading table information for completion of table and column names
You can turn off this feature to get a quicker startup with -A

Database changed
mysql> show tables;
+-----------------------+
| Tables_in_vt_commerce |
+-----------------------+
| customer_seq          |
| order_seq             |
| product               |
+-----------------------+
3 rows in set (0.00 sec)

mysql> use commerce;
Reading table information for completion of table and column names
You can turn off this feature to get a quicker startup with -A

Database changed
mysql> show tables;
+-----------------------+
| Tables_in_vt_commerce |
+-----------------------+
| customer_seq          |
| order_seq             |
| product               |
+-----------------------+
3 rows in set (0.01 sec)

Here is the new behavior:

morgo@morgox1:~/vitess/src/vitess.io/vitess$ mysql
Welcome to the MySQL monitor.  Commands end with ; or \g.
Your MySQL connection id is 29
Server version: 5.5.10-Vitess (Ubuntu)

Copyright (c) 2000, 2019, Oracle and/or its affiliates. All rights reserved.

Oracle is a registered trademark of Oracle Corporation and/or its
affiliates. Other names may be trademarks of their respective
owners.

Type 'help;' or '\h' for help. Type '\c' to clear the current input statement.

mysql> show databases;
+-----------+
| Databases |
+-----------+
| commerce  |
| customer  |
+-----------+
2 rows in set (0.01 sec)

mysql> use commerce;
Reading table information for completion of table and column names
You can turn off this feature to get a quicker startup with -A

Database changed
mysql> show tables;
+-----------------------+
| Tables_in_vt_commerce |
+-----------------------+
| customer_seq          |
| order_seq             |
| product               |
+-----------------------+
3 rows in set (0.00 sec)

mysql> use customer;
Reading table information for completion of table and column names
You can turn off this feature to get a quicker startup with -A

Database changed
mysql> show tables;
+-----------------------+
| Tables_in_vt_customer |
+-----------------------+
| corder                |
| customer              |
+-----------------------+
2 rows in set (0.00 sec)

mysql> use customer;
Reading table information for completion of table and column names
You can turn off this feature to get a quicker startup with -A

Database changed
mysql> show tables;
+-----------------------+
| Tables_in_vt_customer |
+-----------------------+
| corder                |
| customer              |
+-----------------------+
2 rows in set (0.00 sec)

mysql> use commerce;
Reading table information for completion of table and column names
You can turn off this feature to get a quicker startup with -A

Database changed
mysql> show tables;
+-----------------------+
| Tables_in_vt_commerce |
+-----------------------+
| customer_seq          |
| order_seq             |
| product               |
+-----------------------+
3 rows in set (0.01 sec)

@morgo
Copy link
Contributor

morgo commented Nov 3, 2019

@sougou LGTM except the resharding_rbr test is failing. Did you want to bump the CI?

Copy link
Collaborator

@deepthi deepthi left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"use database" handling in mysql CLI client broken

3 participants