> 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.