Re: psql - add SHOW_ALL_RESULTS option

Michael Paquier writes: > On Fri, Apr 09, 2021 at 08:52:20AM +0200, Fabien COELHO wrote: >> There is not a single test of "ctrl-c" which would have caught this trivial >> and irritating regression. ISTM that a TAP test is doable. Should one be >> added? > If you can design something reliable, I

Re: psql - add SHOW_ALL_RESULTS option

On Sun, Apr 11, 2021 at 11:14:07AM -0400, Tom Lane wrote: > It's right: this is dead code because all paths through the if-nest > starting at line 1373 now leave results = NULL. Hence, this patch > has broken the autocommit logic; it's no longer possible to tell > whether we should do anything

Re: Replication slot stats misgivings

On Sat, Apr 10, 2021 at 9:53 PM Amit Kapila wrote: > > On Sat, Apr 10, 2021 at 7:24 AM Masahiko Sawada wrote: > > > > IIUC there are two problems in the case where the drop message is lost: > > > > 1. Writing beyond the end of replSlotStats. > > This can happen if after restarting the number of

Re: [PATCH] force_parallel_mode and GUC categories

Michael Paquier writes: > However, I'd like to think that we can do better than what's proposed > in the patch. There are a couple of things to consider here: > - Should the parameter be renamed to reflect that it should only be > used for testing purposes? -1 to that part, because it would

Parallel INSERT SELECT take 2

This is another try of [1]. BACKGROUND We want to realize parallel INSERT SELECT in the following steps: 1) INSERT + parallel SELECT 2) Parallel INSERT + parallel SELECT Below are example use cases. We don't expect high concurrency or an empty data

Re: [PATCH] force_parallel_mode and GUC categories

On Fri, Apr 09, 2021 at 07:39:28AM -0400, Bruce Momjian wrote: > On Thu, Apr 8, 2021 at 10:17:18PM -0500, Justin Pryzby wrote: >> This is the main motive behind the patch. >> >> Developer options aren't shown in postgresql.conf.sample, which it seems like >> sometimes people read through

Re: psql - add SHOW_ALL_RESULTS option

On Fri, Apr 09, 2021 at 08:52:20AM +0200, Fabien COELHO wrote: > There is not a single test of "ctrl-c" which would have caught this trivial > and irritating regression. ISTM that a TAP test is doable. Should one be > added? If you can design something reliable, I would welcome that. -- Michael

Re: Set access strategy for parallel vacuum workers

On Thu, Apr 8, 2021 at 12:37 PM Bharath Rupireddy wrote: > > On Thu, Apr 8, 2021 at 11:22 AM Amit Kapila wrote: > > Yeah, I will change that before commit unless there are more suggestions. > > I have no further comments on the patch > fix_access_strategy_workers_11.patch, it LGTM. > Thanks,

Re: Uninitialized scalar variable (UNINIT) (src/backend/statistics/extended_stats.c)

On Sun, Apr 11, 2021 at 07:42:20PM -0300, Ranier Vilela wrote: > Em dom., 11 de abr. de 2021 às 16:25, Justin Pryzby > escreveu: >> I think you're right. You can look in the commit history to find the >> relevant >> commit and copy the committer. In this case that's a4d75c8, for Tomas. >> I

Re: Table refer leak in logical replication

On Sat, Apr 10, 2021 at 10:39 AM Justin Pryzby wrote: > I added this as an Open Item. Thanks, Justin. -- Amit Langote EDB: http://www.enterprisedb.com

Re: Have I found an interval arithmetic bug?

On Sun, Apr 11, 2021 at 4:33 PM Zhihong Yu wrote: > > > On Sun, Apr 11, 2021 at 12:57 PM Zhihong Yu wrote: > >> >> >> On Thu, Apr 8, 2021 at 10:24 AM Bruce Momjian wrote: >> >>> On Mon, Apr 5, 2021 at 02:01:58PM -0400, Bruce Momjian wrote: >>> > On Mon, Apr 5, 2021 at 11:33:10AM -0500,

Re: Replication slot stats misgivings

Thanks for the comments. On Fri, Apr 9, 2021 at 4:13 PM Amit Kapila wrote: > > On Wed, Apr 7, 2021 at 2:51 PM vignesh C wrote: > > > > That is not required, I have modified it. > > Attached v4 patch has the fixes for the same. > > > > Few comments: > > 0001 > -- > 1. The first patch

Re: pgsql: Move tablespace path re-creation from the makefiles to pg_regres

On Fri, Apr 09, 2021 at 08:07:10PM -0700, Noah Misch wrote: > "pg_regress --outputdir" is not a great location for a file or directory > created by a user other than the user running pg_regress. If one does "make > check" and then "make installcheck" against a cluster running as a different >

Re: [PATCH] Identify LWLocks in tracepoints

On Mon, 22 Mar 2021 at 17:00, Peter Eisentraut < peter.eisentr...@enterprisedb.com> wrote: > On 20.03.21 01:29, Craig Ringer wrote: > > There is already support for that. See the documentation at the end > of > > this page: > > >

Re: [PATCH] force_parallel_mode and GUC categories

On Mon, Apr 12, 2021 at 10:31 AM Michael Paquier wrote: > > On Fri, Apr 09, 2021 at 07:39:28AM -0400, Bruce Momjian wrote: > > On Thu, Apr 8, 2021 at 10:17:18PM -0500, Justin Pryzby wrote: > >> This is the main motive behind the patch. > >> > >> Developer options aren't shown in

RE: Truncate in synchronous logical replication failed

Hi On Saturday, April 10, 2021 11:52 PM Japin Li wrote: > On Thu, 08 Apr 2021 at 19:20, Japin Li wrote: > > On Wed, 07 Apr 2021 at 16:34, tanghy.f...@fujitsu.com > wrote: > >> On Wednesday, April 7, 2021 5:28 PM Amit Kapila > >> wrote > >> > >>>Can you please check if the behavior is the

Re: Simplify backend terminate and wait logic in postgres_fdw test

On Fri, Apr 09, 2021 at 04:53:01PM +0530, Bharath Rupireddy wrote: > I feel that we can provide a high timeout value (It can be 1hr on the > similar lines of using pg_sleep(3600) for crash tests in > 013_crash_restart.pl with the assumption that the backend running that > command will get killed

Re: Add header support to text format and matching feature

> > >> This now reads "Represents whether the header must be absent, present or > >> match.”. > > Since match shouldn't be preceded with be, I think we can say: > > Represents whether the header must match, be absent or be present. Thanks, here’s a v10 version of the patch that fixes this.

Re: PL/R regression on windows, but not linux with master.

On 4/10/21 8:56 PM, Tomas Vondra wrote: > On 4/11/21 2:38 AM, Dave Cramer wrote: >> >> >> >> On Sat, 10 Apr 2021 at 20:34, Tom Lane > > wrote: >> >> Dave Cramer mailto:davecra...@gmail.com>> writes: >> > On Sat, 10 Apr 2021 at 20:24, Tom Lane >

Re: PL/R regression on windows, but not linux with master.

Andrew Dunstan writes: >> Well, plr.h does this: >> >> #define WARNING 19 >> #define ERROR20 > The coding pattern in plr.h looks quite breakable. Instead of hard > coding values like this they should save the value from the postgres > headers in another variable

Re: PL/R regression on windows, but not linux with master.

On 4/11/21 10:13 AM, Tom Lane wrote: Andrew Dunstan writes: Well, plr.h does this: #define WARNING 19 #define ERROR 20 The coding pattern in plr.h looks quite breakable. Meh -- that code has gone 18+ years before breaking. Indeed. elog.h already provides a "PGERROR"

Re: psql - add SHOW_ALL_RESULTS option

Coverity has pointed out another problem with this patch: /srv/coverity/git/pgsql-git/postgresql/src/bin/psql/common.c: 1425 in SendQuery() 1419/* 1420 * Do nothing if they are messing with savepoints themselves: 1421

Re: Add header support to text format and matching feature

On Sun, Apr 11, 2021 at 4:01 AM Rémi Lapeyre wrote: > > > > Hi, > > >> sure it matches what is expected and exit immediatly if it does not. > > > > Typo: immediately > > > > +CREATE FOREIGN TABLE header_dont_match (a int, foo text) SERVER > file_server > > > > nit: since header is singular, you

Re: Avoid unnecessary table open/close for TRUNCATE foo, foo, foo; kind of commands

On 2021/04/10 11:32, Bharath Rupireddy wrote: On Fri, Apr 9, 2021 at 9:23 PM Fujii Masao wrote: On 2021/04/10 0:39, Amul Sul wrote: On Fri, Apr 9, 2021 at 8:51 PM Bharath Rupireddy wrote: Hi, While checking the ExecuteTruncate code for the FOREIGN TRUNCATE feature, I saw that we filter

Re: proposal: possibility to read dumped table's name from file

Hi út 16. 2. 2021 v 20:32 odesílatel Pavel Stehule napsal: > > Hi > > rebase > fresh rebase Regards Pavel > Regards > > Pavel > diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml index 529b167c96..24bfe07ee9 100644 --- a/doc/src/sgml/ref/pg_dump.sgml +++

Re: Add header support to text format and matching feature

> > Hi, > >> sure it matches what is expected and exit immediatly if it does not. > > Typo: immediately > > +CREATE FOREIGN TABLE header_dont_match (a int, foo text) SERVER file_server > > nit: since header is singular, you can name the table header_doesnt_match > > + from the one

Re: TRUNCATE on foreign table

On Sun, Apr 11, 2021 at 9:47 AM Justin Pryzby wrote: > Find attached language fixes. Thanks for the patches. > I'm also proposing to convert an if/else to an switch(), since I don't like > "if/else if" without an "else", and since the compiler can warn about missing > enum values in switch

Re: PL/R regression on windows, but not linux with master.

I wrote: > Joe Conway writes: >> Would an equivalent "PGWARNING" be something we are open to adding and >> back-patching? > It's not real obvious how pl/r could solve this in a reliable way > otherwise, so adding that would be OK with me, but I wonder whether > back-patching is going to help

Re: PL/R regression on windows, but not linux with master.

On Sun, 11 Apr 2021 at 12:43, Tom Lane wrote: > I wrote: > > Joe Conway writes: > >> Would an equivalent "PGWARNING" be something we are open to adding and > >> back-patching? > > > It's not real obvious how pl/r could solve this in a reliable way > > otherwise, so adding that would be OK with

Re: PANIC: wrong buffer passed to visibilitymap_clear

I wrote: > (It does look like RelationGetBufferForTuple > knows about updating vmbuffer, but there's one code path through the > if-nest at 3850ff that doesn't call that.) Although ... isn't RelationGetBufferForTuple dropping the ball on this point too, in the code path at the end where it has to

Re: PL/R regression on windows, but not linux with master.

Joe Conway writes: > On 4/11/21 10:13 AM, Tom Lane wrote: >> Indeed. elog.h already provides a "PGERROR" macro to use for restoring >> the value of ERROR. We have not heard of a need to do anything special >> for WARNING though --- maybe that's R-specific? > R also defines WARNING in its

Re: PANIC: wrong buffer passed to visibilitymap_clear

Peter Geoghegan writes: > On Sat, Apr 10, 2021 at 10:04 PM Tom Lane wrote: >> Just eyeing the evidence on hand, I'm wondering if something has decided >> it can start setting the page-all-visible bit without adequate lock, >> perhaps only in system catalogs. heap_update is clearly assuming that

Re: PANIC: wrong buffer passed to visibilitymap_clear

On Sun, Apr 11, 2021 at 8:57 AM Tom Lane wrote: > > Does this patch seem to fix the problem? > > Hmm ... that looks pretty suspicious, I agree, but why wouldn't an > exclusive buffer lock be enough to prevent concurrency with heap_update? I don't have any reason to believe that using a

Re: psql - add SHOW_ALL_RESULTS option

... btw, Coverity also doesn't like this fragment of the patch: /srv/coverity/git/pgsql-git/postgresql/src/bin/psql/common.c: 1084 in ShowNoticeMessage() 1078 static void 1079 ShowNoticeMessage(t_notice_messages *notes) 1080 { 1081PQExpBufferData *current = notes->in_flip

Re: PANIC: wrong buffer passed to visibilitymap_clear

On Sun, Apr 11, 2021 at 9:10 AM Peter Geoghegan wrote: > I don't have any reason to believe that using a super-exclusive lock > during heap page vacuuming is necessary. My guess is that returning to > doing it that way might make the buildfarm green again. That would at > least confirm my

Re: PL/R regression on windows, but not linux with master.

On 4/11/21 12:51 PM, Dave Cramer wrote: On Sun, 11 Apr 2021 at 12:43, Tom Lane > wrote: I wrote: > Joe Conway mailto:m...@joeconway.com>> writes: >> Would an equivalent "PGWARNING" be something we are open to adding and >> back-patching? >

Possible SSI bug in heap_update

While re-reading heap_update() in connection with that PANIC we're chasing, my attention was drawn to this comment: /* * Note: beyond this point, use oldtup not otid to refer to old tuple. * otid may very well point at newtup->t_self, which we will overwrite * with the new

Re: PANIC: wrong buffer passed to visibilitymap_clear

Peter Geoghegan writes: > This isn't just any super-exclusive lock, either -- we were calling > ConditionalLockBufferForCleanup() at this point. > I now think that there is a good chance that we are seeing these > symptoms because the "conditional-ness" went away -- we accidentally > relied on

Re: PANIC: wrong buffer passed to visibilitymap_clear

I wrote: > I'm now inclined to think that we should toss every single line of that > code, take RelationGetBufferForTuple out of the equation, and have just > *one* place that rechecks for PageAllVisible having just become set. > It's a rare enough case that optimizing it is completely not worth

Re: PANIC: wrong buffer passed to visibilitymap_clear

On Sat, Apr 10, 2021 at 10:04 PM Tom Lane wrote: > Just eyeing the evidence on hand, I'm wondering if something has decided > it can start setting the page-all-visible bit without adequate lock, > perhaps only in system catalogs. heap_update is clearly assuming that > that flag won't change

Re: multi-install PostgresNode fails with older postgres versions

On 4/7/21 5:06 PM, Alvaro Herrera wrote: > On 2021-Apr-07, Andrew Dunstan wrote: > >> Oh, you want to roll them all up into one file? That could work. It's a >> bit frowned on by perl purists, but I've done similar (see PGBuild/SCM.pm). > Ah! Yeah, pretty much exactly like that, including the

Re: PL/R regression on windows, but not linux with master.

Joe Conway writes: > On 4/11/21 12:51 PM, Dave Cramer wrote: >> On Sun, 11 Apr 2021 at 12:43, Tom Lane > > wrote: >>> Concretely, maybe like the attached? >> +1 from me. >> I especially like the changes to the comments as it's more apparent what >> they >> should be

Re: PANIC: wrong buffer passed to visibilitymap_clear

On Sun, Apr 11, 2021 at 10:55 AM Tom Lane wrote: > Alternatively, we could do what you suggested and redefine things > so that one is only allowed to set the all-visible bit while holding > superexclusive lock; which again would allow an enormous simplification > in heap_update and cohorts.

Re: PANIC: wrong buffer passed to visibilitymap_clear

Peter Geoghegan writes: > On Sun, Apr 11, 2021 at 10:55 AM Tom Lane wrote: >> Either way, it's hard to argue that >> heap_update hasn't crossed the complexity threshold where it's >> impossible to maintain safely. We need to simplify it. > It is way too complicated. I don't think that I quite

Re: PANIC: wrong buffer passed to visibilitymap_clear

On Sun, Apr 11, 2021 at 11:16 AM Tom Lane wrote: > It wasn't very clear, because I hadn't thought it through very much; > but what I'm imagining is that we discard most of the thrashing around > all-visible rechecks and have just one such test somewhere very late > in heap_update, after we've

Re: MultiXact\SLRU buffers configuration

> 8 апр. 2021 г., в 15:22, Thomas Munro написал(а): > > On Thu, Apr 8, 2021 at 7:24 PM Andrey Borodin wrote: >> I agree that this version of eviction seems much more effective and less >> intrusive than RR. And it's still LRU, which is important for subsystem that >> is called SLRU. >>

Uninitialized scalar variable (UNINIT) (src/backend/statistics/extended_stats.c)

Hi, Per Coverity. It seems to me that some recent commit has failed to properly initialize a structure, in extended_stats.c, when is passed to heap_copytuple. regards, Ranier Vilela fix_uninitialized_var_extend_stats.patch Description: Binary data

Re: Possible SSI bug in heap_update

On Mon, Apr 12, 2021 at 4:54 AM Tom Lane wrote: > While re-reading heap_update() in connection with that PANIC we're > chasing, my attention was drawn to this comment: > > /* > * Note: beyond this point, use oldtup not otid to refer to old tuple. > * otid may very well point at

Re: Calendar support in localization

On Wed, Mar 17, 2021 at 8:20 AM Thomas Munro wrote: > *I mean, we can discuss the different "timelines" like UT, UTC, TAI > etc, but that's getting into the weeds, the usual timeline for > computer software outside specialist scientific purposes is UTC > without leap seconds. (Erm, rereading

Re: Uninitialized scalar variable (UNINIT) (src/backend/statistics/extended_stats.c)

Hi Justin, sorry for the delay. Nothing against it, but I looked for similar codes and this is the usual way to initialize HeapTupleData. Perhaps InvalidOid makes a difference. regards, Ranier Vilela Em dom., 11 de abr. de 2021 às 16:25, Justin Pryzby escreveu: > On Sun, Apr 11, 2021 at

Re: Have I found an interval arithmetic bug?

On Sun, Apr 11, 2021 at 12:57 PM Zhihong Yu wrote: > > > On Thu, Apr 8, 2021 at 10:24 AM Bruce Momjian wrote: > >> On Mon, Apr 5, 2021 at 02:01:58PM -0400, Bruce Momjian wrote: >> > On Mon, Apr 5, 2021 at 11:33:10AM -0500, Justin Pryzby wrote: >> > Well, bug or not, we are not going to change

Re: Uninitialized scalar variable (UNINIT) (src/backend/statistics/extended_stats.c)

On Sun, Apr 11, 2021 at 03:38:10PM -0300, Ranier Vilela wrote: > Per Coverity. > > It seems to me that some recent commit has failed to properly initialize a > structure, in extended_stats.c, when is passed to heap_copytuple. I think you're right. You can look in the commit history to find the

Re: Have I found an interval arithmetic bug?

On Thu, Apr 8, 2021 at 10:24 AM Bruce Momjian wrote: > On Mon, Apr 5, 2021 at 02:01:58PM -0400, Bruce Momjian wrote: > > On Mon, Apr 5, 2021 at 11:33:10AM -0500, Justin Pryzby wrote: > > Well, bug or not, we are not going to change back branches for this, and > > if you want a larger

Re: test runner (was Re: SQL-standard function body)

> > > This is nice. Are there any parallelism capabilities? > > Yes. It defaults to number-of-cores processes, but obviously can also be > specified explicitly. One very nice part about it is that it'd work > largely the same on windows (which has practically unusable testing > right now). It

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

On Sun, Apr 11, 2021 at 05:20:35PM -0400, Alvaro Herrera wrote: > On 2021-Mar-31, Tom Lane wrote: > > > diff -U3 > > /home/buildfarm/trilobite/buildroot/HEAD/pgsql.build/src/test/isolation/expected/detach-partition-concurrently-4.out > > > >

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

On 2021-Mar-31, Tom Lane wrote: > diff -U3 > /home/buildfarm/trilobite/buildroot/HEAD/pgsql.build/src/test/isolation/expected/detach-partition-concurrently-4.out > > /home/buildfarm/trilobite/buildroot/HEAD/pgsql.build/src/test/isolation/output_iso/results/detach-partition-concurrently-4.out >