| From: | Richard Guo <guofenglinux(at)gmail(dot)com> |
|---|---|
| To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
| Cc: | Japin Li <japinli(at)hotmail(dot)com>, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, a(dot)kozhemyakin(at)postgrespro(dot)ru, PostgreSQL mailing lists <pgsql-bugs(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: BUG #17570: Unrecognized node type for query with statistics on expressions |
| Date: | 2022-08-05 00:49:30 |
| Message-ID: | CAMbWs4-g44xwS6KCEmSaoc8jqcTd1n8tt8u10=hChFTEyRJQbA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-bugs |
On Thu, Aug 4, 2022 at 11:42 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> * statext_is_compatible_clause_internal is not on board with
> offsetting the attnums.
Correct. Good catch! Fix this in v2 patch.
> * Neither is this check for a whole-row reference (line 1628, in HEAD):
>
> if (bms_is_member(InvalidAttrNumber, clause_attnums))
Actually this has been fixed in my v1 patch [1].
> * Although the coverage report claims this code is exercised, the
> fact that nothing failed when you made only a partial fix says it's
> not exercised well enough.
That's true. We need to provide a better test case to cover this.
>
> * The comments for statext_is_compatible_clause suck. If they'd
> defined what the arguments are, perhaps this mess would have been
> prevented. For extra credit, it'd be really nice to define what
> "is compatible" means. I'd sure not have thought that that would
> include permissions checks.
>
Concur with that.
Thanks
Richard
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-fix-bug-17570.patch | application/octet-stream | 2.5 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Richard Guo | 2022-08-05 01:25:08 | Re: BUG #17570: Unrecognized node type for query with statistics on expressions |
| Previous Message | Masahiko Sawada | 2022-08-05 00:47:57 | Re: Excessive number of replication slots for 12->14 logical replication |