Skip to content

Observations, bug reports, and ideas from comparing Dart MySQL client implementations #5

Description

@kevmoo

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:

  1. All 5 requests evaluate canCreate as true (since 0 connections are active or idle yet).
  2. 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! 🍻

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions