maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Aleksey!

I'm reviewing it separately from other commits in the branch,
because it looks to be functionally independent from, say,

  MDEV-16029 mysqldump: dump and restore historical data

and doesn't have to wait for the latter to be pushed first.

I think I was able to guess what the patch would look like, if you'd
apply it to vanilla 10.7 without MDEV-16029. But just to be sure,
please, let me look it over again after the cherry-pick.

Otherwise no comments code-wise. A couple of comments related to naming,
see below.

On Aug 15, Aleksey Midenkov wrote:
> revision-id: a19495728de (versioning-1.0.5-45-ga19495728de)
> parent(s): be4605ae086
> author: Aleksey Midenkov
> committer: Aleksey Midenkov
> timestamp: 2018-07-19 14:47:35 +0300
> message:
> 
> MDEV-16355 Add option for mysqldump to read data as of specific timestamp from system-versioned tables
> 
> mysqldump changes:
> 
> * --asof-timestamp option specifies historical point;

"asof" isn't a word. I'd suggest --as-of-timestamp or simply --as-of

> * with --dump-history it dumps all prior history, without it dumps only single point in time;
> * query forging protection for --asof-timestamp parameter.
> 
> diff --git a/client/mysqldump.c b/client/mysqldump.c
> index eee12555271..f426e8c58ef 100644
> --- a/client/mysqldump.c
> +++ b/client/mysqldump.c
> @@ -1099,6 +1102,13 @@ static int get_options(int *argc, char ***argv)
>              my_progname_short);
>      return(EX_USAGE);
>    }
> +  if (opt_asof_timestamp && strchr(opt_asof_timestamp, '\''))
> +  {
> +    fprintf(stderr,
> +            "%s: --asof-timestamp wrong argument!\n",
> +            my_progname_short);

May be, use the wording that is closer to the error one would get from
any other invalid character, like, if one would do --asof-timestamp='!@#$%'

That is, instead of "wrong argument", something like

 fprintf(stderr,
         "%s: Incorrect DATETIME value: '%s'\n",
         opt_asof_timestamp, my_progname_short);

Because

  MariaDB [test]> select timestamp'2010!@#$%';
  ERROR 1525 (HY000): Incorrect DATETIME value: '2010!@#$%'

> +    return(EX_USAGE);
> +  }
>    if (strcmp(default_charset, charset_info->csname) &&
>        !(charset_info= get_charset_by_csname(default_charset,
>                                              MY_CS_PRIMARY, MYF(MY_WME))))

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx