fix formatSQL error

[baurine]

Fix formatSQL() error by sql-formatter and close #1157 as well

Currently some SQLs are failed to format, and it reports the following error:

Uncaught TypeError: Cannot read properties of undefined (reading 'slice')
    at parseKey (Tokenizer.js:191:20)
    at Tokenizer2.getPlaceholderTokenWithKey (Tokenizer.js:235:21)
    at Tokenizer2.getIdentNamedPlaceholderToken (Tokenizer.js:187:19)
    at Tokenizer2.getPlaceholderToken (Tokenizer.js:182:19)
    at Tokenizer2.getNextToken (Tokenizer.js:127:147)
    at Tokenizer2.tokenize (Tokenizer.js:107:24)
    at MySqlFormatter2.format7 (Formatter.js:87:38)
    at format8 (sqlFormatter.js:74:29)
    at formatSql (index.ts:15:19)
    at index.ts:32:23

The root cause is discussed here: sql-formatter-org/sql-formatter#147 , and a forked repo - prettier-sql resolved it.

---

I did some comparisons between sql-formatter and sql-formatter-plus-plus.

Target SQL:

select sleep(1);

	webpack	vite(dev)	vite(prod)	esbuild
sql-formatter	v	v	v	v
sql-formatter-plus-plus	v	x	x	x

It reports the following error when formating the above target sql by sql-formatter-plus-plus with vite(dev/prod) and esbuild, reported issue to esbuild: evanw/esbuild#2181

rewrite-pattern.js:50 Uncaught Error: Failed to recognize value `undefined` for property `Alphabetic`.
    at getUnicodePropertyValueSet (rewrite-pattern.js:50:9)
    at handleLoneUnicodePropertyNameOrValue (rewrite-pattern.js:68:9)
    at getUnicodePropertyEscapeSet (rewrite-pattern.js:76:9)
    at processCharacterClass (rewrite-pattern.js:160:13)
    at processTerm (rewrite-pattern.js:205:11)
    at rewrite-pattern.js:259:12
    at Array.map (<anonymous>)
    at processTerm (rewrite-pattern.js:258:26)
    at rewrite-pattern.js:259:12
    at Array.map (<anonymous>)

Target SQL:

select `topics` . `id` from `topics` left outer join `categories` on `categories` . `id` = `topics` . `category_id` where ( `topics` . `archetype` <> ? ) and ( coalesce ( `categories` . `topic_id` , ? ) <> `topics` . `id` ) and `topics` . `visible` = true and ( `topics` . `deleted_at` is ? ) and ( `topics` . `category_id` is ? or `topics` . `category_id` in ( ... ) ) and ( `topics` . `category_id` != ? ) and `topics` . `closed` = false and `topics` . `archived` = false and ( `topics` . `created_at` > ? ) order by `rand` ( ) limit ?

	webpack	vite(dev)	vite(prod)	esbuild
sql-formatter	x	x	x	x
sql-formatter-plus-plus	v	x	x	x

It reports the beginning error by sql-formatter with all (webpack/vite/esbuild).

---

About unit test.

I found it's not easy to do unit test when we using esbuild. if we use jest, it will bring babel back, and the test result may not be correct. See above table, sql-formatter-plus-plus works fine with wepback but not with vite/esbuild.

So I did some a simple way, test it directly under the method definition, but only run it in dev and CI mode.

[ti-chi-bot]

[REVIEW NOTIFICATION]

This pull request has been approved by:

• breeswish

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.

[codecov-commenter]

Codecov Report

Merging #1214 (2d4fbb8) into master (194ab10) will decrease coverage by 9.20%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1214      +/-   ##
==========================================
- Coverage   31.54%   22.34%   -9.21%     
==========================================
  Files         291      164     -127     
  Lines       17104    14140    -2964     
  Branches      684        0     -684     
==========================================
- Hits         5395     3159    -2236     
+ Misses      11478    10750     -728     
  Partials      231      231              

Flag	Coverage Δ
be_integration_test_latest	9.29% <ø> (-0.05%)	⬇️
be_integration_test_v4.0.1	9.32% <ø> (ø)
be_unit_test	24.07% <ø> (ø)
e2e_test	?

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

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 194ab10...2d4fbb8. Read the comment docs.

[breezewish]

It reports the following error when formating the above target sql by sql-formatter-plus-plus with vite(dev/prod) and esbuild.

So what's wrong with the things produced by vite and esbuild? Are there bugs in esbuild? What is the real expected behavior? Is it possible to fix esbuild, or make workarounds in sql-formatter-plus-plus to skip the bug?

[baurine]

It reports the following error when formating the above target sql by sql-formatter-plus-plus with vite(dev/prod) and esbuild.

So what's wrong with the things produced by vite and esbuild? Are there bugs in esbuild? What is the real expected behavior? Is it possible to fix esbuild, or make workarounds in sql-formatter-plus-plus to skip the bug?

I think so, I digged it but have no idea how to figure it out, I have reported issue to esbuild: evanw/esbuild#2181 (it seems has one reply now, let me have a look)

[baurine]

Thanks for the esbuild issue reply, now I have an idea to resolve it by sql-formatter-plus-plus, and I have PoC it successfully by the following code:

+ import { format } from 'sql-formatter-plus-plus/dist/sql-formatter'
- import { format } from 'sql-formatter-plus-plus"

we import from the bundled sql-formatter-plus-plus then we can avoid the dynamic require issue.

I can forked the sql-formatter-plus-plus to make the default entry dist/xxx.js instead of lib/xxx.js, and I can add our tidb SQL dialect formatter, will try it soon.

[breezewish]

Cool! It would be better if we can dig into why esbuild cannot work with a library, by finding out a minimal reproducible example (i.e. a specific code pattern that causes the problem). In this way esbuild can be improved (if there are bugs) and the community can be benefited.

[baurine]

Finally, forked the sql-formatter-plus-plus and did the following changes:

1. change the entry from lib/sqlFormatter.js to bundled dist/sql-formatter.js or dist/sql-formatter.min.js to avoid runtime dynamic require, so esbuild can build it corretly.
2. move TiDBSqlFormatter.js from tidb-dashboard
3. add unit test for TiDBSqlFormatter.js

See details from the PR work with esbuild and adjust tidb sql

Also have tried fix the upstream regexpu-core according to esbuild author's advice but for some reason, failed. (the reason is described in the above PR)

[breezewish]

We should ensure that tests are contained in order to avoid regression.

[baurine]

ok, I will add them back in the dashboard. I added them in the @baurine/sql-formatter-plus as well.