← Back to team overview

maria-developers team mailing list archive

Re: [Commits] 2606b87: MDEV-9117: Client Server capability negotiation for MariaDB specific functionality

 

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


Follow ups

References