Re: [Maria-developers] e3971006cca: Give a readable error in mtr if resolve_at_variable fails

2021-04-06 Thread Michael Widenius
Hi!

On Mon, Apr 5, 2021 at 5:27 PM Sergei Golubchik  wrote:
>
> Hi, Monty!
>
> On Apr 05, Michael Widenius wrote:
> > revision-id: e3971006cca (mariadb-10.5.2-547-ge3971006cca)
> > parent(s): ae009435e72
> > author: Michael Widenius 
> > committer: Michael Widenius 
> > timestamp: 2021-03-24 19:25:09 +0200
> > message:
> >
> > Give a readable error in mtr if resolve_at_variable fails
>
> This change is wrong. Clearly, the || '' part was intentional.
> Check the history, it was added in the commit
>
>   commit 897b51db434
>   Author: Sergei Golubchik 
>   Date:   Thu Sep 10 12:12:47 2020 +0200
>
>   make S3 tests to run when S3 is statically linked
>
>   * use the environment variable HA_S3_SO, not a literal ha_s3 in cnf 
> files
>   * make ConfigFactory to support empty option values
>   * update no_s3.result after MDEV-11412
>
> so, apparently, the idea was that you should be able to write
>
>   plugin-load-add=$HA_S3_SO
>
> and it will *not* fail when $HA_S3_SO is not defined, because S3 engine
> is statically linked in.

I did not do this for the HA_S3 engine.  I got under some conditions
that I cannot remember now, warnings when running mtr about accessing
not allocated
variables.  The patch helped me to find and fix those.

___
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


Re: [Maria-developers] e3971006cca: Give a readable error in mtr if resolve_at_variable fails

2021-04-05 Thread Sergei Golubchik
Hi, Monty!

On Apr 05, Michael Widenius wrote:
> revision-id: e3971006cca (mariadb-10.5.2-547-ge3971006cca)
> parent(s): ae009435e72
> author: Michael Widenius 
> committer: Michael Widenius 
> timestamp: 2021-03-24 19:25:09 +0200
> message:
> 
> Give a readable error in mtr if resolve_at_variable fails

This change is wrong. Clearly, the || '' part was intentional.
Check the history, it was added in the commit

  commit 897b51db434
  Author: Sergei Golubchik 
  Date:   Thu Sep 10 12:12:47 2020 +0200

  make S3 tests to run when S3 is statically linked

  * use the environment variable HA_S3_SO, not a literal ha_s3 in cnf files
  * make ConfigFactory to support empty option values
  * update no_s3.result after MDEV-11412

so, apparently, the idea was that you should be able to write

  plugin-load-add=$HA_S3_SO

and it will *not* fail when $HA_S3_SO is not defined, because S3 engine
is statically linked in.

> diff --git a/mysql-test/lib/My/ConfigFactory.pm 
> b/mysql-test/lib/My/ConfigFactory.pm
> index 3249a06256c..5a5b691998c 100644
> --- a/mysql-test/lib/My/ConfigFactory.pm
> +++ b/mysql-test/lib/My/ConfigFactory.pm
> @@ -347,8 +347,15 @@ sub resolve_at_variable {
>  or croak "There is no group named '$group_name' that ",
>"can be used to resolve '$option_name' for test '$self->{testname}'";
>  
> -my $value= $from_group->value($option_name) || '';
> -$res .= $before.$value;
> +my $value= $from_group->value($option_name);
> +if (!defined($value))
> +{
> +  ::mtr_verbose("group: $group_name  option_name: $option_name is 
> undefined");
> +}
> +else
> +{
> +  $res .= $before.$value;
> +}
>}
>$res .= $after;
>  

Regards,
Sergei
VP of MariaDB Server Engineering
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