Support optional resultset metadata #1150
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tz70s
force-pushed
the
support-resultset-optional-metadata
branch
2 times, most recently
from
d9cf63e
to
46e70d0
Compare
tz70s
changed the title
tz70s
force-pushed
the
support-resultset-optional-metadata
branch
7 times, most recently
from
21f614e
to
38aa54a
Compare
|
Not really sure why seems this test is flaky in OSX travis |
tz70s
force-pushed
the
support-resultset-optional-metadata
branch
from
51e4ca4
to
b819a69
Compare
dolmen
suggested changes
Dec 15, 2021
README.md
Outdated
|
|
||
| Allow resultset metadata being optional. | ||
| By making resultset metadata transfer being optional, can potentially improve queries performance. | ||
|
|
There was a problem hiding this comment.
Document that this requires MySQL 8.0+.
Add link to MySQL documentation: https://dev.mysql.com/doc/refman/8.0/en/server-system-variables.html#sysvar_resultset_metadata
dsn.go
Outdated
| @@ -465,6 +470,18 @@ func parseDSNParams(cfg *Config, params string) (err error) { | |||
| return errors.New("invalid bool value: " + value) | |||
| } | |||
|
|
|||
| // allow resultset metadata being optional | |||
| case "resultSetMetadata": | |||
There was a problem hiding this comment.
As a mysql variable resultset_metadata already exists, it would be better to use it (like time_zone).
connector.go
Outdated
| @@ -129,6 +129,24 @@ func (c *connector) Connect(ctx context.Context) (driver.Conn, error) { | |||
| mc.maxWriteSize = mc.maxAllowedPacket | |||
| } | |||
|
|
|||
| // Additional handling for result set optional metadata | |||
| if mc.cfg.ResultSetMetadata != "" { | |||
| err = mc.exec("SET resultset_metadata=" + mc.cfg.ResultSetMetadata) | |||
There was a problem hiding this comment.
handleParams sends all SET at once. It would be better to use that. See also my remark about the option name.
dsn.go
Outdated
| Timeout time.Duration // Dial timeout | ||
| ReadTimeout time.Duration // I/O read timeout | ||
| WriteTimeout time.Duration // I/O write timeout | ||
| ResultSetMetadata string // Allow optional resultset metadata |
There was a problem hiding this comment.
Not needed if handled il Params like other MySQL variables.
errors.go
Outdated
| ErrPktTooLarge = errors.New("packet for query is too large. Try adjusting the 'max_allowed_packet' variable on the server") | ||
| ErrBusyBuffer = errors.New("busy buffer") | ||
| ErrNoOptionalResultSet = errors.New("requested optional resultset metadata but server does not support") | ||
| ErrOptionalResultSetPkt = errors.New("malformed optional resultset metadata packets") |
There was a problem hiding this comment.
Those variable should contain Metadata in the name, because that's not the resultset which is optional but just the metadata.
tz70s
force-pushed
the
support-resultset-optional-metadata
branch
3 times, most recently
from
cb6c794
to
acbdeec
Compare
Allow optional resultset metadata. Can potentially improve the performance in many scenarios. Issue go-sql-driver#1105
tz70s
force-pushed
the
support-resultset-optional-metadata
branch
from
acbdeec
to
0b7ff91
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Allow optional resultset metadata.
Can potentially improve the performance in many scenario.
Issue #1105
Note
I believe this flag was introduced in MySQL 8.0+ only.
Some open questions are:
maybeSkip.Need your feedback on this, thank you!
Description
Add support optional resultset metadata.
Checklist