Message-ID: <1b3abed6-9014-d3ed-408f-acdf8e1e3c9a@dunslane.net>
Date: Fri, 29 Mar 2024 12:38:38 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Content-Language: en-US
To: Jacob Champion <jacob.champion@enterprisedb.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Peter Smith <smithpb2250@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: <7b0a51d6-0d9d-7366-3a1a-f74397a02f55@dunslane.net>
 <682c8fff-355c-a04f-57ac-81055c4ccda8@dunslane.net>
 <70705cf3-c875-d489-c6a9-7dd61f1dc1af@dunslane.net>
 <CAOYmi+n1dZEMCptykQS7Jd4A_bMyebAD9LuFXM4NaeaYKwH3ng@mail.gmail.com>
 <694fd815-0414-0f09-98b3-db28d77fa37f@dunslane.net>
 <CAOYmi+kgckFsBNOBsrekDipq88G56pVndVXKcA7qMWYx9feChQ@mail.gmail.com>
 <CAD5tBcLjFnTnAvUpQg-K5sJa3ECnEj8CQZ-AYrH1k+F=42D+Vg@mail.gmail.com>
 <CAOYmi+n1fxCt3QGz9g=GfY1+J6__vVqsAud=HYwpUrfZM_VJHw@mail.gmail.com>
 <CAD5tBcLi2ffZkktV2qrsKSBykE-N8CiYgrfbv0vZ-F7=xLFeqw@mail.gmail.com>
 <CAD5tBcJg-Ja3CcY8N+NtziFN8TL8JRWv97s=-siT=Oep2+JGjw@mail.gmail.com>
 <CAOYmi+nY=rF6dJCzaOuA3d-3FbwXCcecOs_S1NutexFA3dRXAw@mail.gmail.com>
 <CAOYmi+m8yeRLFPj68e0zOfZGKm1CAmWwrfsEmCjYxPTWn7itnA@mail.gmail.com>
 <CAD5tBcLm=+LUrGZftmRGxq1tWE9eAcviVJgEF=AX8P_=NDd=eg@mail.gmail.com>
 <CAOYmi+nEs4wVLSU4=sZU1Mf7j-bmsQoYD7CMZoEhwtNvz1jTOQ@mail.gmail.com>
 <CAOYmi+ne1=AKEeAFdN0DmcVHwhLW7J3R0GpAzzG5DMBOK_eerQ@mail.gmail.com>
From: Andrew Dunstan <andrew@dunslane.net>
Subject: Re: WIP Incremental JSON Parser
In-Reply-To: <CAOYmi+ne1=AKEeAFdN0DmcVHwhLW7J3R0GpAzzG5DMBOK_eerQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: <https://www.postgresql.org/message-id/1b3abed6-9014-d3ed-408f-acdf8e1e3c9a%40dunslane.net>
Precedence: bulk


On 2024-03-26 Tu 17:52, Jacob Champion wrote:
> On Mon, Mar 25, 2024 at 3:14 PM Jacob Champion
> <jacob.champion@enterprisedb.com> wrote:
>> - add Assert calls in impossible error cases [2]
> To expand on this one, I think these parts of the code (marked with
> `<- here`) are impossible to reach:
>
>> switch (top)
>> {
>>      case JSON_TOKEN_STRING:
>>          if (next_prediction(pstack) == JSON_TOKEN_COLON)
>>              ctx = JSON_PARSE_STRING;
>>          else
>>              ctx = JSON_PARSE_VALUE;                    <- here
>>          break;
>>      case JSON_TOKEN_NUMBER:                            <- here
>>      case JSON_TOKEN_TRUE:                              <- here
>>      case JSON_TOKEN_FALSE:                             <- here
>>      case JSON_TOKEN_NULL:                              <- here
>>      case JSON_TOKEN_ARRAY_START:                       <- here
>>      case JSON_TOKEN_OBJECT_START:                      <- here
>>          ctx = JSON_PARSE_VALUE;
>>          break;
>>      case JSON_TOKEN_ARRAY_END:                         <- here
>>          ctx = JSON_PARSE_ARRAY_NEXT;
>>          break;
>>      case JSON_TOKEN_OBJECT_END:                        <- here
>>          ctx = JSON_PARSE_OBJECT_NEXT;
>>          break;
>>      case JSON_TOKEN_COMMA:                             <- here
>>          if (next_prediction(pstack) == JSON_TOKEN_STRING)
>>              ctx = JSON_PARSE_OBJECT_NEXT;
>>          else
>>              ctx = JSON_PARSE_ARRAY_NEXT;
>>          break;
> Since none of these cases are non-terminals, the only way to get to
> this part of the code is if (top != tok). But inspecting the
> productions and transitions that can put these tokens on the stack, it
> looks like the only way for them to be at the top of the stack to
> begin with is if (tok == top). (Otherwise, we would have chosen a
> different production, or else errored out on a non-terminal.)
>
> This case is possible...
>
>>      case JSON_TOKEN_STRING:
>>          if (next_prediction(pstack) == JSON_TOKEN_COLON)
>>              ctx = JSON_PARSE_STRING;
> ...if we're in the middle of JSON_PROD_[MORE_]KEY_PAIRS. But the
> corresponding else case is not, because if we're in the middle of a
> _KEY_PAIRS production, the next_prediction() _must_ be
> JSON_TOKEN_COLON.
>
> Do you agree, or am I missing a way to get there?
>


One good way of testing would be to add the Asserts, build with 
-DFORCE_JSON_PSTACK, and run the standard regression suite, which has a 
fairly comprehensive set of JSON errors. I'll play with that.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com