Functions trilogy_packet_parser_execute in src/packet_parser.c and main in example/trilogy_query.c are very large functions with little to no documentation, making them hard to maintain.

See #41 and #42 for more context.

I had a bit more time to dig into this, and collected some data about what's going on. Since the context related to this bug is a bit spread out, I thought I'd open this issue to consolidate the information we have so far.

Background

Currently, we're skipping a test that's intended to test connection errors when we attempt to connect using a user account with insufficient privileges / a nonexistent user: https://github.com/adrianna-chang-shopify/trilogy/blob/b54138b188f19722d7f94614ae72064683e56276/contrib/ruby/test/client_test.rb#L533-L537

This test sometimes fails intermittently with:

1) Failure:
ClientTest#test_connection_error [/Users/adriannachang/src/github.com/trilogy/contrib/ruby/test/client_test.rb:536]:
[Trilogy::BaseConnectionError] exception expected, not
Class: <Trilogy::QueryError>
Message: <"trilogy_auth_recv: TRILOGY_PROTOCOL_VIOLATION">

on MySQL 8.0 (example CI build).

More Info

I added a couple of print statements to see what's going on:

diff --git a/contrib/ruby/test/client_test.rb b/contrib/ruby/test/client_test.rb
index 87ae5dc..7a47c0d 100644
--- a/contrib/ruby/test/client_test.rb
+++ b/contrib/ruby/test/client_test.rb
@@ -531,11 +531,12 @@ class ClientTest < TrilogyTest
   end
 
   def test_connection_error
-    skip("Test fails intermittently with TRILOGY_PROTOCOL_VIOLATION. See https://github.com/github/trilogy/pull/42")
-    err = assert_raises Trilogy::ConnectionError do
+    puts "--------------------- RUNNING TEST CONNECTION ERROR ---------------------"
+    err = assert_raises Trilogy::Error do
       new_tcp_client(username: "foo")
     end
 
+   puts "#{err.class}: #{err.message}"
     assert_includes err.message, "Access denied for user 'foo'"
   end

diff --git a/src/client.c b/src/client.c
index 9cdb923..8d65a42 100644
--- a/src/client.c
+++ b/src/client.c
@@ -250,6 +250,7 @@ static int read_eof_packet(trilogy_conn_t *conn)
 
 static int read_auth_switch_packet(trilogy_conn_t *conn, trilogy_handshake_t *handshake)
 {
+    printf("read_auth_switch_packet\n");
     trilogy_auth_switch_request_packet_t auth_switch_packet;
 
     int rc = trilogy_parse_auth_switch_request_packet(conn->packet_buffer.buff, conn->packet_buffer.len,
@@ -259,6 +260,8 @@ static int read_auth_switch_packet(trilogy_conn_t *conn, trilogy_handshake_t *ha
         return rc;
     }
 
+    printf("auth_switch_packet.auth_plugin: %s\n", auth_switch_packet.auth_plugin);
+
     if (strcmp("mysql_native_password", auth_switch_packet.auth_plugin) &&
         strcmp("caching_sha2_password", auth_switch_packet.auth_plugin)) {
         // Only support native password & caching sha2 password here.
@@ -426,14 +429,17 @@ int trilogy_auth_recv(trilogy_conn_t *conn, trilogy_handshake_t *handshake)
 
     switch (current_packet_type(conn)) {
     case TRILOGY_PACKET_OK:
+        printf("READ TRILOGY_PACKET_OK\n");
         trilogy_auth_clear_password(conn);
         return read_ok_packet(conn);
 
     case TRILOGY_PACKET_ERR:
+        printf("READ TRILOGY_PACKET_ERR\n");
         trilogy_auth_clear_password(conn);
         return read_err_packet(conn);
 
     case TRILOGY_PACKET_EOF:
+        printf("READ TRILOGY_PACKET_EOF\n");
         // EOF is returned here if an auth switch is requested.
         // We still need the password for the switch, it will be cleared
         // in a follow up call to this function after the switch.

Note that for all of these tests, I ran them using the CI dockerfiles, i.e. MYSQL_VERSION=5.7 DOCKERFILE=Dockerfile.test.buster script/cibuild and MYSQL_VERSION=8 DOCKERFILE=Dockerfile.test.buster script/cibuild

On MySQL5.7:

--------------------- RUNNING TEST CONNECTION ERROR ---------------------
READ TRILOGY_PACKET_ERR
Trilogy::BaseConnectionError: 1045: Access denied for user 'foo'@'192.168.16.3'(using password: NO)

TEST ALWAYS PASSES. It immediately reads a packet error, and we see ER_ACCESS_DENIED_ERROR and raise the corresponding BaseConnectionError.

On MySQL8.0:

Okay, so all sorts of different things can happen when we run the tests on MySQL8, even with the same seed. This issue is not meant to be diagnostic, simply to share what I observed.

Sometimes the test passes in the same way MySQL5.7 does:

--------------------- RUNNING TEST CONNECTION ERROR ---------------------
READ TRILOGY_PACKET_ERR
Trilogy::BaseConnectionError: 1045: Access denied for user 'foo'@'172.31.0.3' (using password: NO)

Sometimes the test passes, but with an attempted auth switch!

--------------------- RUNNING TEST CONNECTION ERROR ---------------------
READ TRILOGY_PACKET_EOF
read_auth_switch_packet
auth_switch_packet.auth_plugin: mysql_native_password
READ TRILOGY_PACKET_ERR
Trilogy::BaseConnectionError: 1045: Access denied for user 'foo'@'192.168.160.3' (using password: NO)

In this case, we performed an auth switch to mysql_native_password before seeing the access denied error.

The test fails with a protocol violation error if we attempt to perform an auth switch to the unsupported sha256_password (we raise here):

--------------------- RUNNING TEST CONNECTION ERROR ---------------------
READ TRILOGY_PACKET_EOF
read_auth_switch_packet
auth_switch_packet.auth_plugin: sha256_password
Trilogy::QueryError: trilogy_auth_recv: TRILOGY_PROTOCOL_VIOLATION

Other things I investigated

Setting the auth plugin default to mysql_native_password instead of MySQL8's caching_sha2_password:

diff --git a/docker-compose.yml b/docker-compose.yml
index f2e791c..7429638 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -8,6 +8,7 @@ services:
     - --gtid-mode=ON
     - --enforce-gtid-consistency=ON
     - --log-bin=mysql-bin.log
+    - --default-authentication-plugin=mysql_native_password
     environment:
       MYSQL_ALLOW_EMPTY_PASSWORD: 1
       MYSQL_DATABASE: test

Results:
Didn't make a difference, still sometimes attempted to switch to sha256_password (failure), and other times attempted to switch to caching_sha2_password (test succeeded with auth switch, and then access denied error).

Using the wrong password instead of a nonexistent user

  def test_connection_error
    puts "--------------------- RUNNING TEST CONNECTION ERROR ---------------------"
    err = assert_raises Trilogy::Error do
      new_tcp_client(username: "native", password: "wrong")
    end

    puts "#{err.class}: #{err.message}"
    assert_includes err.message, "Access denied for user 'native'"
  end

Results:
This always passes with the expected access denied error, and I never saw an attempted auth switch here.

However, if I do the same thing for root user, I get a TRILOGY_UNEXPECTED_PACKET error 🤷‍♀️

  def test_connection_error
    err = assert_raises Trilogy::ConnectionError do
      new_tcp_client(username: "root", password: "incorrect")
    end

    assert_includes err.message, "Access denied for user 'root'"
  end

  1) Failure:
ClientTest#test_connection_error [/Users/adriannachang/src/github.com/trilogy/contrib/ruby/test/client_test.rb:534]:
[Trilogy::ConnectionError] exception expected, not
Class: <Trilogy::QueryError>
Message: <"trilogy_auth_recv: TRILOGY_UNEXPECTED_PACKET">

Takeaways

• Still no idea what the bug is, but it seems to be specifically around reading an auth switch packet when we shouldn't be, on MySQL8. Is it possible that we shouldn't always interpret TRILOGY_PACKET_EOF as an auth switch here?
• I think that, instead of skipping the conn error test, we should change it to use an existing user accouont (e.g. native) and an incorrect password, because this should still produce an access denied err without the auth switch flakiness.

cc @composerinteralia @eileencodes @paarthmadan

Trilogy is a client library for MySQL-compatible database servers, designed for performance, flexibility, and ease of embedding.

Would y'all be willing to share some real-world experience (benchmarks, other wins) from running in production at GitHub? What motivated this, and what problems did it solve relative to the previous (mysql2?) library.

🙇🏻

I recently added a Sequel adapter for trilogy: jeremyevans/sequel@0701217. I found trilogy very easy to use and that made the adapter quite easy to create. Thank you for that!

One issue I ran into is that trilogy does not return the number of matched rows in an update statement, it returns the number of rows modified. For example:

c = Trilogy.new(...)
c.query('CREATE TABLE t(a integer)')
c.query('INSERT INTO t VALUES (1)')
c.query('UPDATE t SET a = 1').affected_rows
# => 0 # instead of 1

I think this is MySQL default behavior, so it seems reasonable, but most other databases do not operate this way, instead returning the number of matched rows. It there a way to get the above code to return 1? The mysql2 gem supports this using flags: Mysql2::Client::FOUND_ROWS (which looks like it comes from CLIENT_FOUND_ROWS in the mysql driver). If there is not currently a way, could support be added for it?

This affects Sequel because by default, Sequel checks during model instance updates that one row was updated. If zero rows were updated, that by default indicates to Sequel that the underlying database record was already deleted, in which case Sequel should raise an exception. I disabled that part in Sequel when trilogy is used, but it would be nice to be able to enable it.

There are a few other parts of Sequel that don't work perfectly with trilogy, but those are also issues with mysql2, or deliberately unsupported by trilogy (lack of application_timezone).

So the disparity between the current release and the main branch are pretty big, any chance that a new release will be tagged and build soon ?

In Staging and Production

config = Rails.configuration.database_configuration[ Rails.env ]
connection = Trilogy.new( config )
#=> .../trilogy-2.6.1/lib/trilogy.rb:18:in `_connect': No such file or directory - trilogy_connect - unable to connect to /tmp/mysql.sock (Trilogy::SyscallError::ENOENT)

In Development

config = Rails.configuration.database_configuration[ Rails.env ]
connection = Trilogy.new( config )
#=> <Trilogy:0x000000013a9de1e0

The configs between the environments are similar with just different database, username, password and host values.

Moreover, the actual Rails application (i.e. ActiveRecord) works just fine in all environments using Trilogy.

Any thoughts on this?

Thanks!

Everything works, in that I don't see any issues with trilogy when running with mysql_native_password auth but the moment I switch to caching_sha2_password things break down in weird ways.

I am trying to understand where it fails but I am still learning how this works and I am only seeing a packet error.

It seems to perform authentication and create a successful connection to the mysql server when switching the client over to caching_sha2_password but then it fails during packet parsing for some reason.

I can see in show processlist during the handshake that the user moves from unauthorized to the user name but then it fails on the client just as its about to call trilogy_auth_recv or maybe its calling it too often with the wrong packet sequence/payload and the server disconnects. I am still debugging the issue.

I am using a debug version of the mysql server version 8.0.30-debug and I was about to debug the server side to see if it was indeed closing the connection because it didn't like something from the client.

Is this a known issue with caching_sha2_password and trilogy?

#71 was a good reminder that the gem does a whole bunch of things that currently have little or no documentation. With Trilogy soon to be an officially supported option for Rails, now is a good time to add some better documentation (expanding what we have in https://github.com/github/trilogy/blob/7ceee303b7d440f0b26cba65d00032bfb897c0ef/contrib/ruby/README.md would be a good place to start).

I see here that the memset() function is used to clear passwords.

As passwords are sensitive information I would suggest to replace it with memset_s() so that they can't be recovered from memory.

More information on StakeOverflow: https://stackoverflow.com/questions/56565539/memset-s-what-does-the-standard-mean-with-this-piece-of-text

I'm trying to integrate trilogy in a toy project I was working on with mysql2 without doing a lot of changes. Since I was using ActiveRecord to write my queries, I thought it would be nice if we could simply replace the adapter attrribute in config/database,yml from mysql2 to be trilogy, but it didn't work

.rbenv/versions/2.5.8/lib/ruby/gems/2.5.0/gems/bootsnap-1.7.3/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:23:in `require': Could not load the 'trilogy' Active Record adapter. Ensure that the adapter is spelled correctly in config/database.yml and that you've added the necessary adapter gem to your Gemfile. (LoadError)

So I was wondering if there is a way to add trilogy as an ActiveRecord adapter or if such feature could be supported in the future?

Process

1. Create a new Rails project (Rails 5, ruby 2.5, mysql2 gem)
2. Created a few DB models to test with using rails g model.
3. Added trilogy gem to the gemfile and updated Gemlock bundle update.
4. Updated adapter attribute in config/database.yml to be trilogy instead of mysql2.
5. Ran rails c

Expected result

Running Rails c should work and I can run MyModelName.connection successfully.

Current result

Rails console fails to start

.rbenv/versions/2.5.8/lib/ruby/gems/2.5.0/gems/bootsnap-1.7.3/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:23:in `require': Could not load the 'trilogy' Active Record adapter. Ensure that the adapter is spelled correctly in config/database.yml and that you've added the necessary adapter gem to your Gemfile. (LoadError)

Feature Request: Adding support for multi statement

At Shopify, we're exploring the work required to migrate to using the Trilogy client (and respective Active Record adapter). One pre-requisite for adoption is support for multi statement queries.

We enable Mysql2::Client::MULTI_STATEMENTS (the equivalent to TRILOGY_CAPABILITIES_MULTI_STATEMENTS) to enable some specific performance optimizations and are interested in seeing this supported in Trilogy.

Discussion

It seems there's already interest, as I was happy to stumble on this existing branch by @composerinteralia and @jhawthorn which seems to prototype some of this work.

I'm curious to understand what your thoughts are on bringing support for this. I'd love for this issue to be a place to exchange perhaps already existing context like:

1. Are you willing to accept this feature into Trilogy
2. Are there any particular blockers you've discovered?
3. Any particular API decisions you have considered?
   a. Any thoughts on mimicking mysql2's Ruby API: client.next_result and client.store_result
   b. Should multi statement be something the C library is aware of, or could this be a Ruby-binding-only abstraction

Moving forward

I'd like to note that I'm happy to build this feature out and was intending to draft out some work I have locally. I noticed your branch and figured it would be best to first solicit early feedback and share any context before presenting a solution.

Curious to hear your thoughts and context!

cc: @adrianna-chang-shopify @casperisfine

Something we noticed after upgrading from mysql2 to trilogy was the lack of field names when running a query manually.

Here is a simple example of the differences:

mysql2

Mysql2::Client.new( config ).query( "SELECT * FROM instruments" ).first
#=> {"SerialNumber"=>"ABCD1234", "Manufacturer"=>"Thermo", "Model"=>"TVA2020"}

trilogy

temporary_database_connection = Trilogy.new( config )
temporary_database_connection.select_db( config[ "database" ] )
temporary_database_connection.query( "SELECT * FROM instruments" ).first
#=> ["ABCD1234", "Thermo", "TVA2020"]

Differences

A few differences to point out:

1. In order to establish a database connection and run a query, you need to create a new Trilogy instance and then use select_db or change_db and provide the database name again, otherwise, you get a "No database selected" error.

2. mysql2 returns a Hash while trilogy returns an Array from the query method.

3. In an otherwise incredible drop-in replacement, this prevents the usage of fetching the row values based on the column names, like this:

   temporary_database_connection.query( "SELECT * FROM instruments" ).first.fetch( "SerialNumber" )

   Which worked very well on mysql2.

We're reverting back to mysql2 in the meantime but thought I would post it here to get your input because this is (so far) the only thing preventing us from moving forward with trilogy.

Many thanks!

Joshua

This repository is moving! 🚶🏻

This repository is moving to a new github.com home: https://github.com/trilogy-libraries/trilogy (there's nothing there right now)

Why?

To improve security of GitHub's primary organization, https://github.com/github, GitHub is moving all repositories with outside collaborators to separate organizations. This repository is one of those.

When?

We expect to perform this before June 1, 2023.

What will happen?

The repository in its entirety will be moved to the new organization at https://github.com/trilogy-libraries/trilogy (there's nothing there right now).

All current external contributors will retain their access.

All PRs should automatically rebase to the new repository location so you shouldn't need to do anything. If they don't please comment here and we will fix it.

All issues and discussions will be moved across at the same time too.

I cannot install gem - posted details at:
https://stackoverflow.com/questions/76395150/failed-to-install-rubygem-trilogy-how-can-i-resolve-make-failed-exit-code-2

Thx in advance!

Hello, thanks for this awesome work.

Is this adapter compatible with the fiber scheduler?

Thanks.

cc @adrianna-chang-shopify @casperisfine @composerinteralia @dhruvCW @matthewd

We made some improvements to the Ruby extension's error classes recently but based on recent discussions I think still have a way to go. I wanted to gather opinions in this issue and am happy to implement what we can come to a consensus on.

Problems I see:

• QueryError is our default fallback error class. I don't think this makes any sense, as this is a ProtocolError subclass but all errors other than TRILOGY_ERR are IMO connection-level errors. I think we should make BaseConnectionError the default error class for things which haven't been given a specific meaning (alternatively we could exhaustively define error classes for all trilogy status codes).
• "TRILOGY_CLOSED_CONNECTION". This error represents essentially an EOF over our socket, and it's one of the more common errors we come across. It's also the error we'll see if we attempt communication after a trilogy_sock_shutdown. Currently this appears as QueryError: TRILOGY_CLOSED_CONNECTION and we're forced to do string matching to detect it. I think this deserves its own error class, but also it should probably be distinct from ConnectionClosed, which is only raised from an explicitly closed connection. How about Trilogy::EOFError? For backwards compatibility I'd probably include "TRILOGY_CLOSED_CONNECTION" in the string.
  • Another possibility I'd like to entertain is deleting the translation of EPIPE errors to TRILOGY_CLOSED_CONNECTION, which would break compatibility with those doing string matching but I think otherwise is just better and more accurate information.
• SyscallError vs ConnectionError. As raised in #98, there's some confusion between SyscallError and BaseConnectionError. One option is that we can make Trilogy::SyscallError inherit from ConnectionError and then have only one thing to rescue, or we could better document that users should rescue both classes (I don't feel very strongly either way).
• I think ProtocolError's name is confusing. I read this as "protocol violation" rather than "we were told over the protocol that an error occurred", and I've seen confusion from others about the same. I preferred the previous DatabaseError naming but am open to other options. How about ServerError "an error reported by the server"? Whatever we choose we can make the classes aliases of each other, so there should be minimal compatibility problems.

Unless I'm mistaken, it seems that the ruby Trilogy extension only release the GVL when connecting to the server.

In mysql2, a result has a field_types method to get access to the field types of the result. Trilogy doesn't seem to provide any way to get access to these in the Ruby layer. The C driver does provide it, but it's not exposed in Ruby.

The Ruby driver does use it for casting purposes, so it's internally available. See also:

There the data is read into column_info but that data is ephemeral for the result object and not included in the final result.

We were looking to move to Trilogy, but we do need access to this field type information in some way or another. From purely that perspective, it doesn't have to be compatible 1:1 with how mysql2 works, as long as it's available in some way.

I'm happy to contribute a PR to add this, but I don't really know what the preferred way of adding this would be. Should it be for example to column_info and then lazily create entries in a field_types method? Or should we build field_types directly like fields is built (although often it won't really be needed, as is clear since it's not present yet and already extensively used with Rails).

Or should there be a totally different function than field_types that returns the types not as strings like mysql2 does, but with integer constants + any of the additional flags / size attributes etc. for the type? If there's any preference with how to make this available, I'm also able to start a PR for that then.

info enviroment

windows 11
ruby version 3.2.2
rails version 7.1.2

rails install with ruby installer
just try to install this gem on that machine but error appear with log :

`Temporarily enhancing PATH for MSYS/MINGW...
Building native extensions. This could take a while...
ERROR: Error installing trilogy:
ERROR: Failed to build gem native extension.

current directory: C:/Ruby32-x64/lib/ruby/gems/3.2.0/gems/trilogy-2.6.0/ext/trilogy-ruby

C:/Ruby32-x64/bin/ruby.exe extconf.rb
checking for CRYPTO_malloc() in -lcrypto... yes
checking for SSL_new() in -lssl... yes
checking for rb_interned_str() in ruby.h... yes
creating Makefile

current directory: C:/Ruby32-x64/lib/ruby/gems/3.2.0/gems/trilogy-2.6.0/ext/trilogy-ruby
make DESTDIR= sitearchdir=./.gem.20231203-27348-kv5wwy sitelibdir=./.gem.20231203-27348-kv5wwy clean

current directory: C:/Ruby32-x64/lib/ruby/gems/3.2.0/gems/trilogy-2.6.0/ext/trilogy-ruby
make DESTDIR= sitearchdir=./.gem.20231203-27348-kv5wwy sitelibdir=./.gem.20231203-27348-kv5wwy
generating cext-x64-mingw-ucrt.def
compiling trilogy.c
trilogy.c:2:10: fatal error: poll.h: No such file or directory
2 | #include <poll.h>
| ^~~~~~~~
compilation terminated.
make: *** [Makefile:248: trilogy.o] Error 1

make failed, exit code 2

Gem files will remain installed in C:/Ruby32-x64/lib/ruby/gems/3.2.0/gems/trilogy-2.6.0 for inspection.
Results logged to C:/Ruby32-x64/lib/ruby/gems/3.2.0/extensions/x64-mingw-ucrt/3.2.0/trilogy-2.6.0/gem_make.out
`
thanks

So far I only saw flakes in the CI:

• SslTest#test_raise_proper_invalid_ssl_state [/app/contrib/ruby/test/ssl_test.rb:137]
• ClientTest#test_cast_exception_during_query_does_not_close_the_connection [/app/contrib/ruby/test/client_test.rb:561]
• ClientTest#test_close_terminate_parent_connection [/Users/runner/work/trilogy/trilogy/contrib/ruby/test/client_test.rb:900]

I wrote codes with trilogy and async.

client = Trilogy.new(host: "127.0.0.1", port: 3306, username: "root", password: "test")

barrier = Async::Barrier.new

Sync do |task|
  2.times.map do
    barrier.async do
      client.query("SHOW DATABASES")
    end
  end.map(&:wait)
ensure
  barrier.stop
end

It returns the following error.

❯ ruby tril_to_mysql.rb
/home/koyam/.rbenv/versions/debug/lib/ruby/gems/3.3.0+0/gems/io-event-1.3.2/lib/io/event/support.rb:24: warning: IO::Buffer is experimental and both the Ruby and C interface may change in the future!
tril_to_mysql.rb:40:in `query': trilogy_query_recv: TRILOGY_INVALID_SEQUENCE_ID (Trilogy::QueryError)
        from tril_to_mysql.rb:40:in `block (3 levels) in <main>'
        from /home/koyam/.rbenv/versions/debug/lib/ruby/gems/3.3.0+0/gems/async-2.6.4/lib/async/task.rb:160:in `block in run'
        from /home/koyam/.rbenv/versions/debug/lib/ruby/gems/3.3.0+0/gems/async-2.6.4/lib/async/task.rb:330:in `block in schedule'

However, it is not so surprising for me because the integration the above two gems is now under development.
(I'm watching this pr and assuming that further integration is expected later.)

Then, I decided to try rewriting using threads.

 client = Trilogy.new(host: "127.0.0.1", port: 3306, username: "root", password: "test")
threads = 2.times.map do
  Thread.new do
    res = client.query("SHOW DATABASES")
    puts res
  end
end
threads.map(&:join)

But I still encountered the same error.
I thought the client(one TCP socket) should not be shared between threads.

So, I fixed the above code to create a new client per thread:

threads = 2.times.map do
  Thread.new do
    # create a new client here
    client = Trilogy.new(host: "127.0.0.1", port: 3306, username: "root", password: "test")
    res = client.query("SHOW DATABASES")
    puts res
  end
end
threads.map(&:join)

At first sight, the code does not report any error!

However, if I dump TCP packets using Wireshark, MySQL Server still shows the same error.

Frame 39: 121 bytes on wire (968 bits), 121 bytes captured (968 bits) on interface lo, id 0
Ethernet II, Src: 00:00:00_00:00:00 (00:00:00:00:00:00), Dst: 00:00:00_00:00:00 (00:00:00:00:00:00)
Internet Protocol Version 4, Src: 127.0.0.1, Dst: 127.0.0.1
Transmission Control Protocol, Src Port: 3306, Dst Port: 37856, Seq: 285, Ack: 141, Len: 55
MySQL Protocol - response ERROR
    Packet Length: 51
    Packet Number: 0
    Response Code: ERR Packet (0xff)
    Error Code: 1158
    SQL state: 08S01
    Error message: Got an error reading communication packets

Is this an expected behavior? Or maybe did I write something the wrong codes...?
If so, please close this issue, and sorry for taking your time.

Thank you for your time.

❯ uname -a
Linux kali 6.5.0-rc6-linux-surface #9 SMP PREEMPT_DYNAMIC Wed Aug 16 14:49:51 MSK 2023 x86_64 GNU/Linux

❯ ruby -v
ruby 3.3.0dev (2023-10-05T04:19:09Z master a472fd55da) [x86_64-linux]

# Gemfile
gem "trilogy", "~> 2.6"

Hi folks!

Encountering something a bit odd -- when performing SUM functions, the result is returned as a BigDecimal . A simple repro:

diff --git a/contrib/ruby/test/client_test.rb b/contrib/ruby/test/client_test.rb
index d7bc60c..71ce771 100644
--- a/contrib/ruby/test/client_test.rb
+++ b/contrib/ruby/test/client_test.rb
@@ -132,6 +132,18 @@ class ClientTest < TrilogyTest
     assert_equal [1, 4, 2, 3, 3, 1], result.rows
   end
 
+  def test_trilogy_sum_query
+    client = new_tcp_client
+    create_test_table(client)
+
+    client.query("INSERT INTO trilogy_test (int_test) VALUES ('4')")
+    client.query("INSERT INTO trilogy_test (int_test) VALUES ('3')")
+    client.query("INSERT INTO trilogy_test (int_test) VALUES ('1')")
+
+    result = client.query("SELECT SUM(int_test) FROM trilogy_test")
+    puts result.inspect
+  end
+
   def test_trilogy_query_result_object
     client = new_tcp_client

➜  ruby git:(main) ✗ be rake test TEST=test/client_test.rb TESTOPTS="--name='test_trilogy_sum_query'"
install -c tmp/arm64-darwin21/cext/3.1.2/cext.bundle lib/trilogy/cext.bundle
cp tmp/arm64-darwin21/cext/3.1.2/cext.bundle tmp/arm64-darwin21/stage/lib/trilogy/cext.bundle
/opt/rubies/3.1.2/bin/ruby -w -I"lib:test" -I"/Users/adriannachang/.gem/ruby/3.1.2/gems/rake-13.0.1/lib" "/Users/adriannachang/.gem/ruby/3.1.2/gems/rake-13.0.1/lib/rake/rake_test_loader.rb" "test/client_test.rb" --name='test_trilogy_sum_query'
Run options: --name=test_trilogy_sum_query --seed 62033

# Running:

#<Trilogy::Result:0x0000000104988730 @fields=["SUM(int_test)"], @rows=[[0.8e1]], @query_time=0.000148>
.

Finished in 0.091646s, 10.9116 runs/s, 0.0000 assertions/s.

I'm wondering if this is a bug, or if I'm just missing a query option to cast the result to an Integer properly. At Shopify we're using Sorbet, and it complains loudly if a query ends up returning a BigDecimal when it was expecting an Integer.

Appreciate the help! ❤️

When using trilogy >= 2.5.0 with Vitess and connected to a vtgate (the proxy that mysql clients connect to), trilogy raises trilogy_query_recv: SSL Error: (null) (Trilogy::SSLError) when the vtgate shuts down and closes the connection on the server side. This issue did not occur in Trilogy v2.4.1, which raised trilogy_query_recv: TRILOGY_CLOSED_CONNECTION (Trilogy::QueryError).

This error makes it hard for clients to know whether they should reconnect and retry the query.

e.g. with 2.6.0

irb(main):001:0> vtgate_client = Trilogy.new(host: "127.0.0.1", port: 23306, ssl: true, ssl_mode: Trilogy::SSL_REQUIRED_NOVERIFY)
=> #<Trilogy:0x00007f855c19eb30 @connection_options={:host=>"127.0.0.1", :port=>23306, :ssl=>true, :ssl_mode=>3}>

# vtgate is stopped

irb(main):002:0> vtgate_client.query("SELECT 1")
(irb):2:in `query': trilogy_query_recv: SSL Error: (null) (Trilogy::SSLError)
	from (irb):2:in `<main>'
	from /opt/rubies/ruby-3.1.4-pshopify1/lib/ruby/gems/3.1.0/gems/irb-1.4.1/exe/irb:11:in `<top (required)>'
	from /usr/local/bin/irb:25:in `load'
	from /usr/local/bin/irb:25:in `<main>'

mysql2 raises this as a Mysql2::Error::ConnectionError, e.g.

irb(main):002:0> mysql2_vtgate_client = Mysql2::Client.new(host: "127.0.0.1", port: 23306, ssl_mode: :required)
=>
#<Mysql2::Client:0x00007f275fcd7480

# vtgate is stopped

irb(main):003:0> mysql2_vtgate_client.query("SELECT 1")
/home/spin/.local/share/gem/ruby/3.1.0/gems/mysql2-0.5.5/lib/mysql2/client.rb:151:in `_query': Lost connection to MySQL server during query (Mysql2::Error::ConnectionError)
	from /home/spin/.local/share/gem/ruby/3.1.0/gems/mysql2-0.5.5/lib/mysql2/client.rb:151:in `block in query'
	from /home/spin/.local/share/gem/ruby/3.1.0/gems/mysql2-0.5.5/lib/mysql2/client.rb:150:in `handle_interrupt'
	from /home/spin/.local/share/gem/ruby/3.1.0/gems/mysql2-0.5.5/lib/mysql2/client.rb:150:in `query'
	from (irb):3:in `<main>'
	from /opt/rubies/ruby-3.1.4-pshopify1/lib/ruby/gems/3.1.0/gems/irb-1.4.1/exe/irb:11:in `<top (required)>'
	from /usr/local/bin/irb:25:in `load'
	from /usr/local/bin/irb:25:in `<main>'

Running a git bisect, it looks like this behavior was introduced in #112

If you're interested in reproducing locally, I've added an SSL-enabled vtgate to docker-compose.yml in this branch.

While working on #139 I noticed something suspicious.

We have a number of statements like this:

    if ((val = rb_hash_lookup(opts, ID2SYM(id_host))) != Qnil) {
        Check_Type(val, T_STRING);

        connopt.hostname = StringValueCStr(val);

opts is held as @connection_options on the Trilogy instance, so val and it's char * won't be GCed, however:

• We don't ensure the string is frozen, so it could be mutated later on.
• We these strings aren't pined, so if they are embedded, they could be moved elsewhere, causing connopt to point at garbage / another object.

It looks like trilogy doesn't allow for cleartext password and will always hash the password:

We're using RDS IAM auth and we need to be able to send the token as a cleartext password. Is this something trilogy should support or is there a good reason for not being able to send cleartext password?

Currently the gem raise many different classes of errors, from what I gathered Trilogy#query can raise:

• Errno::ETIMEDOUT on timeout
• IOError when trying to use a closed connection.
• Trilogy::Error for casting errors (and others?)

With database client it's generally very useful to have one base exception type that can be rescued, e.g. Redis::BaseConnectionError, otherwise it's too easy to forget one of the possible type of error on query.

Backward compatibility

To retain backward compatibility it would be possible to use modules, e.g.:

module Trilogy
   ConnectionErrors = Module.new

   class TimeoutError < Errno::ETIMEDOUT
     include ConnectionErrors
   end

   class ConnectionClosed < IOError
     include ConnectionErrors
   end

This would both allow to rescue all connection related errors with rescue Trilogy::ConnectionErrors while retaining compatibility with existing code using rescue Errno::ETIMEDOUT, IOError.

What do you think?

Currently, it looks like trilogy tries to run against two old Ruby versions, 1.9 and 2.8.

But, this is strange because Ruby 2.8 doesn't exist, the last version of 2.x series is 2.7. It seems that the build for 1.9 and 2.8 uses Ruby 3.2.

Build (5.7, debian:buster, 1.9)

I think this is because FROM is set after ARG RUBY_VERSION.

FROM creates a new scope and you need to define ARG after FROM if you want to use it inside the scope.
https://docs.docker.com/engine/reference/builder/#scope

So the current Dockerfile ignores RUBY_VERSION arg and always use Ruby 3.2.

I'm not sure trilogy's support policy for old Ruby versions. So I submitted an issue for just for your information.

We've noticed a crash on Semian's CI: https://github.com/Shopify/semian/actions/runs/4122969863/jobs/7120418231

-- C level backtrace information -------------------------------------------
/usr/local/lib/libruby.so.3.1(rb_print_backtrace+0x11) [0x7f11d3368348] vm_dump.c:759
/usr/local/lib/libruby.so.3.1(rb_vm_bugreport) vm_dump.c:1045
/usr/local/lib/libruby.so.3.1(rb_bug_for_fatal_signal+0xf0) [0x7f11d3166800] error.c:821
/usr/local/lib/libruby.so.3.1(sigsegv+0x49) [0x7f11d32be429] signal.c:964
/lib/x[86](https://github.com/Shopify/semian/actions/runs/4122969863/jobs/7120418231#step:7:87)_64-linux-gnu/libpthread.so.0(__restore_rt+0x0) [0x7f11d305d140]
/__w/semian/semian/gemfiles/vendor/bundle/ruby/3.1.0/bundler/gems/trilogy-2dd91e5d1974/contrib/ruby/lib/trilogy/cext.so(trilogy_sock_read+0xb) [0x7f11ce71c8ad] /__w/semian/semian/gemfiles/vendor/bundle/ruby/3.1.0/bundler/gems/trilogy-2dd91e5d1974/contrib/ruby/ext/trilogy-ruby/inc/trilogy/socket.h:[87](https://github.com/Shopify/semian/actions/runs/4122969863/jobs/7120418231#step:7:88)
/__w/semian/semian/gemfiles/vendor/bundle/ruby/3.1.0/bundler/gems/trilogy-2dd91e5d1974/contrib/ruby/lib/trilogy/cext.so(trilogy_query_recv) trilogy.c:782
/__w/semian/semian/gemfiles/vendor/bundle/ruby/3.1.0/bundler/gems/trilogy-2dd91e5d1974/contrib/ruby/lib/trilogy/cext.so(trilogy_query_recv) (null):0
/__w/semian/semian/gemfiles/vendor/bundle/ruby/3.1.0/bundler/gems/trilogy-2dd91e5d1974/contrib/ruby/lib/trilogy/cext.so(read_query_response+0x5f) [0x7f11ce71f00f] cext.c:612
/usr/local/lib/libruby.so.3.1(rb_protect+0xec) [0x7f11d316efec] eval.c:967
/__w/semian/semian/gemfiles/vendor/bundle/ruby/3.1.0/bundler/gems/trilogy-2dd91e5d1974/contrib/ruby/lib/trilogy/cext.so(execute_read_query_response+0x8a) [0x7f11ce71edca] cext.c:741
/__w/semian/semian/gemfiles/vendor/bundle/ruby/3.1.0/bundler/gems/trilogy-2dd91e5d1974/contrib/ruby/lib/trilogy/cext.so(rb_trilogy_query+0x63) [0x7f11ce71f9f3] cext.c:799
/usr/local/lib/libruby.so.3.1(vm_cfp_consistent_p+0x0) [0x7f11d333f40c] vm_insnhelper.c:3037
/usr/local/lib/libruby.so.3.1(vm_call_cfunc_with_frame) vm_insnhelper.c:3039
/usr/local/lib/libruby.so.3.1(vm_sendish+0x4e) [0x7f11d334e3[89](https://github.com/Shopify/semian/actions/runs/4122969863/jobs/7120418231#step:7:90)] vm_insnhelper.c:4751
/usr/local/lib/libruby.so.3.1(vm_exec_core) insns.def:778
/usr/local/lib/libruby.so.3.1(rb_vm_exec+0xcf) [0x7f11d33537ff] vm.c:2211
/usr/local/lib/libruby.so.3.1(rb_vm_invoke_proc+0x57) [0x7f11d3358e27] vm.c:1521
/usr/local/lib/libruby.so.3.1(thread_do_start_proc+0x12f) [0x7f11d3308cff] thread.c:735
/usr/local/lib/libruby.so.3.1(thread_do_start+0x14) [0x7f11d330[94](https://github.com/Shopify/semian/actions/runs/4122969863/jobs/7120418231#step:7:95)14] thread.c:754
/usr/local/lib/libruby.so.3.1(thread_start_func_2) thread.c:828
/usr/local/lib/libruby.so.3.1(register_cached_thread_and_wait+0x0) [0x7f11d3309e19] thread_pthread.c:[104](https://github.com/Shopify/semian/actions/runs/4122969863/jobs/7120418231#step:7:105)7
/usr/local/lib/libruby.so.3.1(thread_start_func_1) thread_pthread.c:[105](https://github.com/Shopify/semian/actions/runs/4122969863/jobs/7120418231#step:7:106)4
/lib/x86_64-linux-gnu/libpthread.so.0(0x7ea7) [0x7f11d3051ea7]
/lib/x86_64-linux-gnu/libc.so.6(clone+0x3f) [0x7f11d2d5fa2f]

We tried to debug it, unfortunately we weren't able to find a reproduction.

Hey trilogy team,

It seems that MySQL client library translates connections to localhost to a local UNIX socket, instead of making a TCP connection to localhost address - which is 127.0.0.1.

Such behavior was initially reported to MySQL team on 2007, and it seems that they claim it's not a bug, regardless if I use the mysql CLI tool, mysql2 gem, but trilogy is handling that case in the correct way.

I need to borrow your wisdom on this topic:

1. how did you solve this issue?
2. is it possible to point me in the code where did you solve it?

One of Rails unit testBulkAlterTableMigrationsTest#test_default_functions_on_columns fails against trilogy and MariaDB.

Steps to reproduce

1. Install MariaDB

MariaDB [test]> select version();
+----------------+
| version()      |
+----------------+
| 11.1.1-MariaDB |
+----------------+
1 row in set (0.000 sec)

2. Run Active Record unit test

git clone https://github.com/rails/rails
cd rails/activerecord
bundle install
ARCONN=trilogy bin/test test/cases/migration_test.rb -n test_default_functions_on_columns

Expected behavior

It should pass.

Actual behavior

$ ARCONN=trilogy bin/test test/cases/migration_test.rb -n test_default_functions_on_columns
Using trilogy
Run options: -n test_default_functions_on_columns --seed 54410

# Running:

E

Error:
BulkAlterTableMigrationsTest#test_default_functions_on_columns:
ArgumentError: wrong number of arguments (given 1, expected 0)
    /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/bundler/gems/trilogy-8e4ae98569c1/contrib/ruby/lib/trilogy.rb:233:in `each'
    /home/yahonda/src/github.com/rails/rails/activerecord/test/cases/migration_test.rb:1472:in `to_a'
    /home/yahonda/src/github.com/rails/rails/activerecord/test/cases/migration_test.rb:1472:in `test_default_functions_on_columns'


bin/test test/cases/migration_test.rb:1454



Finished in 0.208410s, 4.7982 runs/s, 9.5965 assertions/s.
1 runs, 2 assertions, 0 failures, 1 errors, 0 skips
$

Hello I was running Scorecard against the repo and the score for Token-Permissions was quite low.

Might be great to update top-level permissions on the CI jobs to read-all if possible.

We shipped the C API for prepared statements in #3. The next step is to add support to the Ruby bindings.

API

I imagine we'll want something similar to Mysql2's implementation: a Trilogy#prepare method that returns a Trilogy::Statement object, and a Trilogy::Statement#execute method that can be used to execute a PS with given parameters, e.g.:

client = Trilogy.new(host: "127.0.0.1", port: 3306, username: "root", read_timeout: 2, database: "trilogy_test")

statement = client.prepare("SELECT * FROM users WHERE id = ?")
result1 = statement.execute(1)
result2 = statement.execute(2)

statement = client.prepare("SELECT * FROM users WHERE id >= ? AND name LIKE ?")
result = statement.execute(1, "Joe")

Trilogy::Statement should, at a minimum, expose the underlying data values from the C API: the statement id, column count, parameter count, and warning count. Mysql2::Statement exposes things like affected_rows, fields, last_id, etc. that we may eventually want Trilogy::Statement to expose as well.

Blockers

@byroot and I began working on this and ran into some issues while fleshing out the Ruby class that will wrap the C prepared statement struct (trilogy_stmt_t). As Mysql2 does, we'll need to close the prepared statement when freeing the Ruby Trilogy::Statement object:
https://github.com/brianmario/mysql2/blob/6cf5e1d732b4a6ee5485def98637f100cbc86f1b/ext/mysql2/statement.c#L26-L29
https://github.com/brianmario/mysql2/blob/master/ext/mysql2/statement.c#L75
https://github.com/brianmario/mysql2/blob/master/ext/mysql2/statement.c#L65

trilogy_stmt_close needs the stmt and the connection:

We need the connection to send TRILOGY_CMD_STMT_CLOSE -- so we can create an intermediate struct (that will be wrapped into Trilogy::Statement) that encapsulates both trilogy_stmt_t and the connection. The problem arises if the connection ends up being closed / freed before we attempt to close the prepared statement / free its memory. In this case, attempting to access the connection on the statement wrapper will raise an EXC_BAD_ACCESS exception.

The solution is likely to do something similar to what the native SQL client library does -- it stores a linked list of all prepared statements on the client itself, and then when the client is closed, it "detaches" the statement list by clearing the connection pointer of every statement. That way, we can no-op in trilogy_stmt_close if the connection is gone, and avoid accessing memory for a conn that's already been freed.

If anyone else has opinions on this though, would love to hear!

I've started a branch here for the Ruby bindings, but it's still very much a WIP: https://github.com/trilogy-libraries/trilogy/compare/main...adrianna-chang-shopify:trilogy:ac-prepared-statements-ruby?expand=1

Compiling Ruby takes quite a bit of time, even on multi-core machines. Perhaps the compilation step could be made parallel?

Something like make -j$(nproc --all)