23 Nov
2015
23 Nov
'15
8:16 p.m.
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