Re: PoC/WIP: Extended statistics on expressions
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: Noah Misch <noah@leadboat.com>, Dean Rasheed <dean.a.rasheed@gmail.com>,
Justin Pryzby <pryzby@telsasoft.com>,
PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2021-06-06T20:08:52Z
Lists: pgsql-hackers
Tomas Vondra <tomas.vondra@enterprisedb.com> writes: > On 6/6/21 9:17 PM, Tom Lane wrote: >> I'm curious about how come the buildfarm didn't notice this. The >> animals using COPY_PARSE_PLAN_TREES should have failed. The fact >> that they didn't implies that there's no test case that makes use >> of a nonzero value for this field, which seems like a testing gap. > AFAICS the reason is pretty simple - the COPY_PARSE_PLAN_TREES checks > look like this: > ... > But if the field is missing from all the functions, equal() can't detect > that copyObject() did not actually copy it. Right, that code would only detect a missing copyfuncs.c line if equalfuncs.c did have the line, which isn't all that likely. However, we then pass the copied node on to further processing, which in principle should result in visible failures when copyfuncs.c is missing a line. I think the reason it didn't is that the transformed field would always be zero (false) in grammar output. We could only detect a problem if we copied already-transformed nodes and then used them further. Even then it *might* not fail, because the consequence would likely be an extra round of parse analysis on the expressions, which is likely to be a no-op. Not sure if there's a good way to improve that. I hope sometime soon we'll be able to auto-generate these functions, and then the risk of this sort of mistake will go away (he says optimistically). regards, tom lane
Commits
-
Disallow extended statistics on system columns
- 66061077155d 14.0 landed
- c9eeef2a15c0 15.0 landed
-
Identify simple column references in extended statistics
- 50ba70a957f9 14.0 landed
- 537ca68dbb24 15.0 landed
-
Don't print extra parens around expressions in extended stats
- 4d1816ec26e8 14.0 landed
- 13380e147649 15.0 landed
-
Change position of field "transformed" in struct CreateStatsStmt.
- 13a1ca160dcf 14.0 landed
-
Add transformed flag to nodes/*funcs.c for CREATE STATISTICS
- d57ecebd128c 14.0 landed
-
Stabilize stats_ext test with other collations
- 2a058e938c73 14.0 landed
-
Extended statistics on expressions
- a4d75c86bf15 14.0 landed
-
Reduce duration of stats_ext regression tests
- 98376c18f12e 14.0 landed
-
Allow composite types in catalog bootstrap
- 79f6a942bdb9 14.0 landed
-
Convert Typ from array to list in bootstrap
- e1a5e65703ce 14.0 landed
-
Disallow CREATE STATISTICS on system catalogs
- c22539756ee5 10.16 landed
- f52db969440b 11.11 landed
- 943a113bcb6f 12.6 landed
- d26d4c717dbf 13.2 landed
- c9a0dc34865f 14.0 landed