Re: [PATCH] few fts functions for jsonb

From: Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>
To: Dmitry Dolgov <9erthalion6(at)gmail(dot)com>
Cc: Oleg Bartunov <obartunov(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] few fts functions for jsonb
Date: 2017-03-29 16:28:08
Message-ID: CAA8=A7_mGqeFmrkKLqV3VdaEy=QRY+PFg_n+3+KBoyRybXE=kw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On 26 March 2017 at 17:57, Dmitry Dolgov <9erthalion6(at)gmail(dot)com> wrote:
>> I'm not through looking at this. However, here are a few preliminary
>> comments
>
> I've attached new versions of the patches with improvements related to these
> commentaries.

These patches seem fundamentally OK. But I'm still not happy with the
naming etc.

I think the header changes would probably be better placed in
jsonapi.h or in a new header file.

And the names still seem too general to me. e.g. transform_json_values
should probably be transform_json_string_values, and the static
support functions should be renamed to match. Also the
JsonIterateAction and JsonTransformAction funtion typedefs should
probably be renamed to match.

I'm not sure there is any great point in the is_jsonb_data macro,
which is only used in one spot. I would get rid of it and expand the
test in place.

I don't have much time this week to work on it, as there are one or
two other patches I also want to look at. If you clean these things
up I will commit it. The second patch looks fine.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2017-03-29 17:08:13 Re: Patch: Write Amplification Reduction Method (WARM)
Previous Message Stas Kelvich 2017-03-29 15:55:13 Re: logical decoding of two-phase transactions