maria-developers team mailing list archive

[Oleksandr Byelkin <sanja@xxxxxxxxxxxxxxxx>]

Hi, Sergei!

On 23.11.15 16:01, Sergei Golubchik wrote:
> 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.
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