parser: Fix undocumented password_func function | tidb-test=pr/2458

[dveeden]

What problem does this PR solve?

Issue Number: close #58311

Problem Summary:

What changed and how does it work?

Check List

Tests

• Unit test
• Integration test
• Manual test (add detailed scripts or steps below)
• No need to test

  • I checked and no code files have been changed.

Side effects

• Performance regression: Consumes more CPU
• Performance regression: Consumes more Memory
• Breaking backward compatibility

Documentation

• Affects user behaviors
• Contains syntax changes
• Contains variable changes
• Contains experimental features
• Changes MySQL compatibility

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

The undocumented `password_func` function was removed.

[tiprow]

Hi @dveeden. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[dveeden]

/check-issue-triage-complete

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.8616%. Comparing base (a3c9b79) to head (343601d).
Report is 3 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #58312        +/-   ##
================================================
+ Coverage   73.5693%   73.8616%   +0.2923%     
================================================
  Files          1680       1710        +30     
  Lines        463726     471136      +7410     
================================================
+ Hits         341160     347989      +6829     
+ Misses       101733     101619       -114     
- Partials      20833      21528       +695     

Flag	Coverage Δ
integration	45.9586% <ø> (?)
unit	72.3347% <ø> (-0.0020%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	52.6910% <ø> (ø)
parser	∅ <ø> (∅)
br	45.9807% <ø> (+0.0268%)	⬆️

[dveeden]

/retest

[tiprow]

@dveeden: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[dveeden]

/ok-to-test

[Defined2014]

This function is deprecated in MySQL 5.7 and is removed in MySQL 8.0.

Should we rename it to password or just remove it?

I found it included by #2680, @XuHuaiyu PTAL

[dveeden]

This function is deprecated in MySQL 5.7 and is removed in MySQL 8.0.

Should we rename it to password or just remove it?

I found it included by #2680, @XuHuaiyu PTAL

mysql-8.0.11-TiDB-v8.5.0-alpha-356-gdbd4eeaaf2> \W
Show warnings enabled.
mysql-8.0.11-TiDB-v8.5.0-alpha-356-gdbd4eeaaf2> select password("abc");
+-------------------------------------------+
| password("abc")                           |
+-------------------------------------------+
| *0D3CED9BEC10A777AEC23CCC353A8C08A633045E |
+-------------------------------------------+
1 row in set, 1 warning (0.00 sec)

Warning (Code 1681): PASSWORD is deprecated and will be removed in a future release.

MySQL has removed this function and in TiDB it has been deprecated. I'm ok to remove password(), but that should probably be done in a separate PR.

Note that password() and password_func() behave the same and are basically the same. I think it was never meant to be two separate things. So we don't need to rename things.

See also this from the diff:

diff --git a/pkg/parser/parser_test.go b/pkg/parser/parser_test.go
index 7e60aaabb4..d5601e6915 100644
--- a/pkg/parser/parser_test.go
+++ b/pkg/parser/parser_test.go
@@ -2258 +2258 @@ func TestBuiltin(t *testing.T) {
-               {`SELECT PASSWORD(@str);`, true, "SELECT PASSWORD_FUNC(@`str`)"},
+               {`SELECT PASSWORD(@str);`, true, "SELECT PASSWORD(@`str`)"},

So running PASSWORD() through the parser and returning it gives PASSWORD_FUNC()

[ti-chi-bot]

[LGTM Timeline notifier]

Timeline:

• 2024-12-17 08:19:42.585785399 +0000 UTC m=+944972.674587929: ☑️ agreed by Defined2014.
• 2024-12-17 15:42:24.125594974 +0000 UTC m=+971534.214397516: ☑️ agreed by mjonss.

[D3Hunter]

our doc also contains password_func, see https://docs.pingcap.com/tidb/stable/sql-statement-show-builtins, this pr breaks this, but seems trivial

rest lgtm

[dveeden]

our doc also contains password_func, see https://docs.pingcap.com/tidb/stable/sql-statement-show-builtins, this pr breaks this, but seems trivial

rest lgtm

I won't really call this documentation as it is just example output of SHOW BUILTINS. However this does mean that after this PR we need to update this page. I'll take care of that.

The output of SHOW BUILTINS also demonstrates an issue fixed by this PR:

With v8.4.0

mysql-8.0.11-TiDB-v8.4.0> pager grep password
PAGER set to 'grep password'
mysql-8.0.11-TiDB-v8.4.0> show builtins;
| old_password                |
| password_func               |
| validate_password_strength  |
303 rows in set (0.01 sec)

mysql-8.0.11-TiDB-v8.4.0> pager
Default pager wasn't set, using stdout.
mysql-8.0.11-TiDB-v8.4.0> SELECT old_password("abc");
ERROR 1305 (42000): FUNCTION OLD_PASSWORD does not exist
mysql-8.0.11-TiDB-v8.4.0> SELECT password_func("abc");
+-------------------------------------------+
| password_func("abc")                      |
+-------------------------------------------+
| *0D3CED9BEC10A777AEC23CCC353A8C08A633045E |
+-------------------------------------------+
1 row in set, 1 warning (0.01 sec)

mysql-8.0.11-TiDB-v8.4.0> SELECT password("abc");
+-------------------------------------------+
| password("abc")                           |
+-------------------------------------------+
| *0D3CED9BEC10A777AEC23CCC353A8C08A633045E |
+-------------------------------------------+
1 row in set, 1 warning (0.00 sec)

mysql-8.0.11-TiDB-v8.4.0>

• OLD_PASSWORD() is in the output of SHOW BUILTINS, while it shouldn't. This is fixed in expression: cleanup old functions #58327.
• PASSWORD() is not in the output of SHOW BUILTINS, while it should.
• password_func() is in the output, but otherwise undocumented (and not compatible with MySQL)

With this PR:

mysql-8.0.11-TiDB-v8.5.0-alpha-356-gdbd4eeaaf2> pager grep password
PAGER set to 'grep password'
mysql-8.0.11-TiDB-v8.5.0-alpha-356-gdbd4eeaaf2> show builtins;
| old_password                |
| password                    |
| validate_password_strength  |
303 rows in set (0.00 sec)

mysql-8.0.11-TiDB-v8.5.0-alpha-356-gdbd4eeaaf2> pager
Default pager wasn't set, using stdout.

[D3Hunter]

/approve

[D3Hunter]

/hold

feel free to unhold after you discuss with frank

[Frank945946]

Please update the corresponding documentation at the same time. The rest LGTM.

[ti-chi-bot]

@Frank945946: adding LGTM is restricted to approvers and reviewers in OWNERS files.

In response to this:

Please update the corresponding documentation at the same time. The rest LGTM.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[dveeden]

/unhold

[dveeden]

/retest

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: D3Hunter, Defined2014, Frank945946, mjonss, XuHuaiyu

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [D3Hunter,Defined2014,XuHuaiyu,mjonss]
• pkg/expression/OWNERS [XuHuaiyu]
• pkg/parser/OWNERS [D3Hunter]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment