Re: WIP Incremental JSON Parser

On 2024-04-24 We 04:56, Michael Paquier wrote: On Fri, Apr 19, 2024 at 02:04:40PM -0400, Robert Haas wrote: Yeah, I think this patch invented a new solution to a problem that we've solved in a different way everywhere else. I think we should change it to match what we do in general. As of ba3

Re: WIP Incremental JSON Parser

On Fri, Apr 19, 2024 at 02:04:40PM -0400, Robert Haas wrote: > Yeah, I think this patch invented a new solution to a problem that > we've solved in a different way everywhere else. I think we should > change it to match what we do in general. As of ba3e6e2bca97, did you notice that test_json_parse

Re: WIP Incremental JSON Parser

On Fri, Apr 19, 2024 at 1:35 AM Michael Paquier wrote: > Are you sure that relying on Temp::File is a good thing overall? The > current temporary file knowledge is encapsulated within Utils.pm, with > files removed or kept depending on PG_TEST_NOCLEAN. So it would be > just more consistent to re

Re: WIP Incremental JSON Parser

On Thu, Apr 18, 2024 at 06:03:45AM -0400, Andrew Dunstan wrote: > On 2024-04-18 Th 02:04, Michael Paquier wrote: >> Why usingFile::Temp::tempfile here? Couldn't you just use a >> file in a PostgreSQL::Test::Utils::tempdir() that would be cleaned up >> once the test finishes? > > That's another

Re: WIP Incremental JSON Parser

On 2024-04-18 Th 02:04, Michael Paquier wrote: On Wed, Apr 10, 2024 at 07:47:38AM -0400, Andrew Dunstan wrote: Here's v2 of the cleanup patch 4, that fixes some more typos kindly pointed out to me by Alexander Lakhin. I can see that this has been applied as of 42fa4b660143 with some extra comm

Re: WIP Incremental JSON Parser

On Wed, Apr 10, 2024 at 07:47:38AM -0400, Andrew Dunstan wrote: > Here's v2 of the cleanup patch 4, that fixes some more typos kindly pointed > out to me by Alexander Lakhin. I can see that this has been applied as of 42fa4b660143 with some extra commits. Anyway, I have noticed another thing in t

Re: WIP Incremental JSON Parser

On Tue, Apr 09, 2024 at 09:26:49AM -0700, Jacob Champion wrote: > On Tue, Apr 9, 2024 at 7:30 AM Andrew Dunstan wrote: >> I think Michael's point was that if we carry the code we should test we >> can run it. The other possibility would be just to remove it. I can see >> arguments for both. > > H

Re: WIP Incremental JSON Parser

On 2024-04-09 Tu 15:42, Andrew Dunstan wrote: On 2024-04-09 Tu 07:54, Andrew Dunstan wrote: On 2024-04-09 Tu 01:23, Michael Paquier wrote: On Tue, Apr 09, 2024 at 09:48:18AM +0900, Michael Paquier wrote: There is no direct check on test_json_parser_perf.c, either, only a custom rule in the

Re: WIP Incremental JSON Parser

On Mon, Apr 8, 2024 at 10:24 PM Michael Paquier wrote: > At the end, having a way to generate JSON blobs randomly to test this > stuff would be more appealing For the record, I'm working on an LLVM fuzzer target for the JSON parser. I think that would be a lot more useful than anything we can han

Re: WIP Incremental JSON Parser

On Tue, Apr 9, 2024 at 7:30 AM Andrew Dunstan wrote: > I think Michael's point was that if we carry the code we should test we > can run it. The other possibility would be just to remove it. I can see > arguments for both. Hm. If it's not acceptable to carry this (as a worse-is-better smoke test)

Re: WIP Incremental JSON Parser

On 2024-04-09 Tu 09:45, Jacob Champion wrote: On Tue, Apr 9, 2024 at 4:54 AM Andrew Dunstan wrote: On 2024-04-09 Tu 01:23, Michael Paquier wrote: There is no direct check on test_json_parser_perf.c, either, only a custom rule in the Makefile without specifying something for meson. So it looks

Re: WIP Incremental JSON Parser

On Tue, Apr 9, 2024 at 4:54 AM Andrew Dunstan wrote: > On 2024-04-09 Tu 01:23, Michael Paquier wrote: > There is no direct check on test_json_parser_perf.c, either, only a > custom rule in the Makefile without specifying something for meson. > So it looks like you could do short execution check in

Re: WIP Incremental JSON Parser

On 2024-04-09 Tu 01:23, Michael Paquier wrote: On Tue, Apr 09, 2024 at 09:48:18AM +0900, Michael Paquier wrote: There is no direct check on test_json_parser_perf.c, either, only a custom rule in the Makefile without specifying something for meson. So it looks like you could do short execution c

Re: WIP Incremental JSON Parser

On Tue, Apr 09, 2024 at 09:48:18AM +0900, Michael Paquier wrote: > There is no direct check on test_json_parser_perf.c, either, only a > custom rule in the Makefile without specifying something for meson. > So it looks like you could do short execution check in a TAP test, at > least. While readin

Re: WIP Incremental JSON Parser

On Mon, Apr 08, 2024 at 05:42:35PM -0400, Andrew Dunstan wrote: > Arguably the fact that it points nowhere is a good thing. But feel free to > replace it with something else. It doesn't have to be URLs at all. That > happened simply because it was easy to extract from a very large piece of > JSON I

Re: WIP Incremental JSON Parser

On 2024-04-08 Mo 09:29, Andrew Dunstan wrote: On 2024-04-07 Su 20:58, Tom Lane wrote: Coverity complained that this patch leaks memory: /srv/coverity/git/pgsql-git/postgresql/src/bin/pg_combinebackup/load_manifest.c: 212 in load_backup_manifest() 206 bytes_left -= rc; 207   

Re: WIP Incremental JSON Parser

On 2024-04-08 Mo 14:24, Jacob Champion wrote: Michael pointed out over at [1] that the new tiny.json is pretty inscrutable given its size, and I have to agree. Attached is a patch to pare it down 98% or so. I think people wanting to run the performance comparisons will need to come up with thei

Re: WIP Incremental JSON Parser

On 2024-04-07 Su 20:58, Tom Lane wrote: Coverity complained that this patch leaks memory: /srv/coverity/git/pgsql-git/postgresql/src/bin/pg_combinebackup/load_manifest.c: 212 in load_backup_manifest() 206 bytes_left -= rc; 207 json_parse

Re: WIP Incremental JSON Parser

Coverity complained that this patch leaks memory: /srv/coverity/git/pgsql-git/postgresql/src/bin/pg_combinebackup/load_manifest.c: 212 in load_backup_manifest() 206 bytes_left -= rc; 207 json_parse_manifest_incremental_chunk( 208

Re: WIP Incremental JSON Parser

On 2024-04-05 Fr 11:43, Nathan Bossart wrote: On Fri, Apr 05, 2024 at 10:15:45AM -0400, Andrew Dunstan wrote: On 2024-04-04 Th 15:54, Nathan Bossart wrote: On Thu, Apr 04, 2024 at 03:31:12PM -0400, Andrew Dunstan wrote: Does the attached patch fix it for you? It clears up 2 of the 3 warning

Re: WIP Incremental JSON Parser

On Fri, Apr 05, 2024 at 10:15:45AM -0400, Andrew Dunstan wrote: > On 2024-04-04 Th 15:54, Nathan Bossart wrote: >> On Thu, Apr 04, 2024 at 03:31:12PM -0400, Andrew Dunstan wrote: >> > Does the attached patch fix it for you? >> It clears up 2 of the 3 warnings for me: >> >> ../postgresql/src/common

Re: WIP Incremental JSON Parser

On 2024-04-04 Th 15:54, Nathan Bossart wrote: On Thu, Apr 04, 2024 at 03:31:12PM -0400, Andrew Dunstan wrote: Does the attached patch fix it for you? It clears up 2 of the 3 warnings for me: ../postgresql/src/common/jsonapi.c: In function ‘IsValidJsonNumber’: ../postgresql/src/common/jsonapi.

Re: WIP Incremental JSON Parser

On Thu, 4 Apr 2024 at 23:52, Jelte Fennema-Nio wrote: > Fair enough, I guess I'll change my invocation to include the ninja > "test" target too: > ninja -C build test install-quiet && meson test -C build Actually that doesn't do what I want either because that actually runs the tests too... And I

Re: WIP Incremental JSON Parser

On Thu, 4 Apr 2024 at 23:34, Jacob Champion wrote: > > On Thu, Apr 4, 2024 at 1:42 PM Jelte Fennema-Nio wrote: > > Maybe that's something worth addressing too. I expected that > > install/install-quiet was a strict superset of a plain ninja > > invocation. > > Maybe that's the intent, but I hope

Re: WIP Incremental JSON Parser

On Thu, Apr 4, 2024 at 1:42 PM Jelte Fennema-Nio wrote: > Maybe that's something worth addressing too. I expected that > install/install-quiet was a strict superset of a plain ninja > invocation. Maybe that's the intent, but I hope not, because I don't see any reason for `ninja install` to worry

Re: WIP Incremental JSON Parser

On Thu, 4 Apr 2024 at 20:48, Jacob Champion wrote: > > On Thu, Apr 4, 2024 at 11:12 AM Jacob Champion > wrote: > > What's in the `...`? I wouldn't expect to find the test binary in your > > tmp_install. > > Oh, I wonder if this is just a build dependency thing? I typically run > a bare `ninja` ri

Re: WIP Incremental JSON Parser

On Thu, Apr 04, 2024 at 03:31:12PM -0400, Andrew Dunstan wrote: > Does the attached patch fix it for you? It clears up 2 of the 3 warnings for me: ../postgresql/src/common/jsonapi.c: In function ‘IsValidJsonNumber’: ../postgresql/src/common/jsonapi.c:2018:30: error: ‘dummy_lex.inc_state’ may be

Re: WIP Incremental JSON Parser

On 2024-04-04 Th 14:06, Nathan Bossart wrote: Apologies for piling on, but my compiler (gcc 9.4.0) is unhappy: ../postgresql/src/common/jsonapi.c: In function ‘IsValidJsonNumber’: ../postgresql/src/common/jsonapi.c:2016:30: error: ‘dummy_lex.inc_state’ may be used uninitialized in this functio

Re: WIP Incremental JSON Parser

On Thu, Apr 4, 2024 at 11:06 AM Nathan Bossart wrote: > ../postgresql/src/common/jsonapi.c: In function ‘IsValidJsonNumber’: > ../postgresql/src/common/jsonapi.c:2016:30: error: ‘dummy_lex.inc_state’ may > be used uninitialized in this function [-Werror=maybe-uninitialized] > 2016 | if (lex->in

Re: WIP Incremental JSON Parser

On Thu, Apr 4, 2024 at 11:12 AM Jacob Champion wrote: > What's in the `...`? I wouldn't expect to find the test binary in your > tmp_install. Oh, I wonder if this is just a build dependency thing? I typically run a bare `ninja` right before testing, because I think most of those dependencies are

Re: WIP Incremental JSON Parser

On Thu, Apr 4, 2024 at 10:17 AM Jelte Fennema-Nio wrote: > Command 'test_json_parser_incremental' not found in > /home/jelte/opensource/postgres/build/tmp_install//home/jelte/.pgenv/pgsql-17beta9/bin, > ... I can't reproduce this locally... What's in the `...`? I wouldn't expect to find the test

Re: WIP Incremental JSON Parser

Apologies for piling on, but my compiler (gcc 9.4.0) is unhappy: ../postgresql/src/common/jsonapi.c: In function ‘IsValidJsonNumber’: ../postgresql/src/common/jsonapi.c:2016:30: error: ‘dummy_lex.inc_state’ may be used uninitialized in this function [-Werror=maybe-uninitialized] 2016 | if (lex-

Re: WIP Incremental JSON Parser

On Thu, 4 Apr 2024 at 18:13, Andrew Dunstan wrote: > Argh. You get out of the habit when you're running with meson :-( I'm having some issues with meson too actually. "meson test -C build" is now failing with this for me: Command 'test_json_parser_incremental' not found in /home/jelte/opensource

Re: WIP Incremental JSON Parser

On 2024-04-04 Th 12:04, Tom Lane wrote: Oh, more problems: after running check-world in a non-VPATH build, there are droppings all over my tree: $ git status On branch master Your branch is up to date with 'origin/master'. Untracked files: (use "git add ..." to include in what will be comm

Re: WIP Incremental JSON Parser

Oh, more problems: after running check-world in a non-VPATH build, there are droppings all over my tree: $ git status On branch master Your branch is up to date with 'origin/master'. Untracked files: (use "git add ..." to include in what will be committed) src/interfaces/ecpg/test/sql/s

Re: WIP Incremental JSON Parser

On 2024-04-04 Th 11:16, Tom Lane wrote: I wrote: I think you just need to follow the standard pattern: Yeah, the attached is enough to silence it for me. Thanks, that's what I came up with too (after converting my setup to use clang) (But personally I'd add comments saying that the ty

Re: WIP Incremental JSON Parser

I wrote: > I think you just need to follow the standard pattern: Yeah, the attached is enough to silence it for me. (But personally I'd add comments saying that the typedef appears in thus-and-such header file; see examples in our tree.) regards, tom lane diff --git a/src

Re: WIP Incremental JSON Parser

Andrew Dunstan writes: > On 2024-04-04 Th 10:26, Tom Lane wrote: >> My animals don't like this [1][2]: > Darn it. Ok, will try to fix. I think you just need to follow the standard pattern: typedef struct foo foo; ... foo* is ok to use here ... struct foo { ... fields };

Re: WIP Incremental JSON Parser

On 2024-04-04 Th 10:26, Tom Lane wrote: Andrew Dunstan writes: pushed. My animals don't like this [1][2]: parse_manifest.c:99:3: error: redefinition of typedef 'JsonManifestParseIncrementalState' is a C11 feature [-Werror,-Wtypedef-redefinition] } JsonManifestParseIncrementalState; ^ .

Re: WIP Incremental JSON Parser

Andrew Dunstan writes: > pushed. My animals don't like this [1][2]: parse_manifest.c:99:3: error: redefinition of typedef 'JsonManifestParseIncrementalState' is a C11 feature [-Werror,-Wtypedef-redefinition] } JsonManifestParseIncrementalState; ^ ../../src/include/common/parse_manifest.h:23:

Re: WIP Incremental JSON Parser

On 2024-04-02 Tu 17:12, Andrew Dunstan wrote: On 2024-04-02 Tu 15:38, Jacob Champion wrote: On Mon, Apr 1, 2024 at 4:53 PM Andrew Dunstan wrote: Anyway, here are new patches. I've rolled the new semantic test into the first patch. Looks good! I've marked RfC. Thanks! I appreciate all t

Re: WIP Incremental JSON Parser

On 2024-04-02 Tu 15:38, Jacob Champion wrote: On Mon, Apr 1, 2024 at 4:53 PM Andrew Dunstan wrote: Anyway, here are new patches. I've rolled the new semantic test into the first patch. Looks good! I've marked RfC. Thanks! I appreciate all the work you've done on this. I will give it one

Re: WIP Incremental JSON Parser

On Mon, Apr 1, 2024 at 4:53 PM Andrew Dunstan wrote: > Anyway, here are new patches. I've rolled the new semantic test into the > first patch. Looks good! I've marked RfC. > json_lex() is not really a very hot piece of code. Sure, but I figure if someone is trying to get the performance of the

Re: WIP Incremental JSON Parser

On Fri, Mar 29, 2024 at 9:42 AM Andrew Dunstan wrote: > Here's a new set of patches, with I think everything except the error case > Asserts attended to. There's a change to add semantic processing to the test > suite in patch 4, but I'd fold that into patch 1 when committing. Thanks! 0004 did

Re: WIP Incremental JSON Parser

On 2024-03-26 Tu 17:52, Jacob Champion wrote: On Mon, Mar 25, 2024 at 3:14 PM Jacob Champion wrote: - add Assert calls in impossible error cases [2] To expand on this one, I think these parts of the code (marked with `<- here`) are impossible to reach: switch (top) { case JSON_TOKEN_S

Re: WIP Incremental JSON Parser

On Mon, Mar 25, 2024 at 3:14 PM Jacob Champion wrote: > - add Assert calls in impossible error cases [2] To expand on this one, I think these parts of the code (marked with `<- here`) are impossible to reach: > switch (top) > { > case JSON_TOKEN_STRING: > if (next_prediction(pstack)

Re: WIP Incremental JSON Parser

On Mon, Mar 25, 2024 at 4:24 PM Andrew Dunstan wrote: > OK, so we invent a new error code and have the parser return that if the > stack depth gets too big? Yeah, that seems reasonable. I'd potentially be able to build on that via OAuth for next cycle, too, since that client needs to limit its

Re: WIP Incremental JSON Parser

On Mon, Mar 25, 2024 at 7:12 PM Jacob Champion < jacob.champ...@enterprisedb.com> wrote: > On Mon, Mar 25, 2024 at 4:02 PM Andrew Dunstan > wrote: > > Well, what's the alternative? The current parser doesn't check stack > depth in frontend code. Presumably it too will eventually just run out of >

Re: WIP Incremental JSON Parser

On Mon, Mar 25, 2024 at 4:12 PM Jacob Champion wrote: > Stack size should be pretty limited, at least on the platforms I'm > familiar with. So yeah, the recursive descent will segfault pretty > quickly, but it won't repalloc() an unbounded amount of heap space. > The alternative would just be to g

Re: WIP Incremental JSON Parser

On Mon, Mar 25, 2024 at 4:02 PM Andrew Dunstan wrote: > Well, what's the alternative? The current parser doesn't check stack depth in > frontend code. Presumably it too will eventually just run out of memory, > possibly rather sooner as the stack frames could be more expensive than the > incre

Re: WIP Incremental JSON Parser

On Mon, Mar 25, 2024 at 6:15 PM Jacob Champion < jacob.champ...@enterprisedb.com> wrote: > On Wed, Mar 20, 2024 at 11:56 PM Andrew Dunstan > wrote: > > Thanks, included that and attended to the other issues we discussed. I > think this is pretty close now. > > Okay, looking over the thread, there

Re: WIP Incremental JSON Parser

On Wed, Mar 20, 2024 at 11:56 PM Andrew Dunstan wrote: > Thanks, included that and attended to the other issues we discussed. I think > this is pretty close now. Okay, looking over the thread, there are the following open items: - extend the incremental test in order to exercise the semantic cal

Re: WIP Incremental JSON Parser

This new return path... > + if (!tok_done) > + { > + if (lex->inc_state->is_last_chunk) > + return JSON_INVALID_TOKEN; ...also needs to set the token pointers. See one approach in the attached diff, which additionally asserts that we've consumed the entire chun

Re: WIP Incremental JSON Parser

On Tue, Mar 19, 2024 at 3:07 PM Andrew Dunstan wrote: > On Mon, Mar 18, 2024 at 3:35 PM Jacob Champion > wrote: >> With the incremental parser, I think prev_token_terminator is not >> likely to be safe to use except in very specific circumstances, since >> it could be pointing into a stale chunk

Re: WIP Incremental JSON Parser

On Tue, Mar 19, 2024 at 6:07 PM Andrew Dunstan wrote: > > > > > It also removes the frontend exits I had. In the case of stack depth, we > follow the example of the RD parser and only check stack depth for backend > code. In the case of the check that the lexer is set up for incremental > parsing

Re: WIP Incremental JSON Parser

On Mon, Mar 18, 2024 at 3:32 AM Andrew Dunstan wrote: > Not very easily. But I think and hope I've fixed the issue you've identified > above about returning before lex->token_start is properly set. > > Attached is a new set of patches that does that and is updated for the > json_errdetaiil() ch

Re: WIP Incremental JSON Parser

I've been poking at the partial token logic. The json_errdetail() bug mentioned upthread (e.g. for an invalid input `[12zz]` and small chunk size) seems to be due to the disconnection between the "main" lex instance and the dummy_lex that's created from it. The dummy_lex contains all the informatio

Re: WIP Incremental JSON Parser

On Sun, Mar 10, 2024 at 11:43 PM Andrew Dunstan wrote: > I haven't managed to reproduce this. But I'm including some tests for it. If I remove the newline from the end of the new tests: > @@ -25,7 +25,7 @@ for (my $size = 64; $size > 0; $size--) > foreach my $test_string (@inlinetests) > { >

Re: WIP Incremental JSON Parser

On 2024-03-07 Th 10:28, Jacob Champion wrote: Some more observations as I make my way through the patch: In src/common/jsonapi.c, +#define JSON_NUM_NONTERMINALS 6 Should this be 5 now? Yep. + res = pg_parse_json_incremental(&(incstate->lex), &(incstate->sem), +

Re: WIP Incremental JSON Parser

Some more observations as I make my way through the patch: In src/common/jsonapi.c, > +#define JSON_NUM_NONTERMINALS 6 Should this be 5 now? > + res = pg_parse_json_incremental(&(incstate->lex), &(incstate->sem), > + >

Re: WIP Incremental JSON Parser

On Mon, Feb 26, 2024 at 9:20 PM Andrew Dunstan wrote: > The good news is that the parser is doing fine - this issue was due to a > thinko on my part in the test program that got triggered by the input > file size being an exact multiple of the chunk size. I'll have a new > patch set later this wee

Re: WIP Incremental JSON Parser

On 2024-02-26 Mo 19:20, Andrew Dunstan wrote: On 2024-02-26 Mo 10:10, Jacob Champion wrote: On Mon, Feb 26, 2024 at 7:08 AM Jacob Champion wrote: As a brute force example of the latter, with the attached diff I get test failures at chunk sizes 1, 2, 3, 4, 6, and 12. But this time with the

Re: WIP Incremental JSON Parser

On 2024-02-26 Mo 10:10, Jacob Champion wrote: On Mon, Feb 26, 2024 at 7:08 AM Jacob Champion wrote: As a brute force example of the latter, with the attached diff I get test failures at chunk sizes 1, 2, 3, 4, 6, and 12. But this time with the diff. Ouch. I'll check it out. Thanks! chee

Re: WIP Incremental JSON Parser

On Mon, Feb 26, 2024 at 7:08 AM Jacob Champion wrote: > As a brute force example of the latter, with the attached diff I get > test failures at chunk sizes 1, 2, 3, 4, 6, and 12. But this time with the diff. --Jacob diff --git a/src/test/modules/test_json_parser/t/001_test_json_parser_increment

Re: WIP Incremental JSON Parser

On Thu, Feb 22, 2024 at 3:43 PM Andrew Dunstan wrote: > > 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 mani

Re: WIP Incremental JSON Parser

On 2024-02-22 Th 15:29, Jacob Champion wrote: On Thu, Feb 22, 2024 at 1:38 AM Andrew Dunstan 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

Re: WIP Incremental JSON Parser

On Thu, Feb 22, 2024 at 1:38 AM Andrew Dunstan 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 meso

Re: WIP Incremental JSON Parser

On Wed, Feb 21, 2024 at 6:50 AM Jacob Champion wrote: > On Tue, Feb 20, 2024 at 9:32 PM Andrew Dunstan wrote: > > *sigh* That's weird. I wonder why you can reproduce it and I can't. Can > > you give me details of the build? OS, compiler, path to source, build > > setup etc.? Anything that might b

Re: WIP Incremental JSON Parser

On Tue, Feb 20, 2024 at 9:32 PM Andrew Dunstan wrote: > *sigh* That's weird. I wonder why you can reproduce it and I can't. Can > you give me details of the build? OS, compiler, path to source, build > setup etc.? Anything that might be remotely relevant. Sure: - guest VM running in UTM (QEMU 7.2

Re: WIP Incremental JSON Parser

On 2024-02-20 Tu 19:53, Jacob Champion wrote: On Tue, Feb 20, 2024 at 2:10 PM Andrew Dunstan wrote: Well, that didn't help a lot, but meanwhile the CFBot seems to have decided in the last few days that it's now happy, so full steam aead! ;-) I haven't been able to track down the root cause y

Re: WIP Incremental JSON Parser

On Tue, Feb 20, 2024 at 2:10 PM Andrew Dunstan wrote: > Well, that didn't help a lot, but meanwhile the CFBot seems to have > decided in the last few days that it's now happy, so full steam aead! ;-) I haven't been able to track down the root cause yet, but I am able to reproduce the failure cons

Re: WIP Incremental JSON Parser

On 2024-01-26 Fr 12:15, Andrew Dunstan wrote: On 2024-01-24 We 13:08, Robert Haas wrote: Maybe you should adjust your patch to dump the manifests into the log file with note(). Then when cfbot runs on it you can see exactly what the raw file looks like. Although I wonder if it's possible tha

Re: WIP Incremental JSON Parser

On Wed, Jan 24, 2024 at 10:04 AM Andrew Dunstan wrote: > The cfbot reports an error on a 32 bit build > : > > # Running: pg_basebackup -D > /tmp/cirrus

Re: WIP Incremental JSON Parser

On 2024-01-22 Mo 21:02, Andrew Dunstan wrote: On 2024-01-22 Mo 18:01, Andrew Dunstan wrote: On 2024-01-22 Mo 14:16, Andrew Dunstan wrote: On 2024-01-22 Mo 01:29, Peter Smith wrote: 2024-01 Commitfest. Hi, This patch has a CF status of "Needs Review" [1], but it seems there were CFbot test

Re: WIP Incremental JSON Parser

2024-01 Commitfest. Hi, This patch has a CF status of "Needs Review" [1], but it seems there were CFbot test failures last time it was run [2]. Please have a look and post an updated version if necessary. == [1] https://commitfest.postgresql.org/46/4725/ [2] https://cirrus-ci.com/github/postg

Re: WIP Incremental JSON Parser

On 2024-01-09 Tu 13:46, Jacob Champion wrote: On Tue, Dec 26, 2023 at 8:49 AM Andrew Dunstan wrote: Quite a long time ago Robert asked me about the possibility of an incremental JSON parser. I wrote one, and I've tweaked it a bit, but the performance is significantly worse that that of the cu

Re: WIP Incremental JSON Parser

On Tue, Dec 26, 2023 at 8:49 AM Andrew Dunstan wrote: > Quite a long time ago Robert asked me about the possibility of an > incremental JSON parser. I wrote one, and I've tweaked it a bit, but the > performance is significantly worse that that of the current Recursive > Descent parser. The predic

Re: WIP Incremental JSON Parser

On Wed, Jan 3, 2024 at 6:36 PM Nico Williams wrote: > On Tue, Jan 02, 2024 at 10:14:16AM -0500, Robert Haas wrote: > > It seems like a pretty significant savings no matter what. Suppose the > > backup_manifest file is 2GB, and instead of creating a 2GB buffer, you > > create an 1MB buffer and feed

Re: WIP Incremental JSON Parser

On Tue, Jan 02, 2024 at 10:14:16AM -0500, Robert Haas wrote: > It seems like a pretty significant savings no matter what. Suppose the > backup_manifest file is 2GB, and instead of creating a 2GB buffer, you > create an 1MB buffer and feed the data to the parser in 1MB chunks. > Well, that saves 2GB

Re: WIP Incremental JSON Parser

On 2024-01-03 We 10:12, Robert Haas wrote: On Wed, Jan 3, 2024 at 9:59 AM Andrew Dunstan wrote: Say we have a document with an array 1m objects, each with a field called "color". As it stands we'll allocate space for that field name 1m times. Using a hash table we'd allocated space for it onc

Re: WIP Incremental JSON Parser

On Wed, Jan 3, 2024 at 9:59 AM Andrew Dunstan wrote: > Say we have a document with an array 1m objects, each with a field > called "color". As it stands we'll allocate space for that field name 1m > times. Using a hash table we'd allocated space for it once. And > allocating the memory isn't free,

Re: WIP Incremental JSON Parser

On 2024-01-03 We 08:45, Robert Haas wrote: On Wed, Jan 3, 2024 at 6:57 AM Andrew Dunstan wrote: Yeah. One idea I had yesterday was to stash the field names, which in large JSON docs tent to be pretty repetitive, in a hash table instead of pstrduping each instance. The name would be valid unti

Re: WIP Incremental JSON Parser

On Wed, Jan 3, 2024 at 6:57 AM Andrew Dunstan wrote: > Yeah. One idea I had yesterday was to stash the field names, which in > large JSON docs tent to be pretty repetitive, in a hash table instead of > pstrduping each instance. The name would be valid until the end of the > parse, and would only n

Re: WIP Incremental JSON Parser

On 2024-01-02 Tu 10:14, Robert Haas wrote: On Tue, Dec 26, 2023 at 11:49 AM Andrew Dunstan wrote: Quite a long time ago Robert asked me about the possibility of an incremental JSON parser. I wrote one, and I've tweaked it a bit, but the performance is significantly worse that that of the curr

Re: WIP Incremental JSON Parser

On Tue, Dec 26, 2023 at 11:49 AM Andrew Dunstan wrote: > Quite a long time ago Robert asked me about the possibility of an > incremental JSON parser. I wrote one, and I've tweaked it a bit, but the > performance is significantly worse that that of the current Recursive > Descent parser. Neverthele