Re: Extended Statistics set/restore/clear functions.

From: Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
To: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Tender Wang <tndrwang(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Extended Statistics set/restore/clear functions.
Date: 2026-01-15 20:21:23
Message-ID: CADkLM=eDs0s0MVnjnybSrAxOYE+ksqyOAt5hJTQCwTyPWpBkUQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

>
>
> Here are some comments for v24:
>
> 1.
> ```
> + inherited = PG_GETARG_NAME(INHERITED_ARG);
> ```
>
> Should this use PG_GETARG_BOOL()?
>

Yes. I'm curious why the compiler didn't catch that.

>
> 2
> ```
> proparallel => 'u', prorettype => 'void', proargtypes => 'text text text
> text bool’,
> ```
>
> Should we define proargtypes as “name name name name bool”? As the doc
> change mentions type “name” for the first 4 parameters. Maybe you want to
> avoid that because Name is a structure. Anyway, I don’t have a strong
> opinion here.
>

The existing pg_restore_relation_stats() and pg_restore_attribute_stats()
use the text type for both parameters. This was done mostly to avoid
typecasts that could potentially fail in a pg_restore script, and partly to
make things easier on someone writing the function call. The -clear
variants of those functions followed the same convention of text type for
those paramers, and so this is just doing the same.

>
> 3 - extended_stats_funcs.c
> ```
> * Portions Copyright (c) 1996-2026, PostgreSQL Global Development Group
> ```
>
> This is a brand new file, so the copyright year should be just 2026. If
> you search for “2025-2026”, you will get a bunch of files, they should be
> created in 2025.
>

That may have been done because some of the code that will end up here is
being moved from other files rather than being purely new. My revised patch
is leaving it as-is for now.

>
> 4 - Given comment 1, no test fails. I think that’s because all test cases
> use inherited = false. Maybe we need a test case for inherited = true.
>

Added.

In addition to issues 1 and 4 I added some inheritance-specific tests per
Paquier's off-list request, and the result is attached.

Attachment Content-Type Size
v25-0001-Add-pg_clear_extended_stats.patch text/x-patch 25.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dharin Shah 2026-01-15 20:50:54 Re: [Patch] Add WHERE clause support to REFRESH MATERIALIZED VIEW
Previous Message Andreas Karlsson 2026-01-15 20:03:43 Re: Add IS_INDEX macro to brin and gist index