Re: WIP Incremental JSON Parser
Andrew Dunstan <andrew@dunslane.net>
From: Andrew Dunstan <andrew@dunslane.net>
To: Jacob Champion <jacob.champion@enterprisedb.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Peter Smith <smithpb2250@gmail.com>,
PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-02-22T23:43:01Z
Lists: pgsql-hackers
On 2024-02-22 Th 15:29, Jacob Champion wrote: > On Thu, Feb 22, 2024 at 1:38 AM Andrew Dunstan <andrew@dunslane.net> wrote: >> Patch 5 in this series fixes those issues and >> adjusts most of the tests to add some trailing junk to the pieces of >> json, so we can be sure that this is done right. > This fixes the test failure for me, thanks! I've attached my current > mesonification diff, which just adds test_json_parser to the suite. It > relies on the PATH that's set up, which appears to include the build > directory for both VPATH builds and Meson. OK, thanks, will add this in the next version. > > Are there plans to fill out the test suite more? Since we should be > able to control all the initial conditions, it'd be good to get fairly > comprehensive coverage of the new code. Well, it's tested (as we know) by the backup manifest tests. During development, I tested by making the regular parser use the non-recursive parser (see FORCE_JSON_PSTACK). That doesn't test the incremental piece of it, but it does check that the rest of it is doing the right thing. We could probably extend the incremental test by making it output a stream of tokens and making sure that was correct. > As an aside, I find the behavior of need_escapes=false to be > completely counterintuitive. I know the previous code did this, but it > seems like the opposite of "provides unescaped strings" should be > "provides raw strings", not "all strings are now NULL". Yes, we could possibly call it "need_strings" or something like that. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Commits
-
Post review fixes for test_json_parser test module
- ba3e6e2bca97 17.0 landed
-
Shrink test file for test_json_parser module
- b8a7bfa33324 17.0 landed
-
Add support for incrementally parsing backup manifests
- ea7b4e9a2a7c 17.0 landed
-
Introduce a non-recursive JSON parser
- 3311ea86edc7 17.0 landed
-
Use incremental parsing of backup manifests.
- 222e11a10ae9 17.0 landed