| From: | "Jelte Fennema-Nio" <postgres(at)jeltef(dot)nl> |
|---|---|
| To: | "Chao Li" <li(dot)evan(dot)chao(at)gmail(dot)com> |
| Cc: | "Bertrand Drouvot" <bertranddrouvot(dot)pg(at)gmail(dot)com>, "Thomas Munro" <thomas(dot)munro(at)gmail(dot)com>, <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Safer hash table initialization macro |
| Date: | 2026-01-27 18:10:11 |
| Message-ID: | DFZKNSINQ9I1.1JOM73FKP88HC@jeltef.nl |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Tue Jan 27, 2026 at 8:26 AM CET, Chao Li wrote:
> This function has a lot of duplicate checks on opts!=NULL, I think it can be simplified as:
Good suggestion. Done.
> 2 - 0002
> Why this function returns a structure by value? Which looks quite uncommon. Usually, when a function is named with “new”, it returns a pointer to a new object.
I changed the name to foreach_hash_start and made it a static inline
function in the header instead. I updated the comment to explain why
it's needed (i.e. to initialize the local varaiable).
> 3 - 0002
> foreach_hash feels fragile. It requires to call foreach_hash_term before break, which is easy to forget. And the documentation doesn’t mention how to continue, how to return from a loop, and how to goto from inside a loop.
I agree that it'd be better if foreach_hash_term was not needed. But
that would be a lot bigger change (see the "SEQ SCAN TRACKING" comment).
I've updated the comment on foreach_hash_term to be more specific about
when it's needed.
| Attachment | Content-Type | Size |
|---|---|---|
| v10-0001-Add-hash_make-macros.patch | text/x-patch | 15.8 KB |
| v10-0002-Add-foreach_hash-macro.patch | text/x-patch | 3.6 KB |
| v10-0003-Use-hash_make-macros-throughout-the-codebase.patch | text/x-patch | 97.6 KB |
| v10-0004-Use-foreach_hash-macro-throughout-the-codebase.patch | text/x-patch | 72.3 KB |
| v10-0005-Inline-functions-that-have-now-become-trivial.patch | text/x-patch | 4.5 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Robert Haas | 2026-01-27 18:15:47 | Re: pgsql: Prevent invalidation of newly synced replication slots. |
| Previous Message | David G. Johnston | 2026-01-27 18:02:34 | Re: Add SECURITY_INVOKER_VIEWS option to CREATE DATABASE |