-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
client,mysql: Add support for Query Attributes #976
Conversation
@lance6716 @atercattus this works and is ready for review. However this needs some unit tests etc and more manual testing before it can be merged. Should I add a |
@dveeden seems CI is broken. It may be a good sign to modify the failing cases to cover the new code. |
Example package main
import (
"fmt"
"github.com/go-mysql-org/go-mysql/client"
"github.com/go-mysql-org/go-mysql/mysql"
)
func main() {
conn, err := client.Connect("127.0.0.1:3306", "root", "", "",
func(c *client.Conn) error {
c.IncludeLine(2)
return nil
},
)
if err != nil {
panic(err)
}
defer conn.Quit()
proxyAttr := mysql.QueryAttribute{
Name: "proxy_user",
Value: "user1",
}
secondAttr := mysql.QueryAttribute{
Name: "attr2",
Value: "value2",
}
conn.SetQueryAttributes(proxyAttr, secondAttr)
fmt.Printf("Sending COM_QUERY \"SELECT mysql_query_attribute_string('proxy_user')\" with attributes: %v, %v\n", proxyAttr, secondAttr)
r, err := conn.Execute("SELECT mysql_query_attribute_string('proxy_user')")
if err != nil {
panic(err)
}
defer r.Close()
for _, row := range r.Values {
if len(row) != 1 {
panic("expecting exactly one column")
}
fmt.Printf("%s\n", row[0].Value())
}
fmt.Printf("Preparing 'SELECT mysql_query_attribute_string(?)' via COM_STMT_PREPARE\n")
stmt, err := conn.Prepare("SELECT mysql_query_attribute_string(?)")
if err != nil {
panic(err)
}
defer stmt.Close()
conn.SetQueryAttributes(proxyAttr)
fmt.Printf("Sending COM_STMT_EXECUTE with arg: %s, attributes: %v\n", "proxy_user", proxyAttr)
res, err := stmt.Execute("proxy_user")
if err != nil {
panic(err)
}
defer res.Close()
for _, row := range res.Values {
if len(row) != 1 {
panic("expecting exactly on column")
}
fmt.Printf("%s\n", row[0].Value())
}
} This lets users:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will check protocol code soon
Co-authored-by: lance6716 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5 / 6 files viewed
// - https://archive.fosdem.org/2021/schedule/event/mysql_protocl/ | ||
type QueryAttribute struct { | ||
Name string | ||
Value interface{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems we only support golang string
and uint64
as the type of Value
. Please add this usage in comments.
BTW I haven't clearly understood what types are supported in MySQL query attributes. I have found the function mysql_query_attribute_string
, so MySQL only supports string type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mysql_query_attribute_string()
function returns the value of the query attribute as string. But a client may use other types to actually send data.
https://dev.mysql.com/doc/refman/9.1/en/query-attributes.html#query-attribute-functions
Here is an example which uses an int
/MYSQL_TYPE_LONG
and a string.
#include <stdio.h>
#include <string.h>
#include <mysql.h>
MYSQL mysql;
int main() {
MYSQL_BIND bind[2];
char *mystr = "test test";
const char *name[2] = {"foo", "bar"};
unsigned int ssl_mode = SSL_MODE_DISABLED;
int intparam = 1;
mysql_init(&mysql);
mysql_options(&mysql,MYSQL_OPT_SSL_MODE, &ssl_mode);
if (!mysql_real_connect(&mysql, "127.0.0.1", "root", "", "test", 0, NULL, 0)) {
fprintf(stderr, "Failed to connect to database: Error: %s\n",
mysql_error(&mysql));
}
unsigned long mystrlen = strlen(mystr);
bind[0].buffer_type = MYSQL_TYPE_STRING;
bind[0].buffer = mystr;
bind[0].length = &mystrlen;
bind[0].is_null = 0;
unsigned long intparamlen = sizeof(int);
bind[1].buffer_type = MYSQL_TYPE_LONG;
bind[1].buffer = (char *) &intparam;
bind[1].length = &intparamlen;
bind[1].is_null = 0;
mysql_bind_param(&mysql, 2, bind, name);
const char *query = "SELECT mysql_query_attribute_string('foo'), mysql_query_attribute_string('bar')";
mysql_real_query(&mysql, query, strlen(query));
MYSQL_RES *result = mysql_store_result(&mysql);
MYSQL_ROW row = mysql_fetch_row(result);
unsigned long *lengths = mysql_fetch_lengths(result);
for(int i = 0; i < 2; i++)
{
printf("attribute %d: [%.*s]\n", i+1, (int) lengths[i], row[i] ? row[i] : "NULL");
}
mysql_free_result(result);
mysql_close(&mysql);
}
dvaneeden@dve-carbon:~/dev/mysql-qattr$ gcc -Wall -Wextra $(mysql_config --cflags --libs) -o qattr qattr.c
dvaneeden@dve-carbon:~/dev/mysql-qattr$ ./qattr
attribute 1: [test test]
attribute 2: [1]
@@ -76,76 +92,89 @@ func (s *Stmt) write(args ...interface{}) error { | |||
for i := range args { | |||
if args[i] == nil { | |||
nullBitmap[i/8] |= 1 << (uint(i) % 8) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this part is a bit complicated, I'll review later 😂
mysql/mysql-server#595 / http://bugs.mysql.com/bug.php?id=117391 might help while reviewing this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I can help to add test in next PR.
if numParams > 0 { | ||
// null_bitmap, length: (num_params+7)/8 | ||
for i := 0; i < (numParams+7)/8; i++ { | ||
buf.WriteByte(0x0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we allow NULL query attributes so the bitmap will have 1
? 🤔 we can add it in next PR.
paramsNum := s.params | ||
|
||
if len(args) != paramsNum { | ||
return fmt.Errorf("argument mismatch, need %d but got %d", s.params, len(args)) | ||
} | ||
|
||
paramTypes := make([]byte, paramsNum<<1) | ||
paramValues := make([][]byte, paramsNum) | ||
if (s.conn.capability&CLIENT_QUERY_ATTRIBUTES > 0) && (s.conn.includeLine >= 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (s.conn.capability&CLIENT_QUERY_ATTRIBUTES > 0) && (s.conn.includeLine >= 0) { | |
if s.conn.capability&CLIENT_QUERY_ATTRIBUTES > 0 && s.conn.includeLine >= 0 { |
the old code has clear priority. It's OK to keep it.
linter error
|
Just like connection attributes, there are also query attributes in MySQL. These allow one to pass key/value pairs along with queries.
These can be used to add metadata about:
The page being rendered
The user for the proxy connection
A trace ID for debugging
A shard-id hint
A follower-read setting
An application stack listing
The line of the application code sending the query
https://dev.mysql.com/doc/refman/8.4/en/query-attributes.html
https://archive.fosdem.org/2021/schedule/event/mysql_protocl/
In some other languages, etc:
MySQL Client
MySQL Connector/Python
Output:
go-mysql with this PR
Output: