[PATCH] Cover get_json_table_plan() with tests

I noticed that get_json_table_plan() is never executed by the existing
tests. The proposed patch fixes this.

Hi Aleksander,
I tested this patch on the current PostgreSQL master.The patch applied
cleanly, but make check reported one regression test failure in
sqljson_jsontable.
Looking at regression.diffs, the failure comes from the newly added
JSON_TABLE ... PLAN (...) statements. On my setup, they fail with:
ERROR: syntax error at or near "PLAN"
I also tried running the same JSON_TABLE ... PLAN (...) query manually
in psql and got the same syntax error.
Apart from that, I verified that the existing JSON_TABLE functionality
still works as expected. I tested SELECT, EXPLAIN, EXPLAIN VERBOSE,
and EXPLAIN ANALYZE, and they all produced the expected results.
Is this patch expected to be applied on top of another patch that adds
support for the JSON_TABLE PLAN (...) syntax?

Regards,
Solai

Hi Solai,

Thanks for the feedback!

I tested this patch on the current PostgreSQL master.The patch applied
cleanly, but make check reported one regression test failure in
sqljson_jsontable.
Looking at regression.diffs, the failure comes from the newly added
JSON_TABLE ... PLAN (...) statements. On my setup, they fail with:
ERROR: syntax error at or near "PLAN"
I also tried running the same JSON_TABLE ... PLAN (...) query manually
in psql and got the same syntax error.
Apart from that, I verified that the existing JSON_TABLE functionality
still works as expected. I tested SELECT, EXPLAIN, EXPLAIN VERBOSE,
and EXPLAIN ANALYZE, and they all produced the expected results.
Is this patch expected to be applied on top of another patch that adds
support for the JSON_TABLE PLAN (...) syntax?

The patch needs a rebase after today's commits. I will figure it out
and submit the updated version.

--
Best regards,
Aleksander Alekseev

Hi,

The patch needs a rebase after today's commits. I will figure it out
and submit the updated version.

Rebased.

I also tried running the same JSON_TABLE ... PLAN (...) query manually
in psql and got the same syntax error.

Works for me. Make sure you are using the latest version of the `master` branch.

--
Best regards,
Aleksander Alekseev

Hello!~

Aleksander Alekseev <aleksander@tigerdata.com> writes:

Hi,

The patch needs a rebase after today's commits. I will figure it out
and submit the updated version.

Rebased.

I also tried running the same JSON_TABLE ... PLAN (...) query manually
in psql and got the same syntax error.

Works for me. Make sure you are using the latest version of the `master` branch.

I checked on the coverage that this patch effectively improves, but
reviewing the report I notice that there's still some gap that can be
fixed by just adding another nested path like this:

NESTED PATH '$[*]' AS p1 COLUMNS (NESTED PATH '$[*]' AS p10 COLUMNS (a int))
NESTED PATH '$[*]' AS p2 COLUMNS (NESTED PATH '$[*]' AS p20 COLUMNS (b int))

This will still leave space for another for these lines

IsA(j->rplan, JsonTableSiblingJoin)
and
IsA(j->lplan, JsonTableSiblingJoin)

Probably requires adding another query to the tests? Since the objective
is to increase the test coverage, it may worthy to add another query
that helps to get closer to the 100% coverage of that function.

Regards,
--
Jonathan Gonzalez V.
EDB
https://www.enterprisedb.com

Hi Jonathan,

Thanks for the feedback.

I checked on the coverage that this patch effectively improves, but
reviewing the report I notice that there's still some gap that can be
fixed by just adding another nested path like this:

NESTED PATH '$[*]' AS p1 COLUMNS (NESTED PATH '$[*]' AS p10 COLUMNS (a int))
NESTED PATH '$[*]' AS p2 COLUMNS (NESTED PATH '$[*]' AS p20 COLUMNS (b int))

Could you please clarify which lines of get_json_table_plan() this
will cover which are not currently covered by patch v2?

--
Best regards,
Aleksander Alekseev

On Wed, Jul 22, 2026 at 01:49:34AM +0300, Aleksander Alekseev wrote:

I checked on the coverage that this patch effectively improves, but
reviewing the report I notice that there's still some gap that can be
fixed by just adding another nested path like this:

NESTED PATH '$[*]' AS p1 COLUMNS (NESTED PATH '$[*]' AS p10 COLUMNS (a int))
NESTED PATH '$[*]' AS p2 COLUMNS (NESTED PATH '$[*]' AS p20 COLUMNS (b int))

Could you please clarify which lines of get_json_table_plan() this
will cover which are not currently covered by patch v2?

What are the old and new coverage numbers before and after the fact,
and where are the zones impacted? I am seeing a mention to
"improvements", but zero fact explaining things. This is mostly to
get a feeling of the gain vs the runtime involved.
--
Michael

Hello!

Could you please clarify which lines of get_json_table_plan() this
will cover which are not currently covered by patch v2?

What are the old and new coverage numbers before and after the fact,
and where are the zones impacted? I am seeing a mention to
"improvements", but zero fact explaining things. This is mostly to
get a feeling of the gain vs the runtime involved.

Yes, I'm sorry I forgot to include the numbers and the facts, my bad!

So the v2 to is missing the following lines

12779: castNode(JsonTablePathScan, j->lplan)->child);
12785: castNode(JsonTablePathScan, j->rplan)->child);

Which will be covered by my suggestion, these are marked as not tested
in a 2 branch in the coverage.

The current coverage for the function get_json_table_plan() without the
v2 is:

get_json_table_plan (line 12752) called 3 times, returned 3 times 54.0%

With the v2:

get_json_table_plan (line 12752) called 11 times, returned 11 times 95.0%

Applying my suggestion

get_json_table_plan (line 12752) called 38 times, returned 38 times 100.0%

Those are the full numbers for that function.

Regards!
--
Jonathan Gonzalez V.
EDB
https://www.enterprisedb.com

Hi Michael,

What are the old and new coverage numbers before and after the fact,
and where are the zones impacted? I am seeing a mention to
"improvements", but zero fact explaining things.

I used the command:

```
time -p sh -c 'git clean -df && (rm -r build || true) && 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/src/backend/utils/adt/ruleutils.c.gcov.html'
```

My lcov version is 1.16. Here are before and after:

https://gist.github.com/afiskon/526f847d5e4ad786ca43bb63c2081bdf

Jonathan,

The current coverage for the function get_json_table_plan() without the
v2 is:

get_json_table_plan (line 12752) called 3 times, returned 3 times 54.0%

With the v2:

get_json_table_plan (line 12752) called 11 times, returned 11 times 95.0%

Applying my suggestion

get_json_table_plan (line 12752) called 38 times, returned 38 times 100.0%

OK, I got your idea. Also I missed the fact that with the recent
updates in the `master` branch ~50% of the function is already
executed. This was not the case when I submitted v1.

I don't think that we should be that pedantic to cover `||
castNode(JsonTablePathScan, j->lplan)->child)`. We can't reach 100%
coverage anyway because in this case we should consider the number of
states which grows as fast as pow(2, N) where N is the number of bits
in the state. This is not the goal. The goal is to catch the moment
when changes in the code break something. Which can't be done if the
code never executes. This is the gain vs the runtime involved, as
Michael put it.

So what I actually need to do is *remove* part which executes `if
(IsA(plan, JsonTablePathScan))` branch since now it is executed
without the patch. I will submit an updated version.

--
Best regards,
Aleksander Alekseev

Hi,

So what I actually need to do is *remove* part which executes `if
(IsA(plan, JsonTablePathScan))` branch since now it is executed
without the patch. I will submit an updated version.

Here is the simplified patch.

--
Best regards,
Aleksander Alekseev

Aleksander Alekseev <aleksander@tigerdata.com> writes:

Hello!

Hi,

So what I actually need to do is *remove* part which executes `if
(IsA(plan, JsonTablePathScan))` branch since now it is executed
without the patch. I will submit an updated version.

Here is the simplified patch.

I've reviewed the new version and now the coverage with this version is
92%, the version 2 was 95%, probably this is because the test doesn't
cover the "UNION" case.

I still wonder why you don't want to cover the 100% if it's just a small
changes in the test, but for now, this does third version it does
increase the coverage by 42%.

Regards!
--
Jonathan Gonzalez V.
EDB
https://www.enterprisedb.com

Hi,

I've reviewed the new version and now the coverage with this version is
92%, the version 2 was 95%, probably this is because the test doesn't
cover the "UNION" case.

I still wonder why you don't want to cover the 100% if it's just a small
changes in the test, but for now, this does third version it does
increase the coverage by 42%.

There is a line of code:

```
j->cross ? " CROSS " : " UNION "
```

The difference in coverage you are referring to is that in one case we
cover both if-else branches while in another only one of them. In my
opinion there is little practical value in covering both of them.
Executing one line of the source code at least once is enough,
regardless of what number `lcov` reports. This is always a compromise
between the coverage, the ease of maintaining it and the speed of our
tests on the buildfarm.

If anyone else believes that we should do 92% -> 95% here I will
submit the corrected patch though.

--
Best regards,
Aleksander Alekseev

Hi Aleksander,

I tested v3 of the patch on the current PostgreSQL master.
The patch applied cleanly, and make check completed successfully with
all 245 regression tests passing.
I also tested the existing JSON_TABLE functionality manually using
SELECT, EXPLAIN, and EXPLAIN VERBOSE, and everything worked as
expected.
I didn't run into any issues while testing this version. The patch
looks good to me.

Regards,
solai