| From: | Aleksander Alekseev <aleksander(at)tigerdata(dot)com> |
|---|---|
| To: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Cc: | "Jonathan Gonzalez V(dot)" <jonathan(dot)abdiel(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, solai v <solai(dot)cdac(at)gmail(dot)com> |
| Subject: | Re: [PATCH] Cover get_json_table_plan() with tests |
| Date: | 2026-07-22 09:06:23 |
| Message-ID: | CAJ7c6TN4S7o59E1jaMJTFOTuVws+ibRorM3cokXy5BrcjcctHQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Fujii Masao | 2026-07-22 09:15:15 | Re: Don't use pq_putmessage in socket comm function |
| Previous Message | Antonin Houska | 2026-07-22 08:41:10 | Re: Unexpected behavior after OOM errors |