On Mon, Sep 30, 2019 at 2:26 PM Fabien COELHO <coelho@cri.ensmp.fr> wrote:
> > I am leaning towards approach (b) unless you and or Alvaro feels (a)
> > is good for now or if you have some other idea.
>
> No other idea. I put back the tablespace creation which I just removed,
> with comments about why it is there.
>
> > If we want to go with option (b), I have small comment in your previous test:
> > +# tablespace for testing
> > +my $ts = $node->basedir . '/regress_pgbench_tap_1_ts_dir';
> > +mkdir $ts or die "cannot create directory $ts";
> > +my $ets = TestLib::perl2host($ts);
> > +# add needed escaping!
> > +$ets =~ s/'/''/;
> >
> > I am not sure if we really need this quote skipping stuff. Why can't
> > we write the test as below:
> >
> > # tablespace for testing
> > my $basedir = $node->basedir;
> > my $ts = "$basedir/regress_pgbench_tap_1_ts_dir";
> > mkdir $ts or die "cannot create directory $ts";
> > $ts = TestLib::perl2host($ts);
> > $node->safe_psql('postgres',
> > "CREATE TABLESPACE regress_pgbench_tap_1_ts LOCATION '$ets';"
>
> I think that this last command fails if the path contains a "'", so the
> '-escaping is necessary. I had to make changes in TAP tests before because
> it was not working when the path was a little bit strange, so now I'm
> careful.
>
Hmm, I don't know what kind of issues you have earlier faced, but
tablespace creation doesn't allow quotes. See the message "tablespace
location cannot contain single quotes" in CreateTableSpace. Also,
there are other places in tests like
src/bin/pg_checksums/t/002_actions.pl which uses the way I have
mentioned. I don't think there is any need for escaping single-quotes
here and I am not seeing the use of same. I don't want to introduce a
new pattern in tests which people can then tomorrow copy at other
places even though such code is not required. OTOH, if there is a
genuine need for the same, then I am fine.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com