Re: [PATCH] Add tests for src/backend/nodes/extensible.c

Hi,

On 05.08.26 13:40, Aleksander Alekseev wrote:
>>>> Currently extensible.c is not covered by any tests. The proposed
>>>> patch fixes this.

[...]

> Thanks! Fixed.

Thanks for working on this. I have applied the v3 version of the patch
on my system (gcc 14, --buildtype debug, cassert on) and checked the
coverage. In my test, the test coverage for
src/backend/nodes/extensible.c increased from 0% to 94.29%.

My notes from reading the patch:

- Other tests in src/test/modules contain a .gitignore file. This patch
does not include one.

- test_get_extensible_node_methods() and test_get_custom_scan_methods()
are not marked as STRICT and passing a NULL argument crashes the backend
(both call PG_GETARG_TEXT_PP(0) unconditionally). In contrast,
test_ext_node_callbacks() is already STRICT, so this looks like an
oversight.

A possible enhancement:

The comment in _PG_init() justifies the shared_preload_libraries
requirement by saying that parallel workers read the plan back with
stringToNode(). That is true in principle, but the CustomPath is never
marked parallel safe, so no Gather is ever built and the path is never
taken: nodeOut() and nodeRead() are reached only via
test_ext_node_callbacks(), but never through the executor.

I did a few experiments, and with the following modifications, I was
able to create such a plan:

I changed:

-+-
rel = table_open(tnode->relid, NoLock);
[...]
table_close(rel, NoLock);
-+-

to

-+-
rel = table_open(tnode->relid, AccessShareLock);
[...]
table_close(rel, AccessShareLock);
-+-

Afterward, I could adjust test_set_rel_pathlist() and set:

-+-
cpath->path.parallel_safe = true;
-+-

Now, the following query gives the desired test coverage:

-+-
SET debug_parallel_query = regress;
SELECT id, val FROM test_extensible_tbl ORDER BY id;
RESET debug_parallel_query;
-+-

With 'debug_parallel_query = on', the query uses the following query plan:

-+-
jan=# SET debug_parallel_query = on;
SET
jan=# EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT id, val FROM
test_extensible_tbl ORDER BY id;
QUERY PLAN
----------------------------------------------------------------------
Gather (actual rows=6.00 loops=1)
Workers Planned: 1
Workers Launched: 1
Single Copy: true
Buffers: shared hit=38
-> Sort (actual rows=6.00 loops=1)
Sort Key: id
Buffers: shared hit=38
Worker 0: Sort Method: quicksort Memory: 25kB
-> Custom Scan (TestCustomScan) on test_extensible_tbl [...]
Buffers: shared hit=1
Planning Time: 0.140 ms
Execution Time: 12.729 ms
(13 rows)
-+-

Best regards
Jan

--
Jan Nidzwetzki
PlanetScale Postgres Core Team