Thread
Commits
-
Make some use of anonymous unions [reloptions]
- 69f98fce5bfb 19 (unreleased) landed
-
Make some use of anonymous unions [libpq-oauth]
- f1881c7dd32c 19 (unreleased) landed
-
Make some use of anonymous unions [DSM registry].
- 74b41f5a77b8 19 (unreleased) landed
-
Make some use of anonymous unions [pg_locale_t]
- 4b7e6c73b0df 19 (unreleased) landed
-
Make some use of anonymous unions [pgcrypto]
- efcd5199d8cb 19 (unreleased) landed
-
Make some use of anonymous unions [plpython]
- 8b7f27fef3e2 19 (unreleased) landed
-
Make some use of anonymous unions [reorderbuffer xact_time]
- 57d46dff9b0b 19 (unreleased) landed
-
anonymous unions (C11)
Peter Eisentraut <peter@eisentraut.org> — 2025-09-23T09:38:22Z
Here is another patch set to sprinkle some C11 features around the code. My aim is to make a little bit of use of several C11 features as examples and encouragement for future code, and to test compilers (although we're not yet at the point where this should really stress any compiler). Here, I'm proposing to make some use of anonymous unions, which are a nice notational simplification if you nest unions inside structs, which is pretty common in PostgreSQL code. Often times, you have something like struct important_thing { int a; int b; union { int foo; int bar; } u; } (in PostgreSQL source code, often "u" is used, sometimes "d"), and then you have to address foo as varname.u.foo, which often looks cryptic (especially if there is more nesting). With anonymous unions, you can declare this as struct important_thing { int a; int b; union { int foo; int bar; }; } and write varname.foo, which often seems clearer. (C++ also allows this, so this will preserve C++ compatibility of the headers.) That said, I did go overboard here and converted all the struct/union combinations I could find, but I'm not necessarily proposing to apply all of them. I'm proposing patches 0001 through 0004, which are relatively simple or in areas that have already changed a few times recently (so backpatching would not be trivial anyway), and/or they are somewhat close to my heart because they originally motivated this work a long time ago. But if someone finds among the other patches one that they particularly like, we could add that one as well. -
Re: anonymous unions (C11)
Joel Jacobson <joel@compiler.org> — 2025-09-23T10:16:11Z
On Tue, Sep 23, 2025, at 11:38, Peter Eisentraut wrote: > Here is another patch set to sprinkle some C11 features around the > code. My aim is to make a little bit of use of several C11 features > as examples and encouragement for future code, and to test compilers > (although we're not yet at the point where this should really stress > any compiler). Nice, didn't know about this C11 feature. I've eyeballed the entire output of `git diff --word-diff-regex=.` and can't see any oddities. I've also run the full test suite and it passes. /Joel
-
Re: anonymous unions (C11)
Nathan Bossart <nathandbossart@gmail.com> — 2025-09-23T14:17:50Z
On Tue, Sep 23, 2025 at 11:38:22AM +0200, Peter Eisentraut wrote: > That said, I did go overboard here and converted all the struct/union > combinations I could find, but I'm not necessarily proposing to apply > all of them. I'm proposing patches 0001 through 0004, which are > relatively simple or in areas that have already changed a few times > recently (so backpatching would not be trivial anyway), and/or they > are somewhat close to my heart because they originally motivated this > work a long time ago. But if someone finds among the other patches > one that they particularly like, we could add that one as well. I would have used this in the DSM registry if it was available. Patch attached. -- nathan
-
Re: anonymous unions (C11)
Chao Li <li.evan.chao@gmail.com> — 2025-09-23T23:19:05Z
On Tue, Sep 23, 2025 at 5:38 PM Peter Eisentraut <peter@eisentraut.org> wrote: > > > That said, I did go overboard here and converted all the struct/union > combinations I could find, but I'm not necessarily proposing to apply > all of them. I'm proposing patches 0001 through 0004, which are > relatively simple or in areas that have already changed a few times > recently (so backpatching would not be trivial anyway), and/or they > are somewhat close to my heart because they originally motivated this > work a long time ago. But if someone finds among the other patches > one that they particularly like, we could add that one as well. I went through all commits and code changes are neat and clear. But when I tried to build on my Macbook M4, it got 18 errors against cryptohash.c: ``` cryptohash.c:108:22: error: no member named 'data' in 'struct pg_cryptohash_ctx' 108 | pg_md5_init(&ctx->data.md5); | ~~~ ^ cryptohash.c:111:23: error: no member named 'data' in 'struct pg_cryptohash_ctx' 111 | pg_sha1_init(&ctx->data.sha1); | ~~~ ^ cryptohash.c:114:25: error: no member named 'data' in 'struct pg_cryptohash_ctx' 114 | pg_sha224_init(&ctx->data.sha224); | ~~~ ^ cryptohash.c:117:25: error: no member named 'data' in 'struct pg_cryptohash_ctx' 117 | pg_sha256_init(&ctx->data.sha256); | ~~~ ^ cryptohash.c:120:25: error: no member named 'data' in 'struct pg_cryptohash_ctx' 120 | pg_sha384_init(&ctx->data.sha384); | ~~~ ^ cryptohash.c:123:25: error: no member named 'data' in 'struct pg_cryptohash_ctx' 123 | pg_sha512_init(&ctx->data.sha512); | ~~~ ^ cryptohash.c:144:24: error: no member named 'data' in 'struct pg_cryptohash_ctx' 144 | pg_md5_update(&ctx->data.md5, data, len); | ~~~ ^ cryptohash.c:147:25: error: no member named 'data' in 'struct pg_cryptohash_ctx' 147 | pg_sha1_update(&ctx->data.sha1, data, len); | ~~~ ^ cryptohash.c:150:27: error: no member named 'data' in 'struct pg_cryptohash_ctx' 150 | pg_sha224_update(&ctx->data.sha224, data, len); Fixed build failure on Macbook M4 | ~~~ ^ cryptohash.c:153:27: error: no member named 'data' in 'struct pg_cryptohash_ctx' 153 | pg_sha256_update(&ctx->data.sha256, data, len); | ~~~ ^ cryptohash.c:156:27: error: no member named 'data' in 'struct pg_cryptohash_ctx' 156 | pg_sha384_update(&ctx->data.sha384, data, len); | ~~~ ^ cryptohash.c:159:27: error: no member named 'data' in 'struct pg_cryptohash_ctx' 159 | pg_sha512_update(&ctx->data.sha512, data, len); | ~~~ ^ cryptohash.c:185:23: error: no member named 'data' in 'struct pg_cryptohash_ctx' 185 | pg_md5_final(&ctx->data.md5, dest); | ~~~ ^ cryptohash.c:193:24: error: no member named 'data' in 'struct pg_cryptohash_ctx' 193 | pg_sha1_final(&ctx->data.sha1, dest); | ~~~ ^ cryptohash.c:201:26: error: no member named 'data' in 'struct pg_cryptohash_ctx' 201 | pg_sha224_final(&ctx->data.sha224, dest); | ~~~ ^ cryptohash.c:209:26: error: no member named 'data' in 'struct pg_cryptohash_ctx' 209 | pg_sha256_final(&ctx->data.sha256, dest); | ~~~ ^ cryptohash.c:217:26: error: no member named 'data' in 'struct pg_cryptohash_ctx' 217 | pg_sha384_final(&ctx->data.sha384, dest); | ~~~ ^ cryptohash.c:225:26: error: no member named 'data' in 'struct pg_cryptohash_ctx' 225 | pg_sha512_final(&ctx->data.sha512, dest); | ~~~ ^ 18 errors generated. ``` With the attached fix, the build passed then, "make check" also passed. Chao Li (Evan) --------------------- HighGo Software Co., Ltd. https://www.highgo.com/ -
Re: anonymous unions (C11)
Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2025-09-30T11:21:51Z
On Wed, Sep 24, 2025 at 4:49 AM Chao Li <li.evan.chao@gmail.com> wrote: > > > On Tue, Sep 23, 2025 at 5:38 PM Peter Eisentraut <peter@eisentraut.org> wrote: >> >> >> >> That said, I did go overboard here and converted all the struct/union >> combinations I could find, but I'm not necessarily proposing to apply >> all of them. I'm proposing patches 0001 through 0004, which are >> relatively simple or in areas that have already changed a few times >> recently (so backpatching would not be trivial anyway), and/or they >> are somewhat close to my heart because they originally motivated this >> work a long time ago. But if someone finds among the other patches >> one that they particularly like, we could add that one as well. > > > I went through all commits and code changes are neat and clear. > > But when I tried to build on my Macbook M4, it got 18 errors against cryptohash.c: > ``` > cryptohash.c:108:22: error: no member named 'data' in 'struct pg_cryptohash_ctx' > 108 | pg_md5_init(&ctx->data.md5); > | ~~~ ^ ... snip ... > cryptohash.c:225:26: error: no member named 'data' in 'struct pg_cryptohash_ctx' > 225 | pg_sha512_final(&ctx->data.sha512, dest); > | ~~~ ^ > 18 errors generated. I am using meson and gcc with following versions $ gcc --version gcc (Ubuntu 11.4.0-1ubuntu1~22.04.2) 11.4.0 Copyright (C) 2021 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. $ meson --version 0.61.2 I am getting opposite of those errors ../../coderoot/pg/src/common/cryptohash.c: In function ‘pg_cryptohash_init’: ../../coderoot/pg/src/common/cryptohash.c:108:41: error: ‘pg_cryptohash_ctx’ has no member named ‘md5’ 108 | pg_md5_init(&ctx->md5); | ^~ ... snip ... ../../coderoot/pg/src/common/cryptohash.c:225:45: error: ‘pg_cryptohash_ctx’ has no member named ‘sha512’ 225 | pg_sha512_final(&ctx->sha512, dest); Attached patch fixes those errors for me. -- Best Wishes, Ashutosh Bapat -
Re: anonymous unions (C11)
Álvaro Herrera <alvherre@kurilemu.de> — 2025-09-30T11:44:00Z
On 2025-Sep-30, Ashutosh Bapat wrote: > Attached patch fixes those errors for me. > diff --git a/src/common/cryptohash.c b/src/common/cryptohash.c > index fc0555d2f99..e8a35c2ec35 100644 > --- a/src/common/cryptohash.c > +++ b/src/common/cryptohash.c > @@ -61,7 +61,7 @@ struct pg_cryptohash_ctx > pg_sha256_ctx sha256; > pg_sha384_ctx sha384; > pg_sha512_ctx sha512; > - } data; > + }; > }; Isn't this patch 0015 in Peter's series? -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "Saca el libro que tu religión considere como el indicado para encontrar la oración que traiga paz a tu alma. Luego rebootea el computador y ve si funciona" (Carlos Duclós)
-
Re: anonymous unions (C11)
Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2025-09-30T14:41:08Z
On Tue, Sep 30, 2025 at 5:14 PM Álvaro Herrera <alvherre@kurilemu.de> wrote: > > On 2025-Sep-30, Ashutosh Bapat wrote: > > > Attached patch fixes those errors for me. > > > diff --git a/src/common/cryptohash.c b/src/common/cryptohash.c > > index fc0555d2f99..e8a35c2ec35 100644 > > --- a/src/common/cryptohash.c > > +++ b/src/common/cryptohash.c > > @@ -61,7 +61,7 @@ struct pg_cryptohash_ctx > > pg_sha256_ctx sha256; > > pg_sha384_ctx sha384; > > pg_sha512_ctx sha512; > > - } data; > > + }; > > }; > > Isn't this patch 0015 in Peter's series? I found the compilation errors after pulling the latest sources at that time. I thought the patch series is committed and hence this report and patch. Didn't look at the patch series. But the problematic commit has been reverted since then. -- Best Wishes, Ashutosh Bapat
-
Re: anonymous unions (C11)
Álvaro Herrera <alvherre@kurilemu.de> — 2025-09-30T14:50:52Z
On 2025-Sep-30, Ashutosh Bapat wrote: > I found the compilation errors after pulling the latest sources at > that time. I thought the patch series is committed and hence this > report and patch. Didn't look at the patch series. Ah, okay. -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
-
Re: anonymous unions (C11)
Peter Eisentraut <peter@eisentraut.org> — 2025-10-03T07:01:26Z
On 23.09.25 16:17, Nathan Bossart wrote: > On Tue, Sep 23, 2025 at 11:38:22AM +0200, Peter Eisentraut wrote: >> That said, I did go overboard here and converted all the struct/union >> combinations I could find, but I'm not necessarily proposing to apply >> all of them. I'm proposing patches 0001 through 0004, which are >> relatively simple or in areas that have already changed a few times >> recently (so backpatching would not be trivial anyway), and/or they >> are somewhat close to my heart because they originally motivated this >> work a long time ago. But if someone finds among the other patches >> one that they particularly like, we could add that one as well. > > I would have used this in the DSM registry if it was available. Patch > attached. This looks good to me, and also mostly harmless in terms of backpatching.
-
Re: anonymous unions (C11)
Nathan Bossart <nathandbossart@gmail.com> — 2025-10-03T15:16:11Z
On Fri, Oct 03, 2025 at 09:01:26AM +0200, Peter Eisentraut wrote: > On 23.09.25 16:17, Nathan Bossart wrote: >> I would have used this in the DSM registry if it was available. Patch >> attached. > > This looks good to me, and also mostly harmless in terms of backpatching. Committed, thanks for reviewing. -- nathan
-
Re: anonymous unions (C11)
Jacob Champion <jacob.champion@enterprisedb.com> — 2025-10-16T17:40:10Z
On Tue, Sep 23, 2025 at 7:18 AM Nathan Bossart <nathandbossart@gmail.com> wrote: > I would have used this in the DSM registry if it was available. Patch > attached. Likewise for libpq-oauth, as attached. Thanks, --Jacob
-
Re: anonymous unions (C11)
Nathan Bossart <nathandbossart@gmail.com> — 2025-10-17T14:23:15Z
On Thu, Oct 16, 2025 at 10:40:10AM -0700, Jacob Champion wrote: > Likewise for libpq-oauth, as attached. LGTM -- nathan
-
Re: anonymous unions (C11)
Jacob Champion <jacob.champion@enterprisedb.com> — 2025-11-24T20:27:20Z
On Fri, Oct 17, 2025 at 7:23 AM Nathan Bossart <nathandbossart@gmail.com> wrote: > LGTM (Pushed, thanks!) --Jacob
-
Re: anonymous unions (C11)
Álvaro Herrera <alvherre@kurilemu.de> — 2026-01-19T21:10:25Z
On 2025-Sep-23, Peter Eisentraut wrote: > Here is another patch set to sprinkle some C11 features around the > code. My aim is to make a little bit of use of several C11 features > as examples and encouragement for future code, and to test compilers > (although we're not yet at the point where this should really stress > any compiler). I was going over patch [1] today and came across the union in relopt_value. Vaguely remembering this thread, I patched it out (attached) ... and now that I come here to post it, I realize that it's probably identical to your patch 0009 here. Would you push it or can I? [1] https://postgr.es/m/4047390.3Lj2Plt8kZ@thinkpad-pgpro -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
-
Re: anonymous unions (C11)
Peter Eisentraut <peter@eisentraut.org> — 2026-01-21T11:21:55Z
On 19.01.26 22:10, Álvaro Herrera wrote: > On 2025-Sep-23, Peter Eisentraut wrote: > >> Here is another patch set to sprinkle some C11 features around the >> code. My aim is to make a little bit of use of several C11 features >> as examples and encouragement for future code, and to test compilers >> (although we're not yet at the point where this should really stress >> any compiler). > > I was going over patch [1] today and came across the union in > relopt_value. Vaguely remembering this thread, I patched it out > (attached) ... and now that I come here to post it, I realize that it's > probably identical to your patch 0009 here. > > Would you push it or can I? Go ahead please.