[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