Horton

Thread

Commits

  1. Stabilize postgres_fdw regression tests.

  1. Locale dependency in new postgres_fdw test

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-07-21T17:36:10Z

    So 8bf58c0d9 immediately blew up in the buildfarm, with eg this
on jaguarundi:

***************
*** 130,136 ****
  ALTER SERVER loopback OPTIONS (SET dbname 'no such database');
  SELECT c3, c4 FROM ft1 ORDER BY c3, c1 LIMIT 1;  -- should fail
  ERROR:  could not connect to server "loopback"
! DETAIL:  FATAL:  database "no such database" does not exist
  DO $d$
      BEGIN
          EXECUTE $$ALTER SERVER loopback
--- 130,136 ----
  ALTER SERVER loopback OPTIONS (SET dbname 'no such database');
  SELECT c3, c4 FROM ft1 ORDER BY c3, c1 LIMIT 1;  -- should fail
  ERROR:  could not connect to server "loopback"
! DETAIL:  FATAL:  Datenbank ?no such database? existiert nicht
  DO $d$
      BEGIN
          EXECUTE $$ALTER SERVER loopback
***************

I had not realized (or forgot) that postgres_fdw allows the remote
end to run in whatever lc_messages locale is default for the remote
installation.  It's a bit surprising that none of the existing test
cases expose any remote-side messages directly, but evidently not.

We could stabilize this test result by forcing lc_messages = C in
the foreign server options.  However, that would lose regression
coverage of situations where the remote locale is different, which
doesn't seem like a terribly good thing.

Another option is to temporarily set VERBOSITY to "terse" so that
the DETAIL is suppressed from the test output.  But then we don't
really know why the connection failed, so that could mask issues
that the test case ought to find, too.

Maybe set lc_messages = C in the options only for the duration
of these new test cases?

			regards, tom lane

  2. Re: Locale dependency in new postgres_fdw test

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-07-21T18:34:32Z

    I wrote:
> I had not realized (or forgot) that postgres_fdw allows the remote
> end to run in whatever lc_messages locale is default for the remote
> installation.  It's a bit surprising that none of the existing test
> cases expose any remote-side messages directly, but evidently not.

> We could stabilize this test result by forcing lc_messages = C in
> the foreign server options.  However, that would lose regression
> coverage of situations where the remote locale is different, which
> doesn't seem like a terribly good thing.

It turns out that that way doesn't fix the problem, anyway, because
an lc_messages setting in the startup packet is applied too late
to change the output for the errors we're interested in.  There have
been past discussions about maybe fixing that, but it's certainly
not happening now, much less in back branches.

BTW, I found out while trying to do that that ALTER SERVER does not
accept "options '-c lc_messages=C'" anyway, which surprised me quite
a bit.  The reason turns out to be that libpq labels "options" as a
debug option which causes postgres_fdw to ignore it.  Should we think
about changing that?  Being able to set GUC variables for the remote
session seems useful.

> Another option is to temporarily set VERBOSITY to "terse" so that
> the DETAIL is suppressed from the test output.  But then we don't
> really know why the connection failed, so that could mask issues
> that the test case ought to find, too.

So we're stuck with that solution.  The disadvantage of not being
entirely sure why the connection failed could be solved if psql had
some way to report just the SQLSTATE of the last failure.  I think
there's been some discussions about saving error SQLSTATEs into a
special variable, as we do for LASTOID for instance; but it doesn't
look like that's actually been done yet.  We should revisit this
if that feature ever materializes.

			regards, tom lane

  3. Re: Locale dependency in new postgres_fdw test

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2017-07-21T19:35:28Z

    Tom Lane wrote:

> We could stabilize this test result by forcing lc_messages = C in
> the foreign server options.  However, that would lose regression
> coverage of situations where the remote locale is different, which
> doesn't seem like a terribly good thing.

I don't understand what is the benefit of having a different locale
setting if there's no way that the test would pass except with a very
specific locale.  In other words, what coverage are we really losing by
going this route?

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  4. Re: Locale dependency in new postgres_fdw test

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-07-21T22:50:17Z

    Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> Tom Lane wrote:
>> We could stabilize this test result by forcing lc_messages = C in
>> the foreign server options.  However, that would lose regression
>> coverage of situations where the remote locale is different, which
>> doesn't seem like a terribly good thing.

> I don't understand what is the benefit of having a different locale
> setting if there's no way that the test would pass except with a very
> specific locale.  In other words, what coverage are we really losing by
> going this route?

What the current situation proves (or at least gives evidence for)
is that postgres_fdw doesn't have hidden dependencies on the remote server
running with the same lc_messages that prevails locally.  As an example
--- admittedly a bit far-fetched, because this shouldn't ever get past
code review --- if postgres_fdw were checking for the presence of specific
strings in error messages from the remote, we could hope that the
buildfarm would catch that.  But if we force the remote lc_messages value
throughout the test, we lose that type of coverage.

			regards, tom lane