> On Jan 23, 2020, at 4:27 PM, Andrew Dunstan <andrew.duns...@2ndquadrant.com>
> wrote:
>
> On Fri, Jan 24, 2020 at 7:35 AM Mark Dilger
> <mark.dil...@enterprisedb.com> wrote:
>>
>>
>>> On Jan 22, 2020, at 7:00 PM, Mark Dilger <mark.dil...@enterprisedb.com>
>>> wrote:
>>>
>>> I have this done in my local repo to the point that I can build frontend
>>> tools against the json parser that is now in src/common and also run all
>>> the check-world tests without failure. I’m planning to post my work soon,
>>> possibly tonight if I don’t run out of time, but more likely tomorrow.
>>
>> Ok, I finished merging with Robert’s patches. The attached follow his
>> numbering, with my patches intended to by applied after his.
>>
>> I tried not to change his work too much, but I did a bit of refactoring in
>> 0010, as explained in the commit comment.
>>
>> 0011 is just for verifying the linking works ok and the json parser can be
>> invoked from a frontend tool without error — I don’t really see the point in
>> committing it.
>>
>> I ran some benchmarks for json parsing in the backend both before and after
>> these patches, with very slight changes in runtime. The setup for the
>> benchmark creates an unlogged table with a single text column and loads rows
>> of json formatted text:
>>
>> CREATE UNLOGGED TABLE benchmark (
>> j text
>> );
>> COPY benchmark (j) FROM '/Users/mark.dilger/bench/json.csv’;
>>
>>
>> FYI:
>>
>> wc ~/bench/json.csv
>> 107 34465023 503364244 /Users/mark.dilger/bench/json.csv
>>
>> The benchmark itself casts the text column to jsonb, as follows:
>>
>> SELECT jsonb_typeof(j::jsonb) typ, COUNT(*) FROM benchmark GROUP BY typ;
>>
>> In summary, the times are:
>>
>> pristine patched
>> ————— —————
>> 11.985 12.237
>> 12.200 11.992
>> 11.691 11.896
>> 11.847 11.833
>> 11.722 11.936
>>
>
> OK, nothing noticeable there.
>
> "accept" is a common utility I've used in the past with parsers of
> this kind, but inlining it seems perfectly reasonable.
>
> I've reviewed these patches and Robert's, and they seem basically good to me.
Thanks for the review!
> But I don't think src/bin is the right place for the test program. I
> assume we're not going to ship this program, so it really belongs in
> src/test somewhere, I think. It should also have a TAP test.
Ok, I’ll go do that; thanks for the suggestion.
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
