Re: [PATCH] Generic type subscription

>On 5 October 2016 at 22:59, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru> wrote:
>I've looked at your patch to make some review.

Thanks for the feedback.

> On 04.10.2016 11:28, Victor Wagner wrote: Git complains about whitespace
in
> this version of patch:

Fixed.

> The term "subscription" is confusing me

Yes, you're right. "container" is too general I think, so I renamed
everything
to "subscripting".

> Here '1' is used as a second item index. But from the last discussion
> https://www.postgresql.org/message-id/55D24517.8080609%40agliodbs.com it
> should be a key

Well, I missed that, since I used already implemented function "setPath",
and
this function implies that "all path elements before the last must already
exist", so in this case:

select jsonb_set('{"a": {}}', '{a, a1, 1}', '42');

nothing will be changed. I agree, we can implement this as discussed, but
we need
to do it inside "setPath", so it will be like "globally".

> I'm not sure but maybe we should use here two different functions?

I thought about that, but finally decided to keep changes into "pg_type" as
small as possible (anyway, these two functions will serve one logical
purpose).

> Here typeOid argument is not used. Is it should be here?

No, it shouldn't, fixed. It's interesting, that the same is correct for
"findTypeAnalyzeFunction" (I used this function as an example).

> I think name of the function is confusing. Maybe use
> transformContainerType()?

The point is that "transformArrayType" still contains some array-specific
code,
although it doesn't affect to any other type. So I think it's not completely
correct to use "transformContainerType" name.

> Why did you remove static keyword? setPath() is still static
> Is declaration of "new" variable necessary here?

I changed it back.

Here is a new version of patch.