Horton

Re: [HACKERS] [PATCH] Generic type subscripting

Dmitry Dolgov <9erthalion6@gmail.com>

View thread View on postgr.es
From: Dmitry Dolgov <9erthalion6@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Justin Pryzby <pryzby@telsasoft.com>, Pavel Stehule <pavel.stehule@gmail.com>, David Steele <david@pgmasters.net>, Nikita Glukhov <n.gluhov@postgrespro.ru>, Alvaro Herrera <alvherre@2ndquadrant.com>, David Fetter <david@fetter.org>, Thomas Munro <thomas.munro@gmail.com>, Thomas Munro <thomas.munro@enterprisedb.com>, Oleksandr Shulgin <oleksandr.shulgin@zalando.de>, Robert Haas <robertmhaas@gmail.com>, Andres Freund <andres@anarazel.de>, Michael Paquier <michael.paquier@gmail.com>, Oleg Bartunov <obartunov@gmail.com>, Peter Eisentraut <peter.eisentraut@2ndquadrant.com>, pgsql-hackers@postgresql.org
Date: 2020-08-01T14:33:34Z
Lists: pgsql-hackers 
> On Fri, Jul 31, 2020 at 03:35:22PM -0400, Tom Lane wrote:
>
> I started to look through this again, and really found myself wondering
> why we're going to all this work to invent what are fundamentally pretty
> bogus "features".  The thing that particularly sticks in my craw is the
> 0005 patch, which tries to interpret a subscript of a JSON value as either
> integer or text depending on, seemingly, the phase of the moon.  I don't
> think that will work.  For example, with existing arrays you can do
> something like arraycol['42'] and the unknown-type literal is properly
> cast to an integer.  The corresponding situation with a JSON subscript
> would have no principled resolution.
>
> It doesn't help any that both coercion alternatives are attempted at
> COERCION_ASSIGNMENT level, which makes it noticeably more likely that
> they'll both succeed.  But using ASSIGNMENT level is quite inappropriate
> in any context where it's not 100% certain what the intended type is.
>
> The proposed commit message for 0005 claims that this is somehow improving
> our standards compliance, but I see nothing in the SQL spec suggesting
> that you can subscript a JSON value at all within the SQL language, so
> I think that claim is just false.

It's due to my lack of writing skills. As far as I can remember the
discussion was about JSON path part of the standard, where one allowed
to use float indexes with implementation-defined rounding or truncation.
In this patch series I'm trying to think of JSON subscript as an
equivalent for JSON path, hence this misleading description. Having said
that, I guess the main motivation behind 0005 is performance
improvements. Hopefully Nikita can provide more insights. Overall back
when 0005 patch was suggested its implementation looked reasonable for
me, but I'll review it again.

> Maybe this could be salvaged by flushing 0005 in its current form and
> having the jsonb subscript executor do something like "if the current
> value-to-be-subscripted is a JSON array, then try to convert the textual
> subscript value to an integer".  Not sure about what the error handling
> rules ought to be like, though.

I'm fine with the idea of separating 0005 patch and potentially prusuing
it as an independent item. Just need to rebase 0006, since Pavel
mentioned that it's a reasonable change he would like to see in the
final result.

Commits

  1. Throw error when assigning jsonb scalar instead of a composite object

  2. Filling array gaps during jsonb subscripting

  3. Implementation of subscripting for jsonb

  4. Allow ALTER TYPE to update an existing type's typsubscript value.

  5. Allow subscripting of hstore values.

  6. Support subscripting of arbitrary types, not only arrays.

  7. jit: Reference function pointer types via llvmjit_types.c.

  8. Teach contain_leaked_vars that assignment SubscriptingRefs are leaky.

  9. jit: Correct parameter type for generated expression evaluation functions.

  10. Renaming for new subscripting mechanism

  11. Fix assertion failure for SSL connections.

  12. Teach eval_const_expressions() to handle some more cases.