| From: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> |
|---|---|
| To: | Peter Eisentraut <peter(at)eisentraut(dot)org> |
| Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: rename and move AssertVariableIsOfType |
| Date: | 2026-02-03 13:39:22 |
| Message-ID: | aYH6ii46AvGVCB84@ip-10-97-1-34.eu-west-3.compute.internal |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Tue, Feb 03, 2026 at 09:09:17AM +0100, Peter Eisentraut wrote:
> On 27.01.26 13:55, Bertrand Drouvot wrote:
> > Hi,
> >
> > On Mon, Jan 26, 2026 at 01:17:15PM +0100, Peter Eisentraut wrote:
> > > I'm proposing two changes:
> > >
> > > First, rename AssertVariableIsOfType to StaticAssertVariableIsOfType. The
> > > current name suggests that it is a run-time assertion (like "Assert"), but
> > > it's not. The name change makes that clearer.
> > >
> > > I doubt that the current name is used in many extensions, but if necessary,
> > > extension code could adapt to this quite easily with something like
> > >
> > > #if PG_VERSION_NUM < ...
> > > #define StaticAssertVariableIsOfType(x, y) AssertVariableIsOfType(x, y)
> > > #endif
> > >
> > > Second, change the underlying implementation of StaticAssertVariableIsOfType
> > > to use StaticAssertDecl instead of StaticAssertStmt. This makes
> > > StaticAssertVariableIsOfType behave more like a normal static assertion, and
> > > in many cases we can move the current instances to a more natural position
> > > at file scope. This is similar to previous commits like 493eb0da31b.
> >
> > Both make sense and looks good to me.
>
> Thanks, committed.
>
> > Once they are in, I'm wondering if the remaining StaticAssertStmt ones:
> >
> > src/backend/backup/basebackup.c: StaticAssertStmt(2 * TAR_BLOCK_SIZE <= BLCKSZ,
> > src/backend/storage/lmgr/deadlock.c: StaticAssertStmt(MAX_BACKENDS_BITS <= (32 - 3),
> > src/backend/utils/mmgr/aset.c: StaticAssertStmt(ALLOC_CHUNK_LIMIT == ALLOCSET_SEPARATE_THRESHOLD,
> >
> > could be replaced by StaticAssertDecl() too (that has not been done in 493eb0da31b
> > and (from a quick scan) not mentioned in the linked thread). I did not look in
> > details so maybe there is good reasons to keep them.
>
> Yeah, maybe it would be good to get rid of these remaining few. I suppose
> we could just change Stmt to Decl and put braces around the block, but maybe
> there are some more elegant places to move these.
Yeah, I gave it a try and I did not choose the same place for all the files.
1/ basebackup.c
Since changing the remaining StaticAssertStmt to StaticAssertDecl introduces
a duplicate, I thought it would make sense to:
- remove the StaticAssertStmt
- move the existing StaticAssertDecl at file scope
As it depends of the literal "2" also used in some computation then I introduced
TAR_TERMINATION_BLOCKS and used it in the StaticAssertDecl and the functions.
2/ deadlock.c
It makes sense to keep it near the related code, so:
- changed to StaticAssertDecl
- Added new braces to avoid Wdeclaration-after-statement to trigger
3/ aset.c
Changes the StaticAssertStmt to StaticAssertDecl and move it to file scope (that
looks more appropriate).
Attached 3 patches to ease the review.
After there are no remaining usages of StaticAssertStmt() and we may want to
deprecate it.
Thoughts?
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0001-Change-StaticAssertStmt-to-StaticAssertDecl-in-ba.patch | text/x-diff | 2.4 KB |
| v1-0002-Change-StaticAssertStmt-to-StaticAssertDecl-in-de.patch | text/x-diff | 1.4 KB |
| v1-0003-Change-StaticAssertStmt-to-StaticAssertDecl-in-as.patch | text/x-diff | 1.7 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Shinya Kato | 2026-02-03 13:49:14 | Add LIMIT option to COPY FROM |
| Previous Message | Heikki Linnakangas | 2026-02-03 13:31:36 | Re: Refactor recovery conflict signaling a little |