Horton

Re: PoC/WIP: Extended statistics on expressions

Tomas Vondra <tomas.vondra@enterprisedb.com>

View thread View on postgr.es
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
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:01:23Z
Lists: pgsql-hackers 

On 6/6/21 9:17 PM, Tom Lane wrote:
> Tomas Vondra <tomas.vondra@enterprisedb.com> writes:
>> On 6/6/21 7:37 AM, Noah Misch wrote:
>>> This added a "transformed" field to CreateStatsStmt, but it didn't mention
>>> that field in src/backend/nodes.  Should those functions handle the field?
> 
>> Yup, that's a mistake - it should do whatever CREATE INDEX is doing. Not
>> sure if it can result in error/failure or just inefficiency (due to
>> transforming the expressions repeatedly), but it should do whatever
>> CREATE INDEX is doing.
> 
> 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:

    List       *new_list = copyObject(raw_parsetree_list);

    /* This checks both copyObject() and the equal() routines... */
    if (!equal(new_list, raw_parsetree_list))
        elog(WARNING, "copyObject() failed to produce an equal raw
                       parse tree");
    else
        raw_parsetree_list = new_list;
    }

But if the field is missing from all the functions, equal() can't detect
that copyObject() did not actually copy it. It'd detect a case when the
field was added just to one place, but not this. The CREATE INDEX (which
served as an example for CREATE STATISTICS) has exactly the same issue.


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Commits

  1. Disallow extended statistics on system columns

  2. Identify simple column references in extended statistics

  3. Don't print extra parens around expressions in extended stats

  4. Change position of field "transformed" in struct CreateStatsStmt.

  5. Add transformed flag to nodes/*funcs.c for CREATE STATISTICS

  6. Stabilize stats_ext test with other collations

  7. Extended statistics on expressions

  8. Reduce duration of stats_ext regression tests

  9. Allow composite types in catalog bootstrap

  10. Convert Typ from array to list in bootstrap

  11. Disallow CREATE STATISTICS on system catalogs