maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Sanja!

On Nov 20, sanja@xxxxxxxxxxx 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