Re: Statistics Import and Export - Mailing list pgsql-hackers

From Bertrand Drouvot
Subject Re: Statistics Import and Export
Date
Msg-id Ze7F2b/8EtazZB3u@ip-10-97-1-34.eu-west-3.compute.internal
Whole thread Raw
In response to Re: Statistics Import and Export  (Corey Huinker <corey.huinker@gmail.com>)
List pgsql-hackers
Hi,

On Fri, Mar 08, 2024 at 01:35:40AM -0500, Corey Huinker wrote:
> Anyway, here's v7. Eagerly awaiting feedback.

Thanks!

A few random comments:

1 ===

+        The purpose of this function is to apply statistics values in an
+        upgrade situation that are "good enough" for system operation until

Worth to add a few words about "influencing" the planner use case?

2 ===

+#include "catalog/pg_type.h"
+#include "fmgr.h"

Are those 2 needed?

3 ===

+       if (!HeapTupleIsValid(ctup))
+               elog(ERROR, "pg_class entry for relid %u vanished during statistics import",

s/during statistics import/when setting statistics/?

4 ===

+Datum
+pg_set_relation_stats(PG_FUNCTION_ARGS)
+{
.
.
+       table_close(rel, ShareUpdateExclusiveLock);
+
+       PG_RETURN_BOOL(true);

Why returning a bool? (I mean we'd throw an error or return true).

5 ===

+ */
+Datum
+pg_set_attribute_stats(PG_FUNCTION_ARGS)
+{

This function is not that simple, worth to explain its logic in a comment above?

6 ===

+       if (!HeapTupleIsValid(tuple))
+       {
+               relation_close(rel, NoLock);
+               PG_RETURN_BOOL(false);
+       }
+
+       attr = (Form_pg_attribute) GETSTRUCT(tuple);
+       if (attr->attisdropped)
+       {
+               ReleaseSysCache(tuple);
+               relation_close(rel, NoLock);
+               PG_RETURN_BOOL(false);
+       }

Why is it returning "false" and not throwing an error? (if ok, then I think
we can get rid of returning a bool).

7 ===

+        * If this relation is an index and that index has expressions in
+        * it, and the attnum specified

s/is an index and that index has/is an index that has/?

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com



pgsql-hackers by date:

Previous
From: Svetlana Derevyanko
Date:
Subject: Re: Add Index-level REINDEX with multiple jobs
Next
From: John Naylor
Date:
Subject: Re: Add bump memory context type and use it for tuplesorts