Re: using extended statistics to improve join estimates - Mailing list pgsql-hackers

From Justin Pryzby
Subject Re: using extended statistics to improve join estimates
Date
Msg-id Zhl8AunqdxBNPEAv@pryzbyj2023
Whole thread Raw
In response to Re: using extended statistics to improve join estimates  (Andy Fan <zhihuifan1213@163.com>)
Responses Re: using extended statistics to improve join estimates
List pgsql-hackers
On Tue, Apr 02, 2024 at 04:23:45PM +0800, Andy Fan wrote:
> 
> 0001 is your patch, I just rebase them against the current master. 0006
> is not much relevant with current patch, and I think it can be committed
> individually if you are OK with that.

Your 002 should also remove listidx to avoid warning
../src/backend/statistics/extended_stats.c:2879:8: error: variable 'listidx' set but not used
[-Werror,-Wunused-but-set-variable]

> Subject: [PATCH v1 2/8] Remove estimiatedcluases and varRelid arguments

> @@ -2939,15 +2939,11 @@ statext_try_join_estimates(PlannerInfo *root, List *clauses, int varRelid,
>          /* needs to happen before skipping any clauses */
>          listidx++;
>  
> -        /* Skip clauses that were already estimated. */
> -        if (bms_is_member(listidx, estimatedclauses))
> -            continue;
> -

Your 007 could instead test if relids == NULL:

> Subject: [PATCH v1 7/8] bms_is_empty is more effective than bms_num_members(b)
>-       if (bms_num_members(relids) == 0)
>+       if (bms_is_empty(relids))

typos:
001: s/heuristict/heuristics/
002: s/grantee/guarantee/
002: s/estimiatedcluases/estimatedclauses/

It'd be nice to fix/silence these warnings from 001:

|../src/backend/statistics/extended_stats.c:3151:36: warning: ‘relid’ may be used uninitialized
[-Wmaybe-uninitialized]
| 3151 |                                 if (var->varno != relid)
|      |                                    ^
|../src/backend/statistics/extended_stats.c:3104:33: note: ‘relid’ was declared here
| 3104 |         int                     relid;
|      |                                 ^~~~~
|[1016/1893] Compiling C object src/backend/postgres_lib.a.p/statistics_mcv.c.o
|../src/backend/statistics/mcv.c: In function ‘mcv_combine_extended’:
|../src/backend/statistics/mcv.c:2431:49: warning: declaration of ‘idx’ shadows a previous local
[-Wshadow=compatible-local]

FYI, I also ran the patch with a $large number of reports without
observing any errors or crashes.

I'll try to look harder at the next patch revision.

-- 
Justin



pgsql-hackers by date:

Previous
From: Melanie Plageman
Date:
Subject: Re: Table AM Interface Enhancements
Next
From: Andres Freund
Date:
Subject: Re: Issue with the PRNG used by Postgres