Re: WIP Incremental JSON Parser - Mailing list pgsql-hackers

From Andrew Dunstan
Subject Re: WIP Incremental JSON Parser
Date
Msg-id ac935d31-bc11-457d-be49-3f1372d8c7aa@dunslane.net
Whole thread Raw
In response to Re: WIP Incremental JSON Parser  (Michael Paquier <michael@paquier.xyz>)
Responses Re: WIP Incremental JSON Parser
List pgsql-hackers


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 commits.

Anyway, I have noticed another thing in the surroundings that's
annoying.  003 has this logic:
use File::Temp qw(tempfile);
[...]
my ($fh, $fname) = tempfile();

print $fh $stdout,"\n";

close($fh);

This creates a temporary file in /tmp/ that remains around, slowing
bloating the temporary file space on a node while leaving around some
data. 


My bad, I should have used the UNLINK option like in the other tests.



 Why using File::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 possibility, but I think the above is the simplest.



Per [1], escape_json() has no coverage outside its default path.  Is
that intended?


Not particularly. I'll add some stuff to get complete coverage.



Documenting all these test files with a few comments would be welcome,
as well, with some copyright notices...


ok


        json_file = fopen(testfile, "r");        fstat(fileno(json_file), &statbuf);        bytes_left = statbuf.st_size;

No checks on failure of fstat() here?


ok will fix


        json_file = fopen(argv[2], "r");

Second one in test_json_parser_perf.c, with more stuff for fread().


ok will fix

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

pgsql-hackers by date:

Previous
From: Jelte Fennema-Nio
Date:
Subject: Re: Support a wildcard in backtrace_functions
Next
From: Jelte Fennema-Nio
Date:
Subject: Re: Support a wildcard in backtrace_functions