Re: 11beta crash/assert caused by parameter type changes

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com>
Cc: Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: 11beta crash/assert caused by parameter type changes
Date: 2018-07-26 18:06:16
Message-ID: 28192.1532628376@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com> writes:
> On 24/07/18 22:50, Andrew Gierth wrote:
>> Haven't had time yet to poke at what's changed in detail.

> git bisect lays the blame on 6719b238e8f0ea83c39d2b1728c7670cdaa34e06.

Right. So the reason that this Assert is reachable now is that that
commit made it possible to, as the commit message said, expose the
values of record-field variables to the planner. All the other places
that call paramFetch hooks either believe the returned "prmtype" or
do run-time checks that it matches what they think it should be.
This one's out of step with a simple assertion, so I think a reasonable
fix would be as attached.

Eventually, of course, we should make such cases Actually Work by forcing
a replan of the cached plan. But it was not the intent of anything I did
in v11 to make that happen, only to lay a bit more groundwork for it.
So it'd be unreasonable to try to fix that during beta.

I was about to add Andrew's example as a test case (also shown in
attached), but realized that there's a problem: just as noted in
the similar test for named-composite-type changes a bit above there,
the failure fails to fail in CLOBBER_CACHE_ALWAYS builds. We'd decided
to just omit that test (cf feb1cc559) but having been embarrassed by
this crash I'm feeling like we really could do with some test coverage.
I'm tempted to extract the affected test cases into their own small
test script and provide two variant expected files, one for normal and
one for CLOBBER_CACHE_ALWAYS builds. Thoughts?

regards, tom lane

Attachment Content-Type Size
crash-fixed-but-test-doesnt-work.patch text/x-diff 3.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message matshyeq 2018-07-26 18:06:36 Re: Enhancement request: enable FIRST/LAST_value() also as a regular aggregate (not only as windowing function)
Previous Message Andrey Borodin 2018-07-26 17:39:30 Re: [HACKERS] [PATCH] kNN for SP-GiST