On 06/10/2015 04:02 PM, Peter Geoghegan wrote:
> On Wed, Jun 10, 2015 at 11:48 AM, Andrew Dunstan <andrew@dunslane.net> wrote:
>> Sorry for the delay on this. I've been mostly off the grid, having an all
>> too rare visit from Tom "Mr Enum" Dunstan, and I misunderstood what you were
>> suggesting,
> Thank you for working with me to address this. I've been busy with
> other things the past few days too.
>
>> Please submit a patch to adjust the treatment of negative integers in the
>> old functions to be consistent with their treatment in the new functions.
>> i.e. in the range [-n,-1] they should refer to the corresponding element
>> counting from the right.
> I've almost finished that patch. I'm currently blocked on deciding
> what to do about the old path-orientated operators (#> and #>> for
> json and jsonb types). It's rather painful to support pushing down
> negative subscripting there -- maybe we should just not do so for
> those variants, especially given that they're already notationally
> inconsistent with the other operators that I'll be updating. What do
> you think?
>
> Maybe I'll come up with a simpler way of making that work by taking a
> fresh look at it, but haven't done that yet.
>
> My current, draft approach to making subscripting work with the json
> variants (not the jsonb variants) is to use a second get_worker() call
> in the event of a negative subscript, while making the first such call
> (the existing get_worker() call) establish the number of top-level
> array elements. That isn't beautiful, and involves some amount of
> redundant work, but it's probably better than messing with
> get_worker() in a more invasive way. Besides, this second get_worker()
> call only needs to happen in the event of a negative subscript, and
> I'm only really updating this (that is, updating routines like
> json_array_element()) to preserve consistency with jsonb. What do you
> think of that idea?
>
Just took a quick look. My impression is that the jsonb case should be
fairly easy. If the index is negative, add JB_ROOT_COUNT(container) to
it and use that as the argument to getIthJsonbValueFromContainer().
I agree that the json case looks a bit nasty. Maybe a better approach
would be to provide a function that, given a JsonLexContext, returns the
number of array elements of the current array. In get_array_start we
could call that if the relevant path element is negative and adjust it
accordingly.
cheers
andrew
