Re: First draft of PG 17 release notes

2024-05-09 Thread Álvaro Herrera
this isn't too confusing. The standard's text on this, in section "4.13 Columns, fields, and attributes", is Every column has a nullability characteristic that indicates whether the value from that column can be the null value. A nullability characteristic is either kno

Re: Does a cancelled REINDEX CONCURRENTLY need to be messy?

2023-07-04 Thread Álvaro Herrera
m > could make things easier for a bgworker implementation, perhaps? TBH I don't understand what you are thinking about. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "I can see support will not be a problem. 10 out of 10."(Simon Wittber)

Re: Does a cancelled REINDEX CONCURRENTLY need to be messy?

2023-07-03 Thread Álvaro Herrera
On 2023-Jul-01, Thom Brown wrote: > On Thu, 29 Jun 2023, 14:45 Álvaro Herrera, wrote: > > > ALTER TABLE DETACH CONCURRENTLY had to deal with this also, and it did it > > by having a COMPLETE option you can run later in case things got stuck the > > first time aroun

Re: Does a cancelled REINDEX CONCURRENTLY need to be messy?

2023-06-29 Thread Álvaro Herrera
ALTER TABLE DETACH CONCURRENTLY had to deal with this also, and it did it by having a COMPLETE option you can run later in case things got stuck the first time around. I suppose we could do something similar, where the server automatically does the needful, whatever that is.

Re: Small TAP improvements

2022-11-06 Thread Álvaro Herrera
rray context it gives you a map, so you should be able > to say things like: > > my %node_config = $node->config_data; Hi, it looks to me like these were forgotten? -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/

Re: support for MERGE

2022-08-09 Thread Álvaro Herrera
On 2022-Aug-01, Álvaro Herrera wrote: > > > If MERGE attempts an INSERT > > > and a unique index is present and a duplicate row is concurrently > > > +inserted, then a uniqueness violation error is raised; > > > +MERGE does not att

Re: support for MERGE

2022-08-01 Thread Álvaro Herrera
On 2022-Aug-01, Justin Pryzby wrote: > On Tue, Jun 14, 2022 at 11:27:06AM +0200, Álvaro Herrera wrote: > > @@ -448,9 +448,9 @@ COMMIT; > > and execute the first one that succeeds. > > If MERGE attempts an INSERT > > and a unique index is present and a du

Re: support for MERGE

2022-06-15 Thread Álvaro Herrera
of specific commands, which is where I believe it belongs. Thanks! -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ Thou shalt study thy libraries and strive not to reinvent them without cause, that thy code may be short and readable and thy days pleasant and productive

Re: Small TAP improvements

2022-06-14 Thread Álvaro Herrera
The comment atop config_data still mentions $option, but after the patch that's no longer a name used in the function. (I have to admit that using @_ in the body of the function was a little bit confusing to me at first. Did you do that in order to allow multiple options to be passed?) Also:

Re: support for MERGE

2022-06-14 Thread Álvaro Herrera
On Wed, Jun 1, 2022, at 5:28 PM, Alvaro Herrera wrote: > Re-reading the modified paragraph, I propose "see X for a thorough > explanation on the behavior of MERGE under concurrency". However, in > the proposed patch the link goes to Chapter 13 "Concurrency Control", > and the explanation that we

Re: Using PQexecQuery in pipeline mode produces unexpected Close messages

2022-06-13 Thread Álvaro Herrera
On Fri, Jun 10, 2022, at 8:25 AM, Kyotaro Horiguchi wrote: > > The current implement of PQsendQueryInternal looks like the result of > a misunderstanding of the doc. In the regression tests, that path is > excercised only for an error case, where no CloseComplete comes. > > The attached adds a

Re: support for MERGE

2022-03-09 Thread Álvaro Herrera
logic as UPDATE? > > I think explanation should be given. Actually, the routine in question is only handling insert, not UPDATE, so MERGE is not relevant to the function. I have removed the comment. This was probably a leftover from work prior to 86dc90056dfd; that commit made it

Re: support for MERGE

2022-03-07 Thread Álvaro Herrera
On Mon, Mar 7, 2022, at 4:59 PM, Álvaro Herrera wrote: > I attach v13 here. This version includes a 0002 that's does the split of > nodeModifyTable.c routines, then 0003 implements MERGE on top of that. 0001 > is as before. In 0002, I've opted to have two separate stru

Re: \dP and \dX use ::regclass without "pg_catalog."

2022-01-08 Thread Álvaro Herrera
n automated check somewhere (next to stuff like perlcritic, perhaps). I don't know what, though. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "Cuando no hay humildad las personas se degradan" (A. Christie)

Re: support for MERGE

2021-11-16 Thread Álvaro Herrera
In any case, I have a feeling that the planner would catch any > problematic cases if there're any while converting MergeAction > expressions into the individual child table layouts. Yeah, AFAICS it worked fine for the case I tried. Maybe there are more elaborate ones that I didn't think

Re: support for MERGE

2021-11-15 Thread Álvaro Herrera
ready the case for trigger-redirected tuples in legacy-style partitioning, so I'm not too worried about that. -- Álvaro Herrera Valdivia, Chile — https://www.EnterpriseDB.com/

Re: support for MERGE

2021-11-13 Thread Álvaro Herrera
serious problem actually. I think it represents a gap in testing. Thanks for reporting. -- Álvaro Herrera 39°49'30"S 73°17'W — https://www.EnterpriseDB.com/

Re: Autovacuum on partitioned table (autoanalyze)

2021-08-28 Thread Álvaro Herrera
| 0 I don't disagree, but it's not easy to implement this at present. I think almost all counters should be nulls for partitioned tables. For some of them one could make a case that it'd be more convenient to propagate numbers up from partitions. -- Álvaro Herrera Valdivia, Chile — https://www.EnterpriseDB.com/

Re: \dP and \dX use ::regclass without "pg_catalog."

2021-08-28 Thread Álvaro Herrera
On 2021-Aug-27, Justin Pryzby wrote: > I noticed that for \dP+ since 1c5d9270e, regclass is written without > "pg_catalog." (Alvaro and I failed to notice it in 421a2c483, too). Oops, will fix shortly. -- Álvaro Herrera 39°49'30"S 73°17'W — https://www.EnterpriseDB.com/

Re: Autovacuum on partitioned table (autoanalyze)

2021-08-16 Thread Álvaro Herrera
On 2021-Aug-16, Álvaro Herrera wrote: > Here's the reversal patch for the 14 branch. (It applies cleanly to > master, but the unused member of PgStat_StatTabEntry needs to be > removed and catversion bumped). I have pushed this to both branches. (I did not remove the item from th

Re: Autovacuum on partitioned table (autoanalyze)

2021-08-16 Thread Álvaro Herrera
that? 1. Do nothing. If somebody finds in that situation, they can use ALTER TABLE .. RESET ... to remove the settings. 2. Silently accept the option and do nothing. 3. Accept the option and throw a warning that it's a no-op. 4. Something else Opinions? -- Álvaro Herrera 39

Re: Autovacuum on partitioned table (autoanalyze)

2021-08-16 Thread Álvaro Herrera
nd a tweak needed for this in master. A catversion bump would be required to change the definition of pg_stat_user_tables, which the patch being reverted originally changed to include relkind 'p'. A straight revert would remove that, but in my reversal patch I chose to keep it in place. -- Álvaro

Re: Autovacuum on partitioned table (autoanalyze)

2021-08-16 Thread Álvaro Herrera
Here's the reversal patch for the 14 branch. (It applies cleanly to master, but the unused member of PgStat_StatTabEntry needs to be removed and catversion bumped). -- Álvaro Herrera 39°49'30"S 73°17'W — https://www.EnterpriseDB.com/ Maybe there's lots of data loss but the re

Re: Autovacuum on partitioned table (autoanalyze)

2021-08-16 Thread Álvaro Herrera
On 2021-Aug-13, Álvaro Herrera wrote: > Some doc changes are pending, and some more commentary in parts of the > code, but I think this is much more sensible. I do lament the lack of > a syscache for pg_inherits. Thinking about this again, this one here is the killer problem

Re: Autovacuum on partitioned table (autoanalyze)

2021-08-13 Thread Álvaro Herrera
Here is a proposal for 14. This patch has four main changes: * The mod counts are only propagated to the topmost parent, not to each ancestor. This means that we'll only analyze the topmost partitioned table and not each intermediate partitioned table; seems a good compromise to avoid

Re: Race condition in InvalidateObsoleteReplicationSlots()

2021-07-05 Thread Álvaro Herrera
On 2021-Jul-05, vignesh C wrote: > On Wed, Jun 23, 2021 at 7:32 PM Álvaro Herrera > wrote: > > > > On 2021-Jun-20, Tom Lane wrote: > > > > > Actually ... isn't there a second race, in the opposite direction? > > > IIUC, the point of this

Re: Partitioned index can be not dumped

2021-07-01 Thread Álvaro Herrera
ced that with RelationGetIndexList() and checking if the list is nonempty, specifically for the partitioned table case. Pushed now. Thanks for reporting and fixing this, and to Zhihong Yu for reviewing. -- Álvaro Herrera Valdivia, Chile — https://www.EnterpriseDB.com/ "Investigación es lo que hago cuan

Re: Partitioned index can be not dumped

2021-06-30 Thread Álvaro Herrera
readability improvement. > My thinking is that without hasindex, the code is easier to maintain. You have one less variable but one additional concept (negative nindexes). It doesn't seem easier to me, TBH, rather the opposite. -- Álvaro Herrera39°49'30"

Re: Partitioned index can be not dumped

2021-06-30 Thread Álvaro Herrera
still pass true when there are indexes. Please don't forget to trim the text of the email you're replying to. -- Álvaro HerreraValdivia, Chile https://www.EnterpriseDB.com/

Re: Partitioned index can be not dumped

2021-06-30 Thread Álvaro Herrera
art of that comment seems to be a leftover from ... I don't know when: "We do not analyze index columns if there was an explicit column list in the ANALYZE command, however." I suppose this is about some code that was removed, but I didn't dig into it. -- Álvaro Herrera

Re: Partitioned index can be not dumped

2021-06-30 Thread Álvaro Herrera
s > field suddenly becomes false. Uh, ouch. I'll look into this shortly. -- Álvaro HerreraValdivia, Chile https://www.EnterpriseDB.com/

Re: Race condition in InvalidateObsoleteReplicationSlots()

2021-06-23 Thread Álvaro Herrera
ted for all those things and pushed again. -- Álvaro Herrera Valdivia, Chile "I can see support will not be a problem. 10 out of 10."(Simon Wittber) (http://archives.postgresql.org/pgsql-general/2004-12/msg00159.php)

Re: Race condition in InvalidateObsoleteReplicationSlots()

2021-06-20 Thread Álvaro Herrera
Hah, desmoxytes failed once: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=desmoxytes=2021-06-19%2003%3A06%3A04 I'll revert it and investigate later. There have been no other failures. -- Álvaro Herrera39°49'30"S 73°17'W "Hay que recordar que la

Re: Race condition in InvalidateObsoleteReplicationSlots()

2021-06-18 Thread Álvaro Herrera
Apologies, I inadvertently sent the version before I added a maximum number of iterations in the final loop. -- Álvaro Herrera Valdivia, Chile "La fuerza no está en los medios físicos sino que reside en una voluntad indomable" (Gandhi) >From 1492e9468ecd86167a1253c4a2e9b311398

Re: Race condition in InvalidateObsoleteReplicationSlots()

2021-06-18 Thread Álvaro Herrera
On 2021-Jun-11, Álvaro Herrera wrote: > I tried hard to make this stable, but it just isn't (it works fine one > thousand runs, then I grab some coffee and run it once more and that one > fails. Why? that's not clear to me). Attached is the last one I have, > in case somebody w

Re: Race condition in InvalidateObsoleteReplicationSlots()

2021-06-11 Thread Álvaro Herrera
On 2021-Apr-07, Andres Freund wrote: > After this I don't see a reason to have SAB_Inquire - as far as I can > tell it's practically impossible to use without race conditions? Except > for raising an error - which is "builtin"... Pushed 0002. Thanks!

Re: Race condition in InvalidateObsoleteReplicationSlots()

2021-06-11 Thread Álvaro Herrera
On 2021-Jun-10, Álvaro Herrera wrote: > Here's a version that I feel is committable (0001). There was an issue > when returning from the inner loop, if in a previous iteration we had > released the lock. In that case we need to return with the lock not > held, so that the caller

Re: Race condition in InvalidateObsoleteReplicationSlots()

2021-06-10 Thread Álvaro Herrera
On 2021-Jun-10, Álvaro Herrera wrote: > I wrote a test (0002) to cover the case of signalling a walsender, which > is currently not covered (we only deal with the case of a standby that's > not running). There are some sharp edges in this code -- I had to make > it use background_ps

Re: Race condition in InvalidateObsoleteReplicationSlots()

2021-06-10 Thread Álvaro Herrera
to at least see it in master, to gather some reactions from buildfarm. -- Álvaro Herrera Valdivia, Chile It does it in a really, really complicated way why does it need to be complicated? Because it's MakeMaker. >From f296a5b880a5688656d308300d1015ba0562c1b7 Mon Sep 17 00:00:00 2001 F

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-05-06 Thread Álvaro Herrera
tweaked the wording to match the surrounding text a bit better, at least IMO. Feel free to suggest improvements. Thanks! -- Álvaro Herrera39°49'30"S 73°17'W "That sort of implies that there are Emacs keystrokes which aren't obscure. I've been using it daily for 2

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-05-06 Thread Álvaro Herrera
this out in this paragraph. > May be make sense to add some details about DETACH CONCURRENTLY to the > section '5.11.2.2. Partition Maintenance' and completely remove this point? Maybe you're right, though. -- Álvaro Herrera39°49'30"S 73°17'W "Las muje

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-05-06 Thread Álvaro Herrera
actions. I didn't see it as terribly critical since it's supposed to be very transient, but I may be wrong. -- Álvaro Herrera Valdivia, Chile "Hay quien adquiere la mala costumbre de ser infeliz" (M. A. Evans)

Re: Race condition in InvalidateObsoleteReplicationSlots()

2021-04-29 Thread Álvaro Herrera
ld you like me to handle it? (There seems to be some minor conflict in 13) -- Álvaro Herrera Valdivia, Chile "Pido que me den el Nobel por razones humanitarias" (Nicanor Parra) >From 7f31b0ec12e52b6c967047384353895538161840 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Thu, 29

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-28 Thread Álvaro Herrera
Pushed that now, with a one-line addition to the docs that only one partition can be marked detached. -- Álvaro Herrera39°49'30"S 73°17'W "That sort of implies that there are Emacs keystrokes which aren't obscure. I've been using it daily for 2 years now an

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-28 Thread Álvaro Herrera
Thanks for re-reviewing! This one I hope is the last version. On Wed, Apr 28, 2021, at 10:21 AM, Amit Langote wrote: > I noticed that rd_partdesc_nodetached_xmin can sometimes end up with > value 0. While you seem to be already aware of that, because otherwise > you wouldn't have added

Re: some pointless HeapTupleHeaderIndicatesMovedPartitions calls

2021-03-25 Thread Álvaro Herrera
On 2021-Feb-24, Michael Paquier wrote: > On Mon, Feb 22, 2021 at 05:15:57PM -0300, Álvaro Herrera wrote: > > I changed my mind on this after noticing that > > ItemPointerIndicatesMovedPartitions has a few callers; leaving the > > interface incomplete/asymmetric would be worse

Re: [BUG] segfault during delete

2021-02-27 Thread Álvaro Herrera
Thanks Amit for working on this fix! It seems correct to me, so I pushed it with trivial changes. Thanks Bertrand for reporting the problem. In addition to backpatching the code fix to pg12, I backpatched the test case to pg11. It worked fine for me (with no code changes), but it seems good

Re: libpq debug log

2021-02-25 Thread Álvaro Herrera
This assert I added? > +static bool > +pqTraceMaybeBreakLine(int size, PGconn *conn) > +{ > + Assert(conn->be_msg->length > 0); It breaks. Maybe there's a good reason, maybe there ain't, but in the meantime I just removed it. -- Álvaro Herrera Valdivia, Chile &q

Re: libpq debug log

2021-02-25 Thread Álvaro Herrera
It appears that something is still wrong. I applied lipq pipeline v27 from [1] and ran src/test/modules/test_libpq/pipeline singlerow, after patching it to do PQtrace() after PQconn(). Below is the output I get from that. The noteworthy point is that "ParseComplete" messages appear multiple

Re: libpq debug log

2021-02-25 Thread Álvaro Herrera
I tweaked this code a little bit more. I didn't like that libpq-trace.h was exposing all the internal details of the API to the world; and I liked it even less that libpq-int.h had to include the file, exposing everything everywhere. I added some forward struct declarations in libpq-int.h to

Re: {CREATE INDEX, REINDEX} CONCURRENTLY improvements

2021-02-23 Thread Álvaro Herrera
hould be the same. But if we want to make that claim to save that one assignment, we can make it in a separate commit. However, given that this comparison and assignment is outside the locked area, I don't think it's worth bothering with. Thanks for the input! I have pushed this patch. -- Álvaro

Re: {CREATE INDEX, REINDEX} CONCURRENTLY improvements

2021-02-22 Thread Álvaro Herrera
On 2021-Feb-22, Álvaro Herrera wrote: > Here's an updated patch. > > I haven't added commentary or documentation to account for the new > assumption, per Matthias' comment and Robert's discussion thereof. Done that. I also restructured the code a bit, since one line in Compute

Re: some pointless HeapTupleHeaderIndicatesMovedPartitions calls

2021-02-22 Thread Álvaro Herrera
On 2021-Feb-21, Michael Paquier wrote: > On Sat, Feb 20, 2021 at 12:25:58PM -0300, Álvaro Herrera wrote: > > Hmm. Alternatively, maybe it'd make sense to change > > HeapTupleHeaderSetMovedPartition to use ItemPointerSetMovedPartitions > > instead of doing It

Re: {CREATE INDEX, REINDEX} CONCURRENTLY improvements

2021-02-22 Thread Álvaro Herrera
Here's an updated patch. I haven't added commentary or documentation to account for the new assumption, per Matthias' comment and Robert's discussion thereof. -- Álvaro Herrera39°49'30"S 73°17'W "Cada quien es cada cual y baja las escaleras como quiere&

Re: some pointless HeapTupleHeaderIndicatesMovedPartitions calls

2021-02-20 Thread Álvaro Herrera
PointerSetMovedPartitions instead of doing ItemPointerSet directly. But that looks mostly pointless, since the extensibility aspect of this interface has been superseded by table AM work. I agree we should just remove the macro and update the comment as you suggest. -- Álvaro Herrera Valdivia, Chile

Re: {CREATE INDEX, REINDEX} CONCURRENTLY improvements

2021-01-18 Thread Álvaro Herrera
On 2021-Jan-18, Matthias van de Meent wrote: > On Fri, 15 Jan 2021 at 15:29, Álvaro Herrera wrote: > Would this not need to be the following? Right now, it resets > potentially older h->catalog_oldest_nonremovable (which is set in the > PROC_IN

Re: {CREATE INDEX, REINDEX} CONCURRENTLY improvements

2021-01-18 Thread Álvaro Herrera
table during the initial index build process (which is > currently guaranteed through the use of a snapshot). VACUUM cannot run concurrently with CIC or RI in a table -- both acquire ShareUpdateExclusiveLock, which conflicts with itself, so this cannot occur. I do wonder if the problem you suggest

Re: {CREATE INDEX, REINDEX} CONCURRENTLY improvements

2021-01-15 Thread Álvaro Herrera
So one last remaining improvement was to have VACUUM ignore processes doing CIC and RC to compute the Xid horizon of tuples to remove. I think we can do something simple like the attached patch. -- Álvaro Herrera Valdivia, Chile "Aprender sin pensar es inútil; pensar sin apr

Re: {CREATE INDEX, REINDEX} CONCURRENTLY improvements

2021-01-15 Thread Álvaro Herrera
On 2021-Jan-12, Álvaro Herrera wrote: > On 2021-Jan-12, Álvaro Herrera wrote: > > > > For the 0001 patch, since ReindexIndexInfo is used only within > > > ReindexRelationConcurrently() I think it’s a function-local structure > > > type. So we can declare it withi

Re: {CREATE INDEX, REINDEX} CONCURRENTLY improvements

2021-01-12 Thread Álvaro Herrera
On 2021-Jan-12, Álvaro Herrera wrote: > > For the 0001 patch, since ReindexIndexInfo is used only within > > ReindexRelationConcurrently() I think it’s a function-local structure > > type. So we can declare it within the function. What do you think? > > Tha

Re: {CREATE INDEX, REINDEX} CONCURRENTLY improvements

2021-01-12 Thread Álvaro Herrera
unction. What do you think? That's a good idea. Pushed with that change, thanks. Thanks also to Dmitry and Hamid for the reviews. -- Álvaro Herrera39°49'30"S 73°17'W

Re: Online checksums patch - once again

2020-10-05 Thread Álvaro Herrera
On 2020-Oct-05, Heikki Linnakangas wrote: > The code in sendFile() in basebackup.c seems suspicious in that regard. It > calls DataChecksumsNeedVerify() once before starting to read the file. Isn't > it possible for the checksums flag to change while it's reading the file and > sending it to the