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

Correct MySQL Shell/Client section #8779

Merged
merged 1 commit into from
Oct 21, 2022
Merged

Conversation

dveeden
Copy link
Contributor

@dveeden dveeden commented Jun 1, 2022

What is changed, added or deleted? (Required)

There are two official commandline clients for MySQL, both opensource and released by the Oracle MySQL team.

  1. MySQL Client (mysql), which is part of the MySQL Server code base, but can be installed separately via yum install mysql on most YUM based linux distributions. With the offical YUM repo this becomes yum install mysql-community-client. This client is written in C and has existed for as long as MySQL does.

  2. MySQL Shell (mysqlsh), which is a separate code base and can be installed separately. This is a newer tool and supports SQL, but also JS and Python. It has a more modern code base and is more extendable than the 'old' client.

It looks like these two were mixed up into a single section of the docs.

Which TiDB version(s) do your changes apply to? (Required)

  • master (the latest development version)
  • v6.1 (TiDB 6.1 versions)
  • v6.0 (TiDB 6.0 versions)
  • v5.4 (TiDB 5.4 versions)
  • v5.3 (TiDB 5.3 versions)
  • v5.2 (TiDB 5.2 versions)
  • v5.1 (TiDB 5.1 versions)
  • v5.0 (TiDB 5.0 versions)
  • v4.0 (TiDB 4.0 versions)

Do your changes match any of the following descriptions?

  • Delete files
  • Change aliases
  • Need modification after applied to another branch
  • Might cause conflicts after applied to another branch

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Jun 1, 2022

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • Icemap
  • Oreoxmt

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@ti-chi-bot ti-chi-bot added missing-translation-status This PR does not have translation status info. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 1, 2022
@dveeden
Copy link
Contributor Author

dveeden commented Jun 1, 2022

The removal of this note is intentional:

> **Note:**
>
> The MySQL Shell earlier than version 5.7.7 clears [Optimizer Hints](/optimizer-hints.md#optimizer-hints) by default. If you need to use the Hint syntax in an earlier MySQL Shell version, add the `--comments` option when starting the client.

While MySQL Client (not Shell) stops to strip out optimizer hints, it continues to strip out TiDB specific comments like /*T![clustered_index] NONCLUSTERED */ that is used in the output of SHOW CREATE TABLE. So --comments should always be used with MySQL Client. This is not needed for MySQL Shell as MySQL Shell doesn't strip out comments.

History note:
The query cache in MySQL (now removed) used to cache resultsets of full queries. This only worked with an exact match, including uppercase/lowercase and comments. By stripping out comments this was more likely to work and also somewhat reduced network traffic. With MyISAM replacing InnoDB this is not really needed anymore as the InnoDB Buffer Pool does a reasonable job of caching without the same drawbacks. And for the cases that really need a query cache there is the ProxySQL Query Cache that is doing a better job than the original query cache and works better in a distributed setup.

@dveeden
Copy link
Contributor Author

dveeden commented Jun 1, 2022

Also note that MySQL Shell supports both the "Classic" MySQL protocol and "X Protocol". TiDB doesn't support X Protocol. Using mysql:// URL's instead of mysqlx:// is needed for this to work correctly.

@dveeden dveeden requested review from qiancai and TomShawn June 1, 2022 06:37
@dveeden
Copy link
Contributor Author

dveeden commented Jun 1, 2022

Example sessions:

$ mysql --comments -u root -p -h 127.0.0.1 -P 4000
Enter password: 
Welcome to the MySQL monitor.  Commands end with ; or \g.
Your MySQL connection id is 427
Server version: 5.7.25-TiDB-v6.2.0-alpha TiDB Server (Apache License 2.0) Community Edition, MySQL 5.7 compatible

Copyright (c) 2000, 2022, Oracle and/or its affiliates.

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> ^DBye
$ mysqlsh --sql mysql://root@127.0.0.1:4000
Please provide the password for 'root@127.0.0.1:4000': ****
Save password for 'root@127.0.0.1:4000'? [Y]es/[N]o/Ne[v]er (default No): 
MySQL Shell 8.0.29

Copyright (c) 2016, 2022, Oracle and/or its affiliates.
Oracle is a registered trademark of Oracle Corporation and/or its affiliates.
Other names may be trademarks of their respective owners.

Type '\help' or '\?' for help; '\quit' to exit.
Creating a Classic session to 'root@127.0.0.1:4000'
Fetching schema names for autocompletion... Press ^C to stop.
Your MySQL connection id is 429
Server version: 5.7.25-TiDB-v6.2.0-alpha TiDB Server (Apache License 2.0) Community Edition, MySQL 5.7 compatible
No default schema selected; type \use <schema> to set one.
 MySQL  127.0.0.1:4000  SQL > 
Bye!

@shichun-0415 shichun-0415 requested review from Oreoxmt and Icemap and removed request for TomShawn and shichun-0415 June 1, 2022 12:43
@shichun-0415 shichun-0415 added translation/doing This PR's assignee is translating this PR. area/develop This PR relates to the area of TiDB App development. and removed missing-translation-status This PR does not have translation status info. labels Jun 1, 2022
@Icemap
Copy link
Member

Icemap commented Jun 2, 2022

Hi, @dveeden. We realized that mysql-client or mysql-shell is not a good choice, so we recommend mycli here: pingcap/docs-cn#9643 instead. Is this a good idea? We will sync to the English version later.

@dveeden
Copy link
Contributor Author

dveeden commented Jun 7, 2022

Hi, @dveeden. We realized that mysql-client or mysql-shell is not a good choice, so we recommend mycli here: pingcap/docs-cn#9643 instead. Is this a good idea? We will sync to the English version later.

@Icemap Why isn't MySQL Client or MySQL Shell a good choice? Those are the "official" clients from Oracle MySQL that people that come to TiDB are likely to know already. The MariaDB Client is a fork of the MySQL Client and mostly behaves the same, the biggest difference is how the SSL/TLS options are set.

Side note: tiup client and the TiDB Cloud WebShell uses https://github.com/xo/usql which often seems to confuse users as it tries to mimic the behavior of the PostgreSQL Client (psql).

I think mycli is a good client. Some time ago I fixed a few compatibility issues: dbcli/mycli#1014. I don't think there are more known compatibility issues, but I don't know for sure.

I don't think it would be good if different parts of the documentation would recommend different commandline SQL clients.

@dveeden
Copy link
Contributor Author

dveeden commented Jun 7, 2022

@mjonss @kolbe any opinion on this?

@kolbe
Copy link
Contributor

kolbe commented Jun 7, 2022

I don't believe I've ever used MySQL Shell (mysqlsh), and I don't think I've ever even heard of mycli. I usually use the MariaDB client (because it's usually built with readline).

My gut reaction is I think it is not a good idea to recommend mycli, because it's not as widely used and not as easy to install. But I may be wrong about that.

I certainly do agree that having some examples use the mysql client, while some use mycli and others use tiup client is very confusing, and we should try to avoid that.

@Icemap
Copy link
Member

Icemap commented Jun 7, 2022

@c4pt0r Any opinion on this? mycli is more advanced, but mysql-client is official client.

@Icemap
Copy link
Member

Icemap commented Jun 7, 2022

@dveeden We had a discussion about that. We will use mysql-client in the documentation and additionally use Tips to recommend mycli.

@mjonss
Copy link
Contributor

mjonss commented Jun 7, 2022

My opinion is that we should focus on the older mysql command line client from Oracle/MySQL and use that in our documentation. I don't mind if we have a place in the documentation where we describe other clients and give some tips and tricks on how to use them, but I think all examples etc. in our documentation should use the same old fashioned mysql-client from Oracle/MySQL.

Copy link
Collaborator

@Oreoxmt Oreoxmt left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jun 13, 2022
@dveeden
Copy link
Contributor Author

dveeden commented Oct 18, 2022

@Icemap what's the status of this?

@ti-chi-bot ti-chi-bot removed the status/LGT1 Indicates that a PR has LGTM 1. label Oct 20, 2022
@ti-chi-bot ti-chi-bot added the status/LGT2 Indicates that a PR has LGTM 2. label Oct 20, 2022
@Oreoxmt Oreoxmt added type/enhancement The issue or PR belongs to an enhancement. needs-cherry-pick-release-6.1 Should cherry pick this PR to release-6.1 branch. needs-cherry-pick-release-6.2 labels Oct 21, 2022
@Oreoxmt Oreoxmt closed this Oct 21, 2022
@Oreoxmt Oreoxmt reopened this Oct 21, 2022
@Oreoxmt
Copy link
Collaborator

Oreoxmt commented Oct 21, 2022

/merge

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: 3a7ca48

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Oct 21, 2022
@ti-chi-bot ti-chi-bot merged commit e571134 into pingcap:master Oct 21, 2022
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created: #10946.

@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created: #10947.

@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created: #10948.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/develop This PR relates to the area of TiDB App development. needs-cherry-pick-release-6.1 Should cherry pick this PR to release-6.1 branch. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. translation/done This PR has been translated from English into Chinese and updated to pingcap/docs-cn in a PR. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants