From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Nathan Bossart <nathandbossart(at)gmail(dot)com>, Jelte Fennema-Nio <postgres(at)jeltef(dot)nl>, Amit Langote <amitlangote09(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: WIP Incremental JSON Parser |
Date: | 2024-04-18 10:03:45 |
Message-ID: | ac935d31-bc11-457d-be49-3f1372d8c7aa@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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:
> useFile::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 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 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
From | Date | Subject | |
---|---|---|---|
Next Message | Jelte Fennema-Nio | 2024-04-18 10:21:56 | Re: Support a wildcard in backtrace_functions |
Previous Message | Jelte Fennema-Nio | 2024-04-18 09:54:23 | Re: Support a wildcard in backtrace_functions |