Re: Cleanup: remove unused fields from nodes

2024-04-24 Thread Michael Paquier
On Wed, Apr 24, 2024 at 08:31:57AM -0400, Tom Lane wrote: > I was thinking about wording like > > * True if DEALLOCATE ALL. This is redundant with "name == NULL", > * but we make it a separate field so that exactly this condition > * (and not the precise name) will be accounted

Re: Cleanup: remove unused fields from nodes

2024-04-24 Thread Tom Lane
Michael Paquier writes: > On Tue, Apr 23, 2024 at 11:03:40PM -0400, Tom Lane wrote: >> Hah. Seems like the comment for isall needs to explain that it >> exists for this purpose, so we don't make this mistake again. > How about something like the attached? I was thinking about wording like

Re: Cleanup: remove unused fields from nodes

2024-04-24 Thread Matthias van de Meent
On Wed, 24 Apr 2024 at 09:28, Michael Paquier wrote: > > On Tue, Apr 23, 2024 at 11:03:40PM -0400, Tom Lane wrote: > > Hah. Seems like the comment for isall needs to explain that it > > exists for this purpose, so we don't make this mistake again. > > How about something like the attached?

Re: Cleanup: remove unused fields from nodes

2024-04-24 Thread Michael Paquier
On Tue, Apr 23, 2024 at 11:03:40PM -0400, Tom Lane wrote: > Hah. Seems like the comment for isall needs to explain that it > exists for this purpose, so we don't make this mistake again. How about something like the attached? -- Michael diff --git a/src/include/nodes/parsenodes.h

Re: Cleanup: remove unused fields from nodes

2024-04-23 Thread Tom Lane
Michael Paquier writes: > On Tue, Apr 23, 2024 at 01:01:04PM -0400, Tom Lane wrote: >> That is, query jumbling no longer distinguishes "DEALLOCATE x" from >> "DEALLOCATE ALL", because the DeallocateStmt.name field is marked >> query_jumble_ignore. Now maybe that's fine, but it's a point >> we'd

Re: Cleanup: remove unused fields from nodes

2024-04-23 Thread Michael Paquier
On Tue, Apr 23, 2024 at 01:01:04PM -0400, Tom Lane wrote: > That is, query jumbling no longer distinguishes "DEALLOCATE x" from > "DEALLOCATE ALL", because the DeallocateStmt.name field is marked > query_jumble_ignore. Now maybe that's fine, but it's a point > we'd not considered so far in this

Re: Cleanup: remove unused fields from nodes

2024-04-23 Thread Tom Lane
Michael Paquier writes: > On Mon, Apr 22, 2024 at 06:46:27PM +0200, Matthias van de Meent wrote: >> On Mon, 22 Apr 2024 at 17:41, Tom Lane wrote: >>> I think it would be a good idea to push 0003 for v17, just so nobody >>> grows an unnecessary dependency on that field. 0001 and 0005 could >>>

Re: Cleanup: remove unused fields from nodes

2024-04-22 Thread Michael Paquier
On Mon, Apr 22, 2024 at 06:46:27PM +0200, Matthias van de Meent wrote: > On Mon, 22 Apr 2024 at 17:41, Tom Lane wrote: >> Matthias van de Meent writes: >>> 0002/0004 remove fields in ExecRowMark which were added for FDWs to >>> use, but there are no FDWs which use this: I could only find two

Re: Cleanup: remove unused fields from nodes

2024-04-22 Thread Matthias van de Meent
On Mon, 22 Apr 2024 at 17:41, Tom Lane wrote: > > Matthias van de Meent writes: > > 0002/0004 remove fields in ExecRowMark which were added for FDWs to > > use, but there are no FDWs which use this: I could only find two FDWs > > who implement RefetchForeignRow, one being BlackHoleFDW, and the

Re: Cleanup: remove unused fields from nodes

2024-04-22 Thread Tom Lane
Matthias van de Meent writes: > 0001 removes two old fields that are not in use anywhere anymore, but > at some point these were used. +1. They're not being initialized, so they're useless and confusing. > 0002/0004 remove fields in ExecRowMark which were added for FDWs to > use, but there are

Cleanup: remove unused fields from nodes

2024-04-22 Thread Matthias van de Meent
Hi, While working on the 'reduce nodeToString output' patch, I noticed that my IDE marked one field in the TidScanState node as 'unused'. After checking this seemed to be accurate, and I found a few more such fields in Node structs. PFA some patches that clean this up: 0001 is plain removal of