Skip to content

P
Postgres FD Implementation
Switch branch/tag
  1. 04 Jul, 2019 4 commits
  3. 03 Jul, 2019 8 commits
  5. 02 Jul, 2019 8 commits
  7. 01 Jul, 2019 11 commits
  9. 30 Jun, 2019 7 commits
    • Andrew Gierth's avatar
      Repair logic for reordering grouping sets optimization. · da53be23
      Andrew Gierth authored 
      The logic in reorder_grouping_sets to order grouping set elements to
match a pre-specified sort ordering was defective, resulting in
unnecessary sort nodes (though the query output would still be
correct). Repair, simplifying the code a little, and add a test.

Per report from Richard Guo, though I didn't use their patch. Original
bug seems to have been my fault.

Backpatch back to 9.5 where grouping sets were introduced.

Discussion: https://postgr.es/m/CAN_9JTzyjGcUjiBHxLsgqfk7PkdLGXiM=pwM+=ph2LsWw0WO1A@mail.gmail.com
      da53be23
    • Tom Lane's avatar
      Exclude new src/test/modules/unsafe_tests directory from MSVC build. · c000a47a
      Tom Lane authored 
      There's nothing to build here, and that was confusing AddContrib().
Per buildfarm.
      c000a47a
    • Tom Lane's avatar
      Blind attempt to fix SSPI-auth case in 010_dump_connstr.pl. · 681cca86
      Tom Lane authored 
      Up to now, pg_regress --config-auth had a hard-wired assumption
that the target cluster uses the default bootstrap superuser name.
pg_dump's 010_dump_connstr.pl TAP test uses non-default superuser
names, and was klugily getting around the restriction by listing
the desired superuser name as a role to "create".  This is pretty
confusing (or at least, it confused me).  Let's make it clearer by
allowing --config-auth mode to be told the bootstrap superuser name.
Repurpose the existing --user switch for that, since it has no
other function in --config-auth mode.

Per buildfarm.  I don't have an environment at hand in which I can
test this fix, but the buildfarm should soon show if it works.

Discussion: https://postgr.es/m/3142.1561840611@sss.pgh.pa.us
      681cca86
    • Tom Lane's avatar
      Move rolenames test out of the core regression tests. · c91504b9
      Tom Lane authored 
      This test script is unsafe to run in "make installcheck" mode for
(at least) two reasons: it creates and destroys some role names
that don't follow the "regress_xxx" naming convention, and it
sets and then resets the application_name GUC attached to every
existing role.  While we've not had complaints, these surely are
not good things to do within a production installation, and
regress.sgml pretty clearly implies that we won't do them.

Rather than lose test coverage altogether, let's just move this
script somewhere where it will get run by "make check" but not
"make installcheck".  src/test/modules/ already has that property.

Since it seems likely that we'll want other regression tests in
future that also exceed the constraints of "make installcheck",
create a generically-named src/test/modules/unsafe_tests/
directory to hold them.

Discussion: https://postgr.es/m/16638.1468620817@sss.pgh.pa.us
      c91504b9
    • Peter Eisentraut's avatar
      Fix breakage introduced in pg_lsn_in() · 2e810508
      Peter Eisentraut authored 
      Using PG_RETURN_LSN() from non-fmgr pg_lsn_in_internal() happened to
work on some platforms, but should just be a plain "return".
      2e810508
    • Peter Eisentraut's avatar
      Don't call data type input functions in GUC check hooks · 21f428eb
      Peter Eisentraut authored 
      Instead of calling pg_lsn_in() in check_recovery_target_lsn and
timestamptz_in() in check_recovery_target_time, reorganize the
respective code so that we don't raise any errors in the check hooks.
The previous code tried to use PG_TRY/PG_CATCH to handle errors in a
way that is not safe, so now the code contains no ereport() calls and
can operate safely within the GUC error handling system.

Moreover, since the interpretation of the recovery_target_time string
may depend on the time zone, we cannot do the final processing of that
string until all the GUC processing is done.  Instead,
check_recovery_target_time() now does some parsing for syntax
checking, but the actual conversion to a timestamptz value is done
later in the recovery code that uses it.
Reported-by: default avatarAndres Freund <andres@anarazel.de>
Reviewed-by: default avatarMichael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/20190611061115.njjwkagvxp4qujhp%40alap3.anarazel.de
      21f428eb
    • Peter Eisentraut's avatar
      Remove explicit error handling for obsolete date/time values · 666cbae1
      Peter Eisentraut authored 
      The date/time values 'current', 'invalid', and 'undefined' were
removed a long time ago, but the code still contains explicit error
handling for the transition.  To simplify the code and avoid having to
handle these values everywhere, just remove the recognition of these
tokens altogether now.
Reviewed-by: default avatarMichael Paquier <michael@paquier.xyz>
      666cbae1
  11. 29 Jun, 2019 2 commits
    • Tom Lane's avatar
      Add an enforcement mechanism for global object names in regression tests. · 54100f5c
      Tom Lane authored 
      In commit 18555b13 we tentatively established a rule that regression
tests should use names containing "regression" for databases, and names
starting with "regress_" for all other globally-visible object names, so
as to circumscribe the side-effects that "make installcheck" could have
on an existing installation.

This commit adds a simple enforcement mechanism for that rule: if the code
is compiled with ENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS defined, it
will emit a warning (not an error) whenever a database, role, tablespace,
subscription, or replication origin name is created that doesn't obey the
rule.  Running one or more buildfarm members with that symbol defined
should be enough to catch new violations, at least in the regular
regression tests.  Most TAP tests wouldn't notice such warnings, but
that's actually fine because TAP tests don't execute against an existing
server anyway.

Since it's already the case that running src/test/modules/ tests in
installcheck mode is deprecated, we can use that as a home for tests
that seem unsafe to run against an existing server, such as tests that
might have side-effects on existing roles.  Document that (though this
commit doesn't in itself make it any less safe than before).

Update regress.sgml to define these restrictions more clearly, and
to clean up assorted lack-of-up-to-date-ness in its descriptions of
the available regression tests.

Discussion: https://postgr.es/m/16638.1468620817@sss.pgh.pa.us
      54100f5c
    • Tom Lane's avatar
      Fix regression tests to use only global names beginning with "regress_". · ca129e58
      Tom Lane authored 
      In commit 18555b13 we tentatively established a rule that regression
tests should use names containing "regression" for databases, and names
starting with "regress_" for all other globally-visible object names, so
as to circumscribe the side-effects that "make installcheck" could have on
an existing installation.  However, no enforcement mechanism was created,
so it's unsurprising that some new violations have crept in since then.

In fact, a whole new *category* of violations has crept in, to wit we now
also have globally-visible subscription and replication origin names, and
"make installcheck" could very easily clobber user-created objects of
those types.  So it's past time to do something about this.

This commit sanitizes the tests enough that they will pass (i.e. not
generate any visible warnings) with the enforcement mechanism I'll add
in the next commit.  There are some TAP tests that still trigger the
warnings, but the warnings do not cause test failure.  Since these tests
do not actually run against a pre-existing installation, there's no need
to worry whether they could conflict with user-created objects.

The problem with rolenames.sql testing special role names like "user"
is still there, and is dealt with only very cosmetically in this patch
(by hiding the warnings :-().  What we actually need to do to be safe is
to take that test script out of "make installcheck" altogether, but that
seems like material for a separate patch.

Discussion: https://postgr.es/m/16638.1468620817@sss.pgh.pa.us
      ca129e58