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
signature.asc
Description: PGP signature
