Hi, Alexey!
Please, see few comments below.
On Dec 07, Alexey Botchkov wrote:
> revision-id: b610986c9c226e9b9cbe7c80ceaead1de0a55535
> (mariadb-10.2.11-26-gb610986)
> parent(s): 4d016e6ed2d32298977a66e125cbfcae39e23847
> committer: Alexey Botchkov
> timestamp: 2017-12-07 23:49:12 +0400
> message:
>
> MDEV-14593 human-readable XA RECOVER.
>
> diff --git a/sql/handler.cc b/sql/handler.cc
> index 7eed722..128721c 100644
> --- a/sql/handler.cc
> +++ b/sql/handler.cc
> @@ -1978,6 +1978,97 @@ int ha_recover(HASH *commit_list)
> }
>
> /**
> + return the XID as it appears in the SQL function's arguments.
> + So this string can be passed to XA START, XA PREPARE etc...
> +
> + @note
> + the 'buf' has to have space for at least SQL_XIDSIZE bytes.
> +*/
> +
> +
> +/*
> + 'a'..'z' 'A'..'Z', '0'..'9'
> + and '-' '_' ' ' symbols don't have to be
> + converted.
I'd say it's overcomplication, why not to convert everything to hex?
> +*/
> +
> +static const char xid_needs_conv[128]=
> +{
> + 1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,
> + 1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,
> + 0,1,1,1,1,1,1,1,1,1,1,1,1,0,1,1,
> + 0,0,0,0,0,0,0,0,0,0,1,1,1,1,1,1,
> + 1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
> + 0,0,0,0,0,0,0,0,0,0,0,1,1,1,1,0,
> + 1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
> + 0,0,0,0,0,0,0,0,0,0,0,1,1,1,1,1
> +};
> +
> +uint get_sql_xid(XID *xid, char *buf)
> +{
> + int tot_len= xid->gtrid_length + xid->bqual_length;
> + int i;
> + const char *orig_buf= buf;
> +
> + for (i=0; i<tot_len; i++)
> + {
> + uchar c= ((uchar *) xid->data)[i]
> + if (c >= 128 || xid_needs_conv[c])
> + break;
> + }
> +
> + if (i >= tot_len)
> + {
> + /* No need to convert characters to hexadecimals. */
> + *buf++= '\'';
> + memcpy(buf, xid->data, xid->gtrid_length);
> + buf+= xid->gtrid_length;
> + *buf++= '\'';
> + if (xid->bqual_length > 0 || xid->formatID != 1)
> + {
> + *buf++= ',';
> + *buf++= '\'';
> + memcpy(buf, xid->data+xid->gtrid_length, xid->bqual_length);
> + buf+= xid->bqual_length;
> + *buf++= '\'';
> + }
> + }
> + else
> + {
> + *buf++= 'X';
> + *buf++= '\'';
> + for (i= 0; i < xid->gtrid_length; i++)
> + {
> + *buf++=_dig_vec_lower[((uchar*) xid->data)[i] >> 4];
> + *buf++=_dig_vec_lower[((uchar*) xid->data)[i] & 0x0f];
> + }
> + *buf++= '\'';
> + if (xid->bqual_length > 0 || xid->formatID != 1)
> + {
> + *buf++= ',';
> + *buf++= 'X';
> + *buf++= '\'';
> + for (; i < tot_len; i++)
> + {
> + *buf++=_dig_vec_lower[((uchar*) xid->data)[i] >> 4];
> + *buf++=_dig_vec_lower[((uchar*) xid->data)[i] & 0x0f];
> + }
> + *buf++= '\'';
> + }
> + }
> +
> + if (xid->formatID != 1)
> + {
> + *buf++= ',';
> + buf+= my_longlong10_to_str_8bit(&my_charset_bin, buf,
> + MY_INT64_NUM_DECIMAL_DIGITS, -10, xid->formatID);
> + }
> +
> + return buf - orig_buf;
> +}
> +
> +
> +/**
> return the list of XID's to a client, the same way SHOW commands do.
>
> @note
> diff --git a/sql/handler.h b/sql/handler.h
> index 486ba56..91e3168 100644
> --- a/sql/handler.h
> +++ b/sql/handler.h
> @@ -628,6 +628,16 @@ struct xid_t {
> };
> typedef struct xid_t XID;
>
> +/*
> + The size of XID string representation in the form
> + 'gtrid', 'bqual', formatID
> + see xid_t::get_sql_string() for details.
> +*/
> +#define SQL_XIDSIZE (XIDDATASIZE * 2 + 8 + MY_INT64_NUM_DECIMAL_DIGITS)
> +/* The 'buf' has to have space for at least SQL_XIDSIZE bytes. */
> +uint get_sql_xid(XID *xid, char *buf);
Not sure you need this as an extern function in the header.
Why not to keep it static in handler.cc?
> /* for recover() handlerton call */
> #define MIN_XID_LIST_SIZE 128
> #define MAX_XID_LIST_SIZE (1024*128)
> diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy
> index 17e6904..d0fa642 100644
> --- a/sql/sql_yacc.yy
> +++ b/sql/sql_yacc.yy
> @@ -1705,6 +1705,7 @@ bool my_yyoverflow(short **a, YYSTYPE **b, ulong
> *yystacksize);
> %token WRITE_SYM /* SQL-2003-N */
> %token X509_SYM
> %token XA_SYM
> +%token XID_SYM
> %token XML_SYM
> %token XOR
> %token YEAR_MONTH_SYM
You forgot to add XID_SYM to the keyword_sp rule.
Also I don't see why we should have CONVERT XID syntax if the output is
incompatible. Perhaps it'd be better to have a different
syntax? FORMAT=SQL?
Regards,
Sergei
Chief Architect MariaDB
and secur...@mariadb.org
_______________________________________________
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
