mysql icon indicating copy to clipboard operation
mysql copied to clipboard

Add support for MySQL Connection Attributes

Open andygrunwald opened this issue 4 years ago • 7 comments

Description

This Pull Request adds support for sending connection attributes, which are used for identifying individual connections in MySQL. See MySQL docs: Performance Schema Connection Attribute Tables.

This feature is supported since MySQL v 5.6.6.

The main work of this pull request was done by @Vasfed in https://github.com/go-sql-driver/mysql/pull/737.

This Pull Request reintegrates https://github.com/go-sql-driver/mysql/pull/737 with the current master and gets all unit tests running.

Checklist

  • [X] Code compiles correctly
  • [ ] Created tests which fail without the change (if possible) -> Not applicable
  • [X] All tests passing
  • [X] Extended the README / documentation, if necessary
  • [ ] Added myself / the copyright holder to the AUTHORS file -> Skipped, because my work was minimal

andygrunwald avatar Jul 31 '21 19:07 andygrunwald

I added a small example to showcase / use this feature: https://github.com/andygrunwald/your-connection-deserves-a-name/blob/mysql-go/mysql/go/main.go

This code is using my fork, and I can confirm that this feature is working as expected.

andygrunwald avatar Jul 31 '21 19:07 andygrunwald

@methane @shogo82148 Just checking if there is any update on this or if you found time to have a look at this pull request?

andygrunwald avatar Oct 17 '21 15:10 andygrunwald

I found out that the test TestRawBytesAreNotModified doesn't seem to be stable / a bit flaky. In the test run 1606512033 it failed:

[...]
=== RUN   TestRawBytesAreNotModified
    driver_test.go:3075: context canceled
--- FAIL: TestRawBytesAreNotModified (0.31s)
[...]

Re-running the workflow (without any additional changes) succeeded.

If you wish that I create a dedicated ticket for this, please let me know.

andygrunwald avatar Dec 21 '21 13:12 andygrunwald

@dolmen @methane Thanks for your reviews so far. I incorporated your feedback and re-integrated this branch with master. I kindly ask for a re-review. Let me know if there is anything else

andygrunwald avatar Dec 31 '21 10:12 andygrunwald

@andygrunwald – Any updates on support for connection attributes? We'd love to see this feature land and would be happy to help out if needed.

I've reintegrated master with this branch locally – no conflicts and I can confirm all tests are still passing.

@methane and @dolmen – any advice for moving this forward? I'm happy to take over in another PR if needed. Seems like this is really close and would be a helpful feature to land.

fulghum avatar Jun 21 '22 20:06 fulghum

This pull request contains many changes unrelating to the feature requested. That made both of author and me tired. Please make PR minimum next time. But I can't say when I can review and merge it.

methane avatar Jun 22 '22 01:06 methane

@fulghum @methane Your comments are fair and I should have done a better job here. This PR is not (fully) my work. I grabbed the work from @Vasfed in https://github.com/go-sql-driver/mysql/pull/737 and try to adjust it to the latest main.

It might be a good thing to re-do this again and compare the changes here to the MySQL spec and see what is really needed.

Right now, I am not able to do this, due to limited time available. Just a heads up to set expectations. If someone is reading this and eager to jump in it: Go for it. Otherwise, I will take this on once I have a bit of spare time.

andygrunwald avatar Jun 23 '22 07:06 andygrunwald