Re: pure parsers and reentrant scanners

On 19.12.24 13:48, Peter Eisentraut wrote:
> On 17.12.24 01:46, Andreas Karlsson wrote:
>> On 12/16/24 8:39 AM, Peter Eisentraut wrote:
>>> I'll leave it at this for now and wait for some reviews.
>>
>> I really like this work since it makes the code cleaner to read on top
>> of paving the way for threading.
>>
>> Reviewed the patches and found a couple of issues.
>>
>> - Shouldn't yyext in syncrep_scanner_init() be allocated on the heap?
>> Or at least on the stack but by the caller?
>
> I think it's correct the way it is.  It's only a temporary space for the
> scanner, so we can allocate it in the innermost scope.

>> - Also fixed the static remaining variables in the replication parser
>> in an attached patch.
>
> Thanks, I'll take a look at that.

I see what was going on here. I was allocating yyext as a local
variable in the init function and then it would go out of scope while
the scanner is still in use. That's why this didn't work for me. I had
written essentially the same patch as you for the replication scanner
yyextra but with a local variable, and it was "mysteriously" failing the
tests for me. Your solution is better. (For the jsonpath scanner, the
local variable works because the scanner init and shutdown are called
from the same function.)

Here is an updated patch set on top of what has been committed so far,
with all the issues you pointed out addressed.