feat(query): add static parser

[thomasgauvin]

I'm hoping to help push for getting the interpreting parser available (this makes it possible to use mysql2 as an alternative to mysql on Cloudflare Workers).

I opened this as a new PR because I have some bandwidth to address further comments in the PR if they come up. This code merely applies the recommended fixes as noted by @shiyuhang0 to the original PR by @sidorares #2099 (the original commit history is maintained and the branch was rebased onto master).

This could also be created as a PR to deoptimise-large-rows branch, however the maintainers recommend moving forward.

[codecov]

Codecov Report

Attention: Patch coverage is 15.82734% with 117 lines in your changes missing coverage. Please review.

Project coverage is 87.68%. Comparing base (f2cc820) to head (fdca5d7).

Files with missing lines	Patch %	Lines
lib/parsers/static_text_parser.js	11.45%	116 Missing ⚠️
lib/commands/query.js	83.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3365      +/-   ##
==========================================
- Coverage   88.45%   87.68%   -0.77%     
==========================================
  Files          83       84       +1     
  Lines       13067    13205     +138     
  Branches     1393     1396       +3     
==========================================
+ Hits        11558    11579      +21     
- Misses       1509     1626     +117     

Flag	Coverage Δ
compression-0	87.68% <15.82%> (-0.77%)	⬇️
compression-1	87.68% <15.82%> (-0.77%)	⬇️
tls-0	87.08% <15.82%> (-0.76%)	⬇️
tls-1	87.45% <15.82%> (-0.77%)	⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[wellwelwel]

Thanks, @thomasgauvin! And sorry, january was a hectic month and I didn't really pay attention to your comment in #2289 (comment).

lib/parsers/static_text_parser.js 11.26% 126 Missing ⚠️

I'll add the label needs tests, but I don't intend to block this PR progress because of this. Probably tomorrow I'll go through the tests that currently use evaluation and adapt them for the new parser.

Perhaps a simple approach with parameterized tests can make things easier to avoid duplicate tests.

[wellwelwel]

@thomasgauvin, I added some security points that were resolved after PR #2099 was created.

If it's not a problem, can I commit directly to your branch (including the typeCast / nestTables question and to add the tests I mentioned yesterday)?

[wellwelwel]

Due to the dynamic object properties, I believe it's beneficial to bring #2702 changes here.

There is a PoC from Snyk (Prototype Pollution): https://security.snyk.io/vuln/SNYK-JS-MYSQL2-6861580.

[wellwelwel]

Also, in #2289 it was decided to use disableEval instead of useStaticParser (I'm happy with both, to be honest):

• Adapt to Cloudflare Workers environment #2289 (comment)

[thomasgauvin]

@thomasgauvin, I added some security points that were resolved after PR #2099 was created.

If it's not a problem, can I commit directly to your branch (including the typeCast / nestTables question and to add the tests I mentioned yesterday)?

Yes, please feel free to commit anything to the branch directly. I've added you as collaborator on the fork so you should have commit access.

[thomasgauvin]

Also, in #2289 it was decided to use disableEval instead of useStaticParser (I'm happy with both, to be honest):

• Adapt to Cloudflare Workers environment #2289 (comment)

Missed that, will adjust

[wellwelwel]

🔬 Testing Updates

Note

I'll be updating this comment as I progress. Feel free to help with the debugging process.

---

node-mysql2/test/integration/connection/test-typecast.test.cjs

Line 47 in f2cc820

assert(Buffer.isBuffer(res[0].foo));

• Expect for a Buffer, but is returning a literal string "foobar".

---

node-mysql2/test/integration/connection/test-nested-tables-query.test.cjs

Line 126 in f2cc820

assert.equal(rows2[0].nested_test_id, 1);

• Expect for 1, but is returning undefined.

node-mysql2/test/integration/connection/test-nested-tables-query.test.cjs

Line 127 in f2cc820

assert.equal(rows2[0].nested_test_title, 'test');

• Expect for "test", but is returning undefined.

node-mysql2/test/integration/connection/test-nested-tables-query.test.cjs

Line 155 in f2cc820

assert.deepEqual(rows2, rows2e);

• Expect for [ { nested_test: { id: 1, title: 'test' } } ], but is returning [ { nested_test_id: 1, nested_test_title: 'test' } ].

---

node-mysql2/test/esm/integration/parsers/json-string.test.mjs

Lines 18 to 20 in f2cc820

	const [result] = await connection.query(
	`SELECT CAST('{"test": true}' AS JSON) AS json_result`,
	);

• The connection is never closed when executing this query. To avoid blocking the CI, it's being skipped for now.

[wellwelwel]

@sidorares, as the query (and eventually execute) method is one of the most critical in the entire project, I chose not to create an exclusive test for the static parser, but rather to test the entire test suite against this option, including all Node.js, Bun, and Deno versions.

@thomasgauvin, I won't be committing until Saturday. Feel free to continue or wait 🙋🏻‍♂️

• For the reasons in my comment above, naturally all CI using this option will fail at the moment.
• I'll wait to add this variation to the coverage CI when all the tests have passed.