> On Jan 23, 2020, at 4:27 PM, Andrew Dunstan <andrew.dunstan@2ndquadrant.com> wrote:
>
> On Fri, Jan 24, 2020 at 7:35 AM Mark Dilger
> <mark.dilger@enterprisedb.com> wrote:
>>
>>
>>> On Jan 22, 2020, at 7:00 PM, Mark Dilger <mark.dilger@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
insrc/common and also run all the check-world tests without failure. I’m planning to post my work soon, possibly
tonightif 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
appliedafter 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
inruntime. The setup for the benchmark creates an unlogged table with a single text column and loads rows of json
formattedtext:
>>
>> 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
