Hi Nathan, V2 overall looks good to me. I just got a question and a few nit comments.
> On Dec 1, 2025, at 23:29, Nathan Bossart <[email protected]> wrote: > > -- > nathan > <v2-0001-rework-DSM-registry-view.patch> 1 - dsa.c ``` +size_t +dsa_get_total_size_from_handle(dsa_handle handle) +{ + size_t size; + bool already_attached; + dsm_segment *segment; + dsa_area_control *control; + + already_attached = dsa_is_attached(handle); + if (already_attached) + segment = dsm_find_mapping(handle); + else + segment = dsm_attach(handle); + + if (segment == NULL) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("could not attach to dynamic shared area"))); ``` Why do you error out instead of reporting 0/NULL usage here? When users call this function as shown in the test script: ``` CREATE VIEW pg_dsm_registry_allocations AS SELECT * FROM pg_get_dsm_registry_allocations(); ``` User doesn’t pass in a handle, if the DSA has been released, it should treat a missing mapping as “no size available” and maybe return NULL. 2 ``` + else if (entry->type == DSMR_ENTRY_TYPE_DSH && + entry->dsh.dsa_handle !=DSA_HANDLE_INVALID) ``` Missing a white space after !=. 3 ``` - Size of the allocation in bytes. NULL for entries of type - <literal>area</literal> and <literal>hash</literal>. + Size of the allocation in bytes. NULL for entries that failed + initialization. ``` “Failed initialization”, I guess “to” is needed after “failed”. 4 ``` -SELECT name, type, size IS DISTINCT FROM 0 AS size +SELECT name, type, size > 0 AS size ``` As you changed the third column from “size” to “size > 0”, now it’s a bool column, so maybe rename the column alias from “size” to something indicating a bool type, such as “size_ok”, “has_size”, etc. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
