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