Em qua., 23 de jun. de 2021 às 03:04, Greg Nancarrow <gregn4...@gmail.com> escreveu:
> On Tue, Jun 22, 2021 at 10:56 PM Ranier Vilela <ranier...@gmail.com> > wrote: > > > > On Mon, Jun 21, 2021 at 04:19:27PM -0700, Jim Nasby wrote: > > > The following generates an assertion failure. Quick testing with start > and > > > stop as well as the core dump shows it’s failing on the execution of > > > `schema_name := schema_name(i)` immediately after COMMIT, because > there’s no > > > active snapshot. On a build without asserts I get a failure in > > > GetActiveSnapshot() (second stack trace). This works fine on > 12_STABLE, but > > > fails on 13_STABLE and HEAD. > > > > For me it's a typo. > > need_snapshot = (expr->expr_simple_mutable || !estate->readonly_func); > > > ... > > > > The comments in the function are clear: > > If expression is mutable OR is a non-read-only function, so need a > snapshot. > > > > I have to agree with you. > Looks like the "&&" should really be an "||". The explanation in the > code comment is pretty clear on this, as you say. > > I was able to reproduce the problem using your example. It produced a > coredump, pointing to the failed "Assert(ActiveSnapshotSet());" in > pg_plan_query(). > Yes before d102aafb6, Jim Nasby example fires a coredump. I also verified that your patch seemed to fix the problem. > Both fix the Jim example. > However, I found that this issue is masked by the following recent commit: > > commit d102aafb6259a6a412803d4b1d8c4f00aa17f67e > Author: Tom Lane <t...@sss.pgh.pa.us> > Date: Tue Jun 22 17:48:39 2021 -0400 > Restore the portal-level snapshot for simple expressions, too. > > With this commit in place, there is an active snapshot in place > anyway, so for your example, that Assert no longer fires as it did > before. > However, I still think that your fix is valid and needed. > I agreed. Before 84f5c29, only the not-read-only function NOT push a new snapshot. Now only mutable expression AND not-read-only function, pushes a new snapshot. Under which conditions did Jim's example not fit? With && is very restricted. We have: 1. Mutable expression AND not-read-only function -> push a new snapshot 2. Mutable expression AND read-only-function -> not push a new snapshot 3. Not mutable expression AND not-read-only function -> not push a new snapshot 4. Not mutable expression AND read-only function -> not push a new snapshot We agree that 2 and 3 should push a new snapshot. If the user function is declared as not-read-only, even though read-only, it's a failure to be fixed either by the user, or by the parser, not here. regards, Ranier Vilela
