I hate AI slop, but I love a good comparative analysis. I think this is worth your time to read, so I'm posting it. I'm pretty sure about most of the claims, but I wanted to be transparent!
It looks like you and I both forked the same repo. I've done a bunch of cleanup (about 18 commits) for a personal project - https://github.com/kevmoo/mysql.dart
I saw your work and wanted to compare notes. I don't really want to be in the MySQL client library business, so I'm not planning to publish my fork. Feel free to pull in bit you find useful, etc.
Here's the Agent-driven comparison as of the time of this issue being posted.
I noticed and really appreciated several great fixes you’ve landed—especially adding support for caching_sha2_password, fixing multi-byte UTF-8 decoding for Arabic/Urdu, and adding the defensive length checks for Apache Doris compatibility!
While comparing our implementations, I noticed a couple of subtle bugs and edge cases in mysql_client_plus that you might want to look into. I wanted to share them here constructively in case they save you (or your users) some debugging time!
1. 🐛 Unencrypted caching_sha2_password RSA flow is currently unreachable
In lib/src/mysql_client/connection.dart around line 275, when handling MySQLPacketExtraAuthData:
if (_activeAuthPluginName != 'caching_sha2_password') { ... }
if (_secure == false) {
throw MySQLClientException(
"Auth plugin caching_sha2_password is supported only with secure connections. Pass secure: true or use another auth method");
}
Because this exception is thrown immediately whenever secure: false, the connection aborts before it can receive status == 4 or request the server's RSA public key.
Furthermore, further down in the MySQLPacketAuthMoreData handler (line 306):
if (_activeAuthPluginName != 'sha256_password') {
throw MySQLClientException("Unexpected plugin for AuthMoreData: $_activeAuthPluginName");
}
if (_secure == false) {
throw MySQLClientException("Auth plugin sha256_password is supported only...");
}
This hard-check prevents caching_sha2_password from utilizing the encryptPasswordWithRSA helper you built in packet_extra_auth_more_data.dart. If a user attempts to connect to a standard MySQL 8 instance over unencrypted TCP (secure: false) inside a private VPC, the RSA challenge code cannot be reached.
Suggestion: You could relax the _secure == false exception on initial extra auth data arrival, allowing status == 4 to proceed to public key request (0x01) when unencrypted, and update line 306 to check _activeAuthPluginName == 'caching_sha2_password' || _activeAuthPluginName == 'sha256_password'.
2. 🌊 Connection Pool race condition during async socket connection
In pool.dart (line 136), permission to spawn a new connection is checked like this:
final canCreate = _idleConnections.length + _activeConnections.length < maxConnections;
Because canCreate evaluates synchronously while socket establishment (await conn.connect()) runs asynchronously, there is a small window during startup or traffic spikes where concurrent queries bypass the pool limit. For example, if 5 execute() calls arrive simultaneously when the pool is empty and maxConnections: 2:
- All 5 requests evaluate
canCreate as true (since 0 connections are active or idle yet).
- The pool initiates 5 TCP connections concurrently.
Additionally, if an active connection drops unexpectedly (conn.onClose), it gets removed from _activeConnections, but any pending _waiters aren't triggered or failed, which can leave Completer instances hung.
Suggestion: Tracking an internal _connectingCount integer for in-flight socket attempts and accounting for it in allConnectionsQty prevents concurrent over-spawning. Triggering _releaseConnection or queue evaluation inside conn.onClose ensures waiting requests recover cleanly.
3. 🤔 Consideration: Dynamic jsonDecode on JSON columns
In mysql_column_type.dart (line 426), columns of type JSON (0xf5) automatically run jsonDecode(), returning a Map or List dynamically:
case mysqlColumnTypeJson:
...
final jsonObject = jsonDecode(jsonString);
return Tuple2(jsonObject, bytesConsumed);
While this is super handy for quick scripts, some users relying on ORMs, repositories, or typed serializers (MyModel.fromJson(...)) might expect database drivers to return raw scalar String payloads across all text/JSON columns so their deserialization pipelines don't encounter unexpected runtime type exceptions. Offering a flag or returning raw JSON text by default might improve predictability for larger apps!
💡 Ideas for Dart 3 Modernization
If you ever decide to bump your minimum SDK constraint to Dart 3.0+, you could replace package:tuple (Tuple2) across the protocol parsing extensions with native Dart Records (value, offset). It cleans up the stack traces nicely and drops an external dependency!
Thanks again for keeping this project going! If you're ever interested in comparing notes or sharing test suites between our forks, feel free to ping me anytime. Keep up the awesome work! 🍻
I hate AI slop, but I love a good comparative analysis. I think this is worth your time to read, so I'm posting it. I'm pretty sure about most of the claims, but I wanted to be transparent!
It looks like you and I both forked the same repo. I've done a bunch of cleanup (about 18 commits) for a personal project - https://github.com/kevmoo/mysql.dart
I saw your work and wanted to compare notes. I don't really want to be in the MySQL client library business, so I'm not planning to publish my fork. Feel free to pull in bit you find useful, etc.
Here's the Agent-driven comparison as of the time of this issue being posted.
I noticed and really appreciated several great fixes you’ve landed—especially adding support for
caching_sha2_password, fixing multi-byte UTF-8 decoding for Arabic/Urdu, and adding the defensive length checks for Apache Doris compatibility!While comparing our implementations, I noticed a couple of subtle bugs and edge cases in
mysql_client_plusthat you might want to look into. I wanted to share them here constructively in case they save you (or your users) some debugging time!1. 🐛 Unencrypted
caching_sha2_passwordRSA flow is currently unreachableIn
lib/src/mysql_client/connection.dartaround line 275, when handlingMySQLPacketExtraAuthData:Because this exception is thrown immediately whenever
secure: false, the connection aborts before it can receivestatus == 4or request the server's RSA public key.Furthermore, further down in the
MySQLPacketAuthMoreDatahandler (line 306):This hard-check prevents
caching_sha2_passwordfrom utilizing theencryptPasswordWithRSAhelper you built inpacket_extra_auth_more_data.dart. If a user attempts to connect to a standard MySQL 8 instance over unencrypted TCP (secure: false) inside a private VPC, the RSA challenge code cannot be reached.Suggestion: You could relax the
_secure == falseexception on initial extra auth data arrival, allowingstatus == 4to proceed to public key request (0x01) when unencrypted, and update line 306 to check_activeAuthPluginName == 'caching_sha2_password' || _activeAuthPluginName == 'sha256_password'.2. 🌊 Connection Pool race condition during async socket connection
In
pool.dart(line 136), permission to spawn a new connection is checked like this:Because
canCreateevaluates synchronously while socket establishment (await conn.connect()) runs asynchronously, there is a small window during startup or traffic spikes where concurrent queries bypass the pool limit. For example, if 5execute()calls arrive simultaneously when the pool is empty andmaxConnections: 2:canCreateastrue(since 0 connections are active or idle yet).Additionally, if an active connection drops unexpectedly (
conn.onClose), it gets removed from_activeConnections, but any pending_waitersaren't triggered or failed, which can leaveCompleterinstances hung.Suggestion: Tracking an internal
_connectingCountinteger for in-flight socket attempts and accounting for it inallConnectionsQtyprevents concurrent over-spawning. Triggering_releaseConnectionor queue evaluation insideconn.onCloseensures waiting requests recover cleanly.3. 🤔 Consideration: Dynamic
jsonDecodeon JSON columnsIn
mysql_column_type.dart(line 426), columns of typeJSON(0xf5) automatically runjsonDecode(), returning aMaporListdynamically:While this is super handy for quick scripts, some users relying on ORMs, repositories, or typed serializers (
MyModel.fromJson(...)) might expect database drivers to return raw scalarStringpayloads across all text/JSON columns so their deserialization pipelines don't encounter unexpected runtime type exceptions. Offering a flag or returning raw JSON text by default might improve predictability for larger apps!💡 Ideas for Dart 3 Modernization
If you ever decide to bump your minimum SDK constraint to Dart 3.0+, you could replace
package:tuple(Tuple2) across the protocol parsing extensions with native Dart Records(value, offset). It cleans up the stack traces nicely and drops an external dependency!Thanks again for keeping this project going! If you're ever interested in comparing notes or sharing test suites between our forks, feel free to ping me anytime. Keep up the awesome work! 🍻