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

On Tue, 31 Mar 2026 at 20:48, Aleksander Alekseev <aleksander@tigerdata.com>
wrote:

Hi,

Currently extensible.c is not covered by any tests. The proposed patch
fixes this. Also it can serve as an example of using CustomScan and
ExtensibleNode.

For the reviewers
-----------------------

Here is how to check the code coverage:

```
git clean -df
rm -r build
meson setup --buildtype debug -Db_coverage=true -Dcassert=true
-Dinjection_points=true -Dtap_tests=enabled -Dldap=disabled
-Dicu=disabled -DPG_TEST_EXTRA="kerberos ldap libpq_encryption
load_balance oauth regress_dump_restore ssl wal_consistency_checking
xid_wraparound" -Dprefix=/home/eax/pginstall build
ninja -C build
meson test -C build
ninja -C build coverage-html
open build/meson-logs/coveragereport/index.html
```

You are going to need `lcov` 1.16 in your $PATH because there are
certain problems with newer versions [1].

[1]:
/messages/by-id/CAJ7c6TN+MCh99EZ8YGhXZAdnqvNQYir6E34B_mmcB5KsxCB00A@mail.gmail.com

--
Best regards,
Aleksander Alekseev

I looked into this patch and have a few comments.
Firstly, in test_create_custom_scan_state() why hard coding css.slotOps =
&TTSOpsBufferHeapTuple and not using the table_slot_callbacks() for the
purpose and getting the real AM.
Next, in test_plan_custom_path, the last two arguments are never used.
There are no tests for nodeCopy, nodeEqual, nodeOut, and nodeRead routines.

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

Hi Rafia,

I looked into this patch and have a few comments.
Firstly, in test_create_custom_scan_state() why hard coding css.slotOps = &TTSOpsBufferHeapTuple and not using the table_slot_callbacks() for the purpose and getting the real AM.
Next, in test_plan_custom_path, the last two arguments are never used.
There are no tests for nodeCopy, nodeEqual, nodeOut, and nodeRead routines.

Many thanks for the feedback! Here is the corrected patch.

--
Best regards,
Aleksander Alekseev

On Thu, 30 Jul 2026 at 18:21, Aleksander Alekseev <aleksander@tigerdata.com>
wrote:

Hi Rafia,

I looked into this patch and have a few comments.
Firstly, in test_create_custom_scan_state() why hard coding css.slotOps

= &TTSOpsBufferHeapTuple and not using the table_slot_callbacks() for the
purpose and getting the real AM.

Next, in test_plan_custom_path, the last two arguments are never used.
There are no tests for nodeCopy, nodeEqual, nodeOut, and nodeRead

routines.

Many thanks for the feedback! Here is the corrected patch.

Thanks. There is a typo in the current patch --... the CustomScanStateand

and is expected
Other than that it looks good to me.

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

Hi Rafia,

Thanks. There is a typo in the current patch --... the CustomScanStateand and is expected
Other than that it looks good to me.

Thanks! Fixed.

--
Best regards,
Aleksander Alekseev

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

On Tue, Aug 18, 2026 at 06:03:01PM +0200, Jan Nidzwetzki wrote:

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%.

FWIW, I have wanted an example of module for custom nodes for ages.
We should have added that years ago for coverage as much as to provide
a simple example of what can be done. Count me in.

- 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.

Yep.

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

Afterward, I could adjust test_set_rel_pathlist() and set:

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

Manipulations of the plans with GUCs would be nice as part of the
regression tests. We don't have to show all the possible patterns,
but to demonstrate some of the capabilities of this facility,
parallelism is a good one to see a custom node pushed down in a plan
tree.

+   tnode->relid = rte->relid;
+   tnode->repeat_count = 2;    /* each row will be returned twice */

Hmm. Let's make that parameterized to offer more variation. Having a
counter to trick the scan and have more tuples returns is a nice
concept for such a test module. Nice approach. But we could make
things slightly spicier:
- GUC to force a new per-query repeat_count.
- Much better for me: add_int_reloption() to force a custom scan to
return a number of rows you want with a CREATE TABLE .. WITH
(repeat_count = N). That's doable once the library is loaded, and
that should be a few extra lines of code.
So let's add the reloption to control the scans rather than hardcoding
it.

I am not much a fan of how test_ext_node_callbacks() is shaped; it
lacks extensibility. I'd like to think that we should be able to
define multiple TestExtNode with different repeat_count and then
compare them. It would be nice for the pluggability of the tests to
be able to pass around TestExtNode pointers (have an extra SQL
function to initialize a TestExtNode based on a repeat count), and
then have a set of SQL functions that act as thin wrappers on top of
the operations you want to do. You can see for example
test_bitmapset() for one idea. In short, I don't think that tests
like the incrementation of a repeat_count before a node compare offers
much value in itself. My idea would be to add one "init" function
that returns a TestExtNode pointer with a repeat_count in input,
passed around a few other functions to do more granular individual
operations (node print, node comparison, etc.).

Note: I am noting some missing trailing newlines, one in the 1.0.sql
script at least.
--
Michael