Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR removes C++-only constructs from several extension source files so they can be compiled as C, aligning with the goal of “convert C++ files to C where simple”.
Changes:
- Replaced C++-only syntax/types (
extern "C",nullptr,static_cast,std::array,<csignal>, etc.) with C equivalents. - Updated several function signatures to C-style prototypes (
(void)for no-arg functions) and Postgres-stylepg_attribute_unused()for unused parameters. - Converted some
struct/enumdeclarations in headers totypedefforms to improve C ergonomics.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/worker/bgworker.c | Removes C++ linkage blocks and C++-isms; switches to C headers and prototypes. |
| src/queue/shmem.c | Converts globals/casts/nulls to C equivalents; removes C++ linkage blocks. |
| src/queue/ring_entry.h | Introduces typedef struct for easier C usage. |
| src/queue/psch_dsa.h | Introduces typedef struct and updates API signature to use typedef name. |
| src/queue/psch_dsa.c | Removes C++ linkage blocks and replaces C++ casts/nulls with C equivalents. |
| src/queue/local_batch.c | Replaces std::array local batching with a plain C array; removes C++ linkage blocks. |
| src/queue/event.h | Converts enum/struct to typedef forms and updates related inline function signature. |
| src/pg_stat_ch.c | Removes C++ linkage blocks and replaces nullptr with NULL. |
| src/hooks/string_utils.h | Replaces C++ headers/casts/nulls with C equivalents; makes header helpers static inline. |
| src/hooks/query_normalize.c | Removes C++ linkage blocks and replaces C++ casts/nulls with C equivalents. |
| src/hooks/hooks.c | Removes C++ linkage blocks/types and replaces std::array with C arrays; converts casts/nulls. |
| src/config/guc.c | Removes C++ linkage blocks/types; replaces std::array enum table with a C array. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/queue/ring_entry.h:32
PschRingEntry/PschEventlayout verification is currently guarded by#ifdef __cplusplus(thestatic_assertblock). After converting the struct to a C-friendly typedef, this header appears to be included only from C translation units, so those compile-time layout checks no longer run. Consider adding equivalent assertions for C builds (e.g., using_Static_assertor PostgreSQL's static-assert macros) so ABI/layout mismatches are still caught at compile time.
typedef struct PschRingEntry {
// === Timing ===
TimestampTz ts_start;
uint64 duration_us;
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
src/hooks/hooks.c:6
postgres.his expected to be the first include in source files (per repo style guidance) to avoid subtle macro/typedef ordering issues with PostgreSQL headers. Here it comes after<sys/resource.h>; please move#include "postgres.h"to the top and include system headers afterwards.
src/queue/shmem.c:90- The comment says the layout is "Verified by static assert in ring_entry.h", but the code uses
StaticAssertDecl(...)(and more generally this is a static assertion, not a "static assert"). Please update the wording to match the actual mechanism to avoid confusion when grepping/debugging.
No description provided.