Re: [Maria-developers] [Commits] 2606b87: MDEV-9117: Client Server capability negotiation for MariaDB specific functionality
Hi, Sanja! On Nov 20, sanja@mariadb.com wrote:
revision-id: 2606b8744404f352b44c85ee23d41b12cddc3470 (mariadb-10.1.8-51-g2606b87) parent(s): 81e4ce5e31ba0753d7acfab28bc6c3d83bfad1c6 committer: Oleksandr Byelkin timestamp: 2015-11-20 18:23:52 +0100 message:
MDEV-9117: Client Server capability negotiation for MariaDB specific functionality
diff --git a/client/mysql.cc b/client/mysql.cc index 92324c6..e242c25 100644 --- a/client/mysql.cc +++ b/client/mysql.cc @@ -152,7 +152,7 @@ static ulong opt_max_allowed_packet, opt_net_buffer_length; static uint verbose=0,opt_silent=0,opt_mysql_port=0, opt_local_infile=0; static uint my_end_arg; static char * opt_mysql_unix_port=0; -static int connect_flag=CLIENT_INTERACTIVE; +static ulong long connect_flag=CLIENT_INTERACTIVE;
Either "ulonglong" or "unsigned long long". But not "ulong long", please.
static my_bool opt_binary_mode= FALSE; static int interrupted_query= 0; static char *current_host,*current_db,*current_user=0,*opt_password=0, diff --git a/include/mysql.h b/include/mysql.h index f088ad6..d398734 100644 --- a/include/mysql.h +++ b/include/mysql.h @@ -270,7 +270,7 @@ typedef struct st_mysql unsigned long thread_id; /* Id for connection in server */ unsigned long packet_length; unsigned int port; - unsigned long client_flag,server_capabilities; + unsigned long long client_flag,server_capabilities;
This is an incompatible change, it breaks the ABI. If we have to have this change in libmysqlclient, you can put the new flags in the extension area. But, really, perhaps you should switch to Connector/C?
unsigned int protocol_version; unsigned int field_count; unsigned int server_status; diff --git a/sql-common/client.c b/sql-common/client.c index 609eef5..e108c4b 100644 --- a/sql-common/client.c +++ b/sql-common/client.c @@ -2672,15 +2673,26 @@ static int send_client_reply_packet(MCPVIO_EXT *mpvio, if (mysql->client_flag & CLIENT_PROTOCOL_41) { /* 4.1 server and 4.1 client has a 32 byte option flag */ - int4store(buff,mysql->client_flag); + bzero(buff+9, 32-9); + if (mysql->server_capabilities & MARIADB_CLIENT_EXTENDED_FLAGS) + { + DBUG_ASSERT(mysql->client_flag & CLIENT_LONG_PASSWORD);
1. add DBUG_ASSERT(mysql->client_flag & CLIENT_PROTOCOL_41) 2. define CLIENT_LONG_PASSWORD to be 0 and CLIENT_MYSQL to be 1. Currently CLIENT_LONG_PASSWORD is not used anywhere, it's never checked. So you can safely define it to be 0 and use bit 1 for CLIENT_MYSQL, as we've discussed.
+ /* + first part of bytes - CLIENT_LONG_PASSWORD to signal mariadb extensions + */ + mysql->client_flag|= MARIADB_CLIENT_EXTENDED_FLAGS; + int4store(buff, (mysql->client_flag & (~CLIENT_LONG_PASSWORD)));
I think MariaDB clients should never set CLIENT_MYSQL. Independently from MARIADB_CLIENT_EXTENDED_FLAGS.
+ int4store(buff + 28, (mysql->client_flag >> 32)); + } + else + int4store(buff, mysql->client_flag); int4store(buff+4, net->max_packet_size); buff[8]= (char) mysql->charset->number; - bzero(buff+9, 32-9); end= buff+32; } else { - int2store(buff, mysql->client_flag); + int2store(buff, (mysql->client_flag & 0xffff)); int3store(buff+2, net->max_packet_size); end= buff+5; } @@ -3682,7 +3697,20 @@ CLI_MYSQL_REAL_CONNECT(MYSQL *mysql,const char *host, const char *user, if ((mysql->server_capabilities & CLIENT_PLUGIN_AUTH) && strncmp(mysql->server_version, RPL_VERSION_HACK, sizeof(RPL_VERSION_HACK) - 1) == 0) + { mysql->server_version+= sizeof(RPL_VERSION_HACK) - 1; + mysql->server_capabilities|= ext_server_capabilities; + if (!(mysql->server_capabilities & MARIADB_CLIENT_EXTENDED_FLAGS) && + (mysql->client_flag & MARIADB_CLIENT_PROGRESS)) + { + /* old mariadb fix progress flag */ + mysql->client_flag|= CLIENT_PROGRESS_OBSOLETE; + if (mysql->server_capabilities & CLIENT_PROGRESS_OBSOLETE) + { + mysql->server_capabilities|= MARIADB_CLIENT_PROGRESS; + }
looks good, but please test this. New client, old server. And old client, new server too.
+ } + }
if (pkt_end >= end + SCRAMBLE_LENGTH - SCRAMBLE_LENGTH_323 + 1) { diff --git a/tests/mysql_client_test.c b/tests/mysql_client_test.c index 98f42ff..255ac25 100644 --- a/tests/mysql_client_test.c +++ b/tests/mysql_client_test.c @@ -15336,7 +15336,7 @@ static void test_mysql_insert_id()
myheader("test_mysql_insert_id");
- rc= mysql_query(mysql, "drop table if exists t1"); + rc= mysql_query(mysql, "drop table if exists t1,t2");
why?
myquery(rc); /* table without auto_increment column */ rc= mysql_query(mysql, "create table t1 (f1 int, f2 varchar(255), key(f1))"); @@ -16186,6 +16186,10 @@ static void test_change_user() sprintf(buff, "drop database if exists %s", db); rc= mysql_query(mysql, buff); myquery(rc); + sprintf(buff, "set local sql_mode=''");
why?
+ rc= mysql_query(mysql, buff); + myquery(rc);
Regards, Sergei
Hi, Sergei!
On 23.11.15 16:01, Sergei Golubchik wrote:
> Hi, Sanja!
>
> On Nov 20, sanja@mariadb.com wrote:
>> revision-id: 2606b8744404f352b44c85ee23d41b12cddc3470 (mariadb-10.1.8-51-g2606b87)
>> parent(s): 81e4ce5e31ba0753d7acfab28bc6c3d83bfad1c6
>> committer: Oleksandr Byelkin
>> timestamp: 2015-11-20 18:23:52 +0100
>> message:
>>
>> MDEV-9117: Client Server capability negotiation for MariaDB specific functionality
>>
>> diff --git a/client/mysql.cc b/client/mysql.cc
>> index 92324c6..e242c25 100644
>> --- a/client/mysql.cc
>> +++ b/client/mysql.cc
>> @@ -152,7 +152,7 @@ static ulong opt_max_allowed_packet, opt_net_buffer_length;
>> static uint verbose=0,opt_silent=0,opt_mysql_port=0, opt_local_infile=0;
>> static uint my_end_arg;
>> static char * opt_mysql_unix_port=0;
>> -static int connect_flag=CLIENT_INTERACTIVE;
>> +static ulong long connect_flag=CLIENT_INTERACTIVE;
> Either "ulonglong" or "unsigned long long".
> But not "ulong long", please.
OK
>> static my_bool opt_binary_mode= FALSE;
>> static int interrupted_query= 0;
>> static char *current_host,*current_db,*current_user=0,*opt_password=0,
>> diff --git a/include/mysql.h b/include/mysql.h
>> index f088ad6..d398734 100644
>> --- a/include/mysql.h
>> +++ b/include/mysql.h
>> @@ -270,7 +270,7 @@ typedef struct st_mysql
>> unsigned long thread_id; /* Id for connection in server */
>> unsigned long packet_length;
>> unsigned int port;
>> - unsigned long client_flag,server_capabilities;
>> + unsigned long long client_flag,server_capabilities;
> This is an incompatible change, it breaks the ABI.
> If we have to have this change in libmysqlclient, you can put the new
> flags in the extension area. But, really, perhaps you should switch to
> Connector/C?
The problem is that we have to remove CLIENT_PROGRESS (there is no such
flag in mysql)
>> unsigned int protocol_version;
>> unsigned int field_count;
>> unsigned int server_status;
>> diff --git a/sql-common/client.c b/sql-common/client.c
>> index 609eef5..e108c4b 100644
>> --- a/sql-common/client.c
>> +++ b/sql-common/client.c
>> @@ -2672,15 +2673,26 @@ static int send_client_reply_packet(MCPVIO_EXT *mpvio,
>> if (mysql->client_flag & CLIENT_PROTOCOL_41)
>> {
>> /* 4.1 server and 4.1 client has a 32 byte option flag */
>> - int4store(buff,mysql->client_flag);
>> + bzero(buff+9, 32-9);
>> + if (mysql->server_capabilities & MARIADB_CLIENT_EXTENDED_FLAGS)
>> + {
>> + DBUG_ASSERT(mysql->client_flag & CLIENT_LONG_PASSWORD);
> 1. add DBUG_ASSERT(mysql->client_flag & CLIENT_PROTOCOL_41)
> 2. define CLIENT_LONG_PASSWORD to be 0 and CLIENT_MYSQL to be 1.
I found that CLIENT_LONG_PASSWORD used without CLIENT_PROTOCOL_41in
mysqlbinlog. it will be wrong with CLIENT_LONG_PASSWORD defined as 0
#define CLIENT_CAPABILITIES (CLIENT_LONG_PASSWORD | CLIENT_LONG_FLAG
| CLIENT_LOCAL_FILES)
>
> Currently CLIENT_LONG_PASSWORD is not used anywhere, it's never checked.
> So you can safely define it to be 0 and use bit 1 for CLIENT_MYSQL, as
> we've discussed.
>
>> + /*
>> + first part of bytes - CLIENT_LONG_PASSWORD to signal mariadb extensions
>> + */
>> + mysql->client_flag|= MARIADB_CLIENT_EXTENDED_FLAGS;
>> + int4store(buff, (mysql->client_flag & (~CLIENT_LONG_PASSWORD)));
> I think MariaDB clients should never set CLIENT_MYSQL. Independently
> from MARIADB_CLIENT_EXTENDED_FLAGS.
>
>> + int4store(buff + 28, (mysql->client_flag >> 32));
>> + }
>> + else
>> + int4store(buff, mysql->client_flag);
>> int4store(buff+4, net->max_packet_size);
>> buff[8]= (char) mysql->charset->number;
>> - bzero(buff+9, 32-9);
>> end= buff+32;
>> }
>> else
>> {
>> - int2store(buff, mysql->client_flag);
>> + int2store(buff, (mysql->client_flag & 0xffff));
>> int3store(buff+2, net->max_packet_size);
>> end= buff+5;
>> }
>> @@ -3682,7 +3697,20 @@ CLI_MYSQL_REAL_CONNECT(MYSQL *mysql,const char *host, const char *user,
>> if ((mysql->server_capabilities & CLIENT_PLUGIN_AUTH) &&
>> strncmp(mysql->server_version, RPL_VERSION_HACK,
>> sizeof(RPL_VERSION_HACK) - 1) == 0)
>> + {
>> mysql->server_version+= sizeof(RPL_VERSION_HACK) - 1;
>> + mysql->server_capabilities|= ext_server_capabilities;
>> + if (!(mysql->server_capabilities & MARIADB_CLIENT_EXTENDED_FLAGS) &&
>> + (mysql->client_flag & MARIADB_CLIENT_PROGRESS))
>> + {
>> + /* old mariadb fix progress flag */
>> + mysql->client_flag|= CLIENT_PROGRESS_OBSOLETE;
>> + if (mysql->server_capabilities & CLIENT_PROGRESS_OBSOLETE)
>> + {
>> + mysql->server_capabilities|= MARIADB_CLIENT_PROGRESS;
>> + }
> looks good, but please test this. New client, old server. And old
> client, new server too.
OK
>
>> + }
>> + }
>>
>> if (pkt_end >= end + SCRAMBLE_LENGTH - SCRAMBLE_LENGTH_323 + 1)
>> {
>> diff --git a/tests/mysql_client_test.c b/tests/mysql_client_test.c
>> index 98f42ff..255ac25 100644
>> --- a/tests/mysql_client_test.c
>> +++ b/tests/mysql_client_test.c
>> @@ -15336,7 +15336,7 @@ static void test_mysql_insert_id()
>>
>> myheader("test_mysql_insert_id");
>>
>> - rc= mysql_query(mysql, "drop table if exists t1");
>> + rc= mysql_query(mysql, "drop table if exists t1,t2");
> why?
because two tables used later (was checking it separately and found this)
>
>> myquery(rc);
>> /* table without auto_increment column */
>> rc= mysql_query(mysql, "create table t1 (f1 int, f2 varchar(255), key(f1))");
>> @@ -16186,6 +16186,10 @@ static void test_change_user()
>> sprintf(buff, "drop database if exists %s", db);
>> rc= mysql_query(mysql, buff);
>> myquery(rc);
>> + sprintf(buff, "set local sql_mode=''");
> why?
Otherwise it can't make grant.
>> + rc= mysql_query(mysql, buff);
>> + myquery(rc);
> Regards,
> Sergei
Hi, Oleksandr! On Nov 23, Oleksandr Byelkin wrote:
diff --git a/include/mysql.h b/include/mysql.h index f088ad6..d398734 100644 --- a/include/mysql.h +++ b/include/mysql.h @@ -270,7 +270,7 @@ typedef struct st_mysql unsigned long thread_id; /* Id for connection in server */ unsigned long packet_length; unsigned int port; - unsigned long client_flag,server_capabilities; + unsigned long long client_flag,server_capabilities; This is an incompatible change, it breaks the ABI. If we have to have this change in libmysqlclient, you can put the new flags in the extension area. But, really, perhaps you should switch to Connector/C? The problem is that we have to remove CLIENT_PROGRESS (there is no such flag in mysql)
Right. So either we can keep CLIENT_PROGRESS in libmysqlclient or remove it. As we're moving to Connector/C, I think it might be ok to remove it, for MySQL compatibility. Or we can keep it for a while, for compatibility with older MariaDB clients. Either change does not break the ABI.
unsigned int protocol_version; unsigned int field_count; unsigned int server_status; diff --git a/sql-common/client.c b/sql-common/client.c index 609eef5..e108c4b 100644 --- a/sql-common/client.c +++ b/sql-common/client.c @@ -2672,15 +2673,26 @@ static int send_client_reply_packet(MCPVIO_EXT *mpvio, if (mysql->client_flag & CLIENT_PROTOCOL_41) { /* 4.1 server and 4.1 client has a 32 byte option flag */ - int4store(buff,mysql->client_flag); + bzero(buff+9, 32-9); + if (mysql->server_capabilities & MARIADB_CLIENT_EXTENDED_FLAGS) + { + DBUG_ASSERT(mysql->client_flag & CLIENT_LONG_PASSWORD);
1. add DBUG_ASSERT(mysql->client_flag & CLIENT_PROTOCOL_41) 2. define CLIENT_LONG_PASSWORD to be 0 and CLIENT_MYSQL to be 1. I found that CLIENT_LONG_PASSWORD used without CLIENT_PROTOCOL_41in mysqlbinlog. it will be wrong with CLIENT_LONG_PASSWORD defined as 0 #define CLIENT_CAPABILITIES (CLIENT_LONG_PASSWORD | CLIENT_LONG_FLAG | CLIENT_LOCAL_FILES)
Why wrong? Check the code - CLIENT_LONG_PASSWORD is *never* checked, there is no code that does if (flags & CLIENT_LONG_PASSWORD) ... no logic depends on it. That's why I'm saying that it's not used, and should be renamed to match its new meaning.
diff --git a/tests/mysql_client_test.c b/tests/mysql_client_test.c index 98f42ff..255ac25 100644 --- a/tests/mysql_client_test.c +++ b/tests/mysql_client_test.c @@ -16186,6 +16186,10 @@ static void test_change_user() sprintf(buff, "drop database if exists %s", db); rc= mysql_query(mysql, buff); myquery(rc); + sprintf(buff, "set local sql_mode=''"); why? Otherwise it can't make grant.
What do you mean? How does it work now? Regards, Sergei
On 24.11.15 12:27, Sergei Golubchik wrote:
Hi, Oleksandr!
On Nov 23, Oleksandr Byelkin wrote:
diff --git a/include/mysql.h b/include/mysql.h index f088ad6..d398734 100644 --- a/include/mysql.h +++ b/include/mysql.h @@ -270,7 +270,7 @@ typedef struct st_mysql unsigned long thread_id; /* Id for connection in server */ unsigned long packet_length; unsigned int port; - unsigned long client_flag,server_capabilities; + unsigned long long client_flag,server_capabilities; This is an incompatible change, it breaks the ABI. If we have to have this change in libmysqlclient, you can put the new flags in the extension area. But, really, perhaps you should switch to Connector/C? The problem is that we have to remove CLIENT_PROGRESS (there is no such flag in mysql) Right. So either we can keep CLIENT_PROGRESS in libmysqlclient or remove it. As we're moving to Connector/C, I think it might be ok to remove it, for MySQL compatibility. Or we can keep it for a while, for compatibility with older MariaDB clients.
unsigned int protocol_version; unsigned int field_count; unsigned int server_status; diff --git a/sql-common/client.c b/sql-common/client.c index 609eef5..e108c4b 100644 --- a/sql-common/client.c +++ b/sql-common/client.c @@ -2672,15 +2673,26 @@ static int send_client_reply_packet(MCPVIO_EXT *mpvio, if (mysql->client_flag & CLIENT_PROTOCOL_41) { /* 4.1 server and 4.1 client has a 32 byte option flag */ - int4store(buff,mysql->client_flag); + bzero(buff+9, 32-9); + if (mysql->server_capabilities & MARIADB_CLIENT_EXTENDED_FLAGS) + { + DBUG_ASSERT(mysql->client_flag & CLIENT_LONG_PASSWORD);
1. add DBUG_ASSERT(mysql->client_flag & CLIENT_PROTOCOL_41) 2. define CLIENT_LONG_PASSWORD to be 0 and CLIENT_MYSQL to be 1. I found that CLIENT_LONG_PASSWORD used without CLIENT_PROTOCOL_41in mysqlbinlog. it will be wrong with CLIENT_LONG_PASSWORD defined as 0 #define CLIENT_CAPABILITIES (CLIENT_LONG_PASSWORD | CLIENT_LONG_FLAG | CLIENT_LOCAL_FILES) Why wrong? Check the code - CLIENT_LONG_PASSWORD is *never* checked,
Either change does not break the ABI. OK, then I'll also remove client translation of PROGRESS you liked (as well as all client code). there is no code that does
if (flags & CLIENT_LONG_PASSWORD) ...
no logic depends on it. That's why I'm saying that it's not used, and should be renamed to match its new meaning. OK
diff --git a/tests/mysql_client_test.c b/tests/mysql_client_test.c index 98f42ff..255ac25 100644 --- a/tests/mysql_client_test.c +++ b/tests/mysql_client_test.c @@ -16186,6 +16186,10 @@ static void test_change_user() sprintf(buff, "drop database if exists %s", db); rc= mysql_query(mysql, buff); myquery(rc); + sprintf(buff, "set local sql_mode=''"); why? Otherwise it can't make grant. What do you mean? How does it work now? I have no idea (probably it saves by change other connection where other test set sql_mode=''). If run the same commands manually you will get an error:
bell@bell-ThinkPad:~/maria/git/server$ ./client/mysql --user=root --socket=/home/bell/maria/git/server/mysql-test/var/tmp/mysqld.1.sock Welcome to the MariaDB monitor. Commands end with ; or \g. Your MariaDB connection id is 2 Server version: 10.1.9-MariaDB-debug-log Source distribution Copyright (c) 2000, 2015, Oracle, MariaDB Corporation Ab and others. Type 'help;' or '\h' for help. Type '\c' to clear the current input statement. MariaDB [(none)]> create database mysqltest_user_test_database; Query OK, 1 row affected (0.00 sec) MariaDB [(none)]> grant select on mysqltest_user_test_database.* to mysqltest_pw@'%' identified by 'password'; Query OK, 0 rows affected (0.00 sec) MariaDB [(none)]> grant select on mysqltest_user_test_database.* to mysqltest_pw@'localhost' identified by 'password'; Query OK, 0 rows affected (0.00 sec) MariaDB [(none)]> grant select on mysqltest_user_test_database.* to mysqltest_no_pw@'%'; ERROR 1133 (28000): Can't find any matching row in the user table MariaDB [(none)]>
Hi, Oleksandr! On Nov 24, Oleksandr Byelkin wrote:
On Nov 23, Oleksandr Byelkin wrote:
diff --git a/include/mysql.h b/include/mysql.h index f088ad6..d398734 100644 --- a/include/mysql.h +++ b/include/mysql.h @@ -270,7 +270,7 @@ typedef struct st_mysql unsigned long thread_id; /* Id for connection in server */ unsigned long packet_length; unsigned int port; - unsigned long client_flag,server_capabilities; + unsigned long long client_flag,server_capabilities; This is an incompatible change, it breaks the ABI. If we have to have this change in libmysqlclient, you can put the new flags in the extension area. But, really, perhaps you should switch to Connector/C? The problem is that we have to remove CLIENT_PROGRESS (there is no such flag in mysql) Right. So either we can keep CLIENT_PROGRESS in libmysqlclient or remove it. As we're moving to Connector/C, I think it might be ok to remove it, for MySQL compatibility. Or we can keep it for a while, for compatibility with older MariaDB clients.
Either change does not break the ABI. OK, then I'll also remove client translation of PROGRESS you liked (as well as all client code).
Ok, but I think it'll be needed in Connector/C, when it connects to the old MariaDB server. Regards, Sergei
Hi, Serg! On 24.11.2015 20:38, Sergei Golubchik wrote: [skip]
Ok, but I think it'll be needed in Connector/C, when it connects to the old MariaDB server. I informed Georg and he found it reasonable.
While I am trying to fix git triggers for mail, here is the after-review patch diff --git a/client/mysql.cc b/client/mysql.cc index 3e08c87..de5ecb1 100644 --- a/client/mysql.cc +++ b/client/mysql.cc @@ -152,7 +152,7 @@ static ulong opt_max_allowed_packet, opt_net_buffer_length; static uint verbose=0,opt_silent=0,opt_mysql_port=0, opt_local_infile=0; static uint my_end_arg; static char * opt_mysql_unix_port=0; -static int connect_flag=CLIENT_INTERACTIVE; +static ulong connect_flag=CLIENT_INTERACTIVE; static my_bool opt_binary_mode= FALSE; static int interrupted_query= 0; static char *current_host,*current_db,*current_user=0,*opt_password=0, diff --git a/include/mysql_com.h b/include/mysql_com.h index aa6ab0f..57092dc 100644 --- a/include/mysql_com.h +++ b/include/mysql_com.h @@ -188,7 +188,8 @@ enum enum_server_command #define REFRESH_GENERIC (1ULL << 30) #define REFRESH_FAST (1ULL << 31) /* Intern flag */ -#define CLIENT_LONG_PASSWORD 1 /* new more secure passwords */ +#define CLIENT_LONG_PASSWORD 0 /* obsolete flag */ +#define CLIENT_MYSQL 1 /* mysql/old mariadb server/client */ #define CLIENT_FOUND_ROWS 2 /* Found instead of affected rows */ #define CLIENT_LONG_FLAG 4 /* Get all column flags */ #define CLIENT_CONNECT_WITH_DB 8 /* One can specify db on connect */ @@ -215,7 +216,8 @@ enum enum_server_command /* Don't close the connection for a connection with expired password. */ #define CLIENT_CAN_HANDLE_EXPIRED_PASSWORDS (1UL << 22) -#define CLIENT_PROGRESS (1UL << 29) /* Client support progress indicator */ +#define CLIENT_PROGRESS_OBSOLETE (1UL << 29) +#define CLIENT_PROGRESS 0 #define CLIENT_SSL_VERIFY_SERVER_CERT (1UL << 30) /* It used to be that if mysql_real_connect() failed, it would delete any @@ -228,6 +230,13 @@ enum enum_server_command */ #define CLIENT_REMEMBER_OPTIONS (1UL << 31) +/* MariaDB extended capability flags */ +#define MARIADB_CLIENT_FLAGS_MASK 0xffffffff00000000ULL +/* Client support progress indicator */ +#define MARIADB_CLIENT_PROGRESS (1ULL << 32) +/* Client support mariaDB extended flags */ +#define MARIADB_CLIENT_EXTENDED_FLAGS (1ULL << 63) + #ifdef HAVE_COMPRESS #define CAN_CLIENT_COMPRESS CLIENT_COMPRESS #else @@ -235,7 +244,7 @@ enum enum_server_command #endif /* Gather all possible capabilites (flags) supported by the server */ -#define CLIENT_ALL_FLAGS (CLIENT_LONG_PASSWORD | \ +#define CLIENT_ALL_FLAGS (CLIENT_MYSQL | \ CLIENT_FOUND_ROWS | \ CLIENT_LONG_FLAG | \ CLIENT_CONNECT_WITH_DB | \ @@ -256,7 +265,7 @@ enum enum_server_command CLIENT_PS_MULTI_RESULTS | \ CLIENT_SSL_VERIFY_SERVER_CERT | \ CLIENT_REMEMBER_OPTIONS | \ - CLIENT_PROGRESS | \ + MARIADB_CLIENT_PROGRESS | \ CLIENT_PLUGIN_AUTH | \ CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA | \ CLIENT_CONNECT_ATTRS) diff --git a/libmysql/client_settings.h b/libmysql/client_settings.h index b233614..2577870 100644 --- a/libmysql/client_settings.h +++ b/libmysql/client_settings.h @@ -27,7 +27,7 @@ extern char * mysql_unix_port; When adding capabilities here, consider if they should be also added to the server's version. */ -#define CLIENT_CAPABILITIES (CLIENT_LONG_PASSWORD | \ +#define CLIENT_CAPABILITIES (CLIENT_MYSQL | \ CLIENT_LONG_FLAG | \ CLIENT_TRANSACTIONS | \ CLIENT_PROTOCOL_41 | \ diff --git a/sql-common/client.c b/sql-common/client.c index 2a73eae..0ec0842 100644 --- a/sql-common/client.c +++ b/sql-common/client.c @@ -2486,7 +2486,7 @@ static int send_client_reply_packet(MCPVIO_EXT *mpvio, if (mysql->client_flag & CLIENT_PROTOCOL_41) { /* 4.1 server and 4.1 client has a 32 byte option flag */ - int4store(buff,mysql->client_flag); + int4store(buff, mysql->client_flag); int4store(buff+4, net->max_packet_size); buff[8]= (char) mysql->charset->number; bzero(buff+9, 32-9); @@ -2494,7 +2494,7 @@ static int send_client_reply_packet(MCPVIO_EXT *mpvio, } else { - int2store(buff, mysql->client_flag); + int2store(buff, (mysql->client_flag)); int3store(buff+2, net->max_packet_size); end= buff+5; } diff --git a/sql/client_settings.h b/sql/client_settings.h index d6a157f..cc90d6a 100644 --- a/sql/client_settings.h +++ b/sql/client_settings.h @@ -28,7 +28,7 @@ When adding capabilities here, consider if they should be also added to the libmysql version. */ -#define CLIENT_CAPABILITIES (CLIENT_LONG_PASSWORD | \ +#define CLIENT_CAPABILITIES (CLIENT_MYSQL | \ CLIENT_LONG_FLAG | \ CLIENT_TRANSACTIONS | \ CLIENT_PROTOCOL_41 | \ diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 1240d5d..d07e0a4 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -11245,7 +11245,8 @@ static bool send_server_handshake_packet(MPVIO_EXT *mpvio, *end++= protocol_version; - thd->client_capabilities= CLIENT_BASIC_FLAGS; + thd->client_capabilities= + (CLIENT_BASIC_FLAGS | MARIADB_CLIENT_EXTENDED_FLAGS); if (opt_using_transactions) thd->client_capabilities|= CLIENT_TRANSACTIONS; @@ -11313,7 +11314,8 @@ static bool send_server_handshake_packet(MPVIO_EXT *mpvio, int2store(end+5, thd->client_capabilities >> 16); end[7]= data_len; DBUG_EXECUTE_IF("poison_srv_handshake_scramble_len", end[7]= -100;); - bzero(end + 8, 10); + bzero(end + 8, 6); + int4store(end + 14, thd->client_capabilities >> 32); end+= 18; /* write scramble tail */ end= (char*) memcpy(end, data + SCRAMBLE_LENGTH_323, @@ -11729,18 +11731,38 @@ static ulong parse_client_handshake_packet(MPVIO_EXT *mpvio, */ DBUG_ASSERT(net->read_pos[pkt_len] == 0); - ulong client_capabilities= uint2korr(net->read_pos); + ulonglong client_capabilities= uint2korr(net->read_pos); + compile_time_assert(sizeof(client_capabilities) >= 8); if (client_capabilities & CLIENT_PROTOCOL_41) { - if (pkt_len < 4) + if (pkt_len < 32) return packet_error; client_capabilities|= ((ulong) uint2korr(net->read_pos+2)) << 16; + if (!(client_capabilities & CLIENT_MYSQL)) + { + // it is client with mariadb extensions + client_capabilities|= CLIENT_MYSQL; + ulonglong ext_client_capabilities= + (((ulonglong)uint4korr(net->read_pos + 28)) << 32); + if (ext_client_capabilities & MARIADB_CLIENT_EXTENDED_FLAGS) + client_capabilities|= ext_client_capabilities; + else + { + DBUG_PRINT("error", ("CLIENT_PROTOCOL_41: on, " + "CLIENT_LONG_PASSWORD/CLIENT_MYSQL off, " + "but MARIADB_CLIENT_EXTENDED_FLAGS is off. " + "flags: %llx ext flags %llx", + client_capabilities, ext_client_capabilities)); + return packet_error; + } + } } /* Disable those bits which are not supported by the client. */ + compile_time_assert(sizeof(thd->client_capabilities) >= 8); thd->client_capabilities&= client_capabilities; - DBUG_PRINT("info", ("client capabilities: %lu", thd->client_capabilities)); + DBUG_PRINT("info", ("client capabilities 1: %llu", thd->client_capabilities)); if (thd->client_capabilities & CLIENT_SSL) { unsigned long errptr __attribute__((unused)); @@ -11768,8 +11790,6 @@ static ulong parse_client_handshake_packet(MPVIO_EXT *mpvio, if (client_capabilities & CLIENT_PROTOCOL_41) { - if (pkt_len < 32) - return packet_error; thd->max_client_packet_length= uint4korr(net->read_pos+4); DBUG_PRINT("info", ("client_character_set: %d", (uint) net->read_pos[8])); if (thd_init_client_charset(thd, (uint) net->read_pos[8])) @@ -12546,7 +12566,7 @@ bool acl_authenticate(THD *thd, uint com_change_user_pkt_len) } DBUG_PRINT("info", - ("Capabilities: %lu packet_length: %ld Host: '%s' " + ("Capabilities: %llu packet_length: %ld Host: '%s' " "Login user: '%s' Priv_user: '%s' Using password: %s " "Access: %lu db: '%s'", thd->client_capabilities, thd->max_client_packet_length, diff --git a/sql/sql_class.cc b/sql/sql_class.cc index e95e32a..b496e4c 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -4330,7 +4330,7 @@ extern "C" void thd_progress_init(MYSQL_THD thd, uint max_stage) is a high level command (like ALTER TABLE) and we are not in a stored procedure */ - thd->progress.report= ((thd->client_capabilities & CLIENT_PROGRESS) && + thd->progress.report= ((thd->client_capabilities & MARIADB_CLIENT_PROGRESS) && thd->progress.report_to_client && !thd->in_sub_stmt); thd->progress.next_report_time= 0; diff --git a/sql/sql_class.h b/sql/sql_class.h index aee6d56..b13e5f6 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -1410,7 +1410,7 @@ class Sub_statement_state Discrete_intervals_list auto_inc_intervals_forced; ulonglong limit_found_rows; ha_rows cuted_fields, sent_row_count, examined_row_count; - ulong client_capabilities; + ulonglong client_capabilities; ulong query_plan_flags; uint in_sub_stmt; bool enable_slow_log; @@ -2043,7 +2043,7 @@ class THD :public Statement, /* Needed by MariaDB semi sync replication */ Trans_binlog_info *semisync_info; - ulong client_capabilities; /* What the client supports */ + ulonglong client_capabilities; /* What the client supports */ ulong max_client_packet_length; HASH handler_tables_hash; @@ -2085,7 +2085,7 @@ class THD :public Statement, bool report_to_client; /* true, if we will send progress report packets to a client - (client has requested them, see CLIENT_PROGRESS; report_to_client + (client has requested them, see MARIADB_CLIENT_PROGRESS; report_to_client is true; not in sub-statement) */ bool report; diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index bc26780..28e9b3b 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -637,7 +637,7 @@ void execute_init_command(THD *thd, LEX_STRING *init_command, mysql_rwlock_t *var_lock) { Vio* save_vio; - ulong save_client_capabilities; + ulonglong save_client_capabilities; mysql_rwlock_rdlock(var_lock); if (!init_command->length) diff --git a/tests/mysql_client_fw.c b/tests/mysql_client_fw.c index 990fdb1..f26de62 100644 --- a/tests/mysql_client_fw.c +++ b/tests/mysql_client_fw.c @@ -326,7 +326,8 @@ mysql_simple_prepare(MYSQL *mysql_arg, const char *query) @return pointer to initialized and connected MYSQL object */ -static MYSQL* client_connect(ulong flag, uint protocol, my_bool auto_reconnect) +static MYSQL* client_connect(ulonglong flag, uint protocol, + my_bool auto_reconnect) { MYSQL* mysql; int rc; diff --git a/tests/mysql_client_test.c b/tests/mysql_client_test.c index a1a52e8..31fca0f 100644 --- a/tests/mysql_client_test.c +++ b/tests/mysql_client_test.c @@ -15336,7 +15336,7 @@ static void test_mysql_insert_id() myheader("test_mysql_insert_id"); - rc= mysql_query(mysql, "drop table if exists t1"); + rc= mysql_query(mysql, "drop table if exists t1,t2"); myquery(rc); /* table without auto_increment column */ rc= mysql_query(mysql, "create table t1 (f1 int, f2 varchar(255), key(f1))"); @@ -18700,7 +18700,8 @@ static void test_progress_reporting() myheader("test_progress_reporting"); conn= client_connect(CLIENT_PROGRESS, MYSQL_PROTOCOL_TCP, 0); - DIE_UNLESS(conn->client_flag & CLIENT_PROGRESS); + if (!(conn->client_flag & CLIENT_PROGRESS)) + return; mysql_options(conn, MYSQL_PROGRESS_CALLBACK, (void*) report_progress); rc= mysql_query(conn, "set @save=@@global.progress_report_time");
On 25.11.2015 14:03, Oleksandr Byelkin wrote:
+++ b/sql-common/client.c @@ -2486,7 +2486,7 @@ static int send_client_reply_packet(MCPVIO_EXT *mpvio, if (mysql->client_flag & CLIENT_PROTOCOL_41) { /* 4.1 server and 4.1 client has a 32 byte option flag */ - int4store(buff,mysql->client_flag); + int4store(buff, mysql->client_flag); int4store(buff+4, net->max_packet_size); buff[8]= (char) mysql->charset->number; bzero(buff+9, 32-9); @@ -2494,7 +2494,7 @@ static int send_client_reply_packet(MCPVIO_EXT *mpvio, } else { - int2store(buff, mysql->client_flag); + int2store(buff, (mysql->client_flag)); int3store(buff+2, net->max_packet_size); end= buff+5; }
oops, sorry, I'll fix above
Hi,
Either "ulonglong" or "unsigned long long". But not "ulong long", please.
+1 for ulonglong
- unsigned long client_flag,server_capabilities; + unsigned long long client_flag,server_capabilities;
This is an incompatible change, it breaks the ABI. If we have to have this change in libmysqlclient, you can put the new flags in the extension area. But, really, perhaps you should switch to Connector/C?
That was also my concern - if we can move something to (which is not used yet, at least in Connector/C) to mysql->extensions I'm fine with it. If we need to break ABI, then we should fix other things too, like changing ulong to size_t.
looks good, but please test this. New client, old server. And old client, new server too.
If we want to exchange libmysql by C/C, I would really appreciate to add tests to Connector/C. In the 10.2-georg branch there is already a 10.2-features test in place. I'm currently moving unit tests from static to dynamic linking, so it can be tested with both libmysq and libmariadb. /Georg
_______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
participants (3)
-
G R -
Oleksandr Byelkin -
Sergei Golubchik