Re: multivariate statistics v14

Petr Jelinek <petr@2ndquadrant.com>

From: Petr Jelinek <petr@2ndquadrant.com>
To: Tomas Vondra <tomas.vondra@2ndquadrant.com>, Tatsuo Ishii <ishii@postgresql.org>
Cc: jeff.janes@gmail.com, alvherre@2ndquadrant.com, pgsql-hackers@postgresql.org
Date: 2016-03-23T18:23:40Z
Lists: pgsql-hackers
Hi,

I'll add couple of code comments from my first cursory read through 
(this is huge):

0002:
there is some whitespace noise between the varlistentries in 
alter_statistics.sgml

+	parentobject.classId = RelationRelationId;
+	parentobject.objectId = ObjectIdGetDatum(RelationGetRelid(rel));
+	parentobject.objectSubId = 0;
+	childobject.classId = MvStatisticRelationId;
+	childobject.objectId = statoid;
+	childobject.objectSubId = 0;

I wonder if this (several places similar code) would be simpler done 
using ObjectAddressSet()

The common.h in backend/utils/mvstat is slightly weird header file 
placement and naming.


0004:
+/* used for merging bitmaps - AND (min), OR (max) */
+#define MAX(x, y) (((x) > (y)) ? (x) : (y))
+#define MIN(x, y) (((x) < (y)) ? (x) : (y))

Huh? We have Max and Min macros defined in c.h

+		values[Anum_pg_mv_statistic_stamcv  - 1] = PointerGetDatum(data);

Why the double space (that's actually in several places in several of 
the patches).

I don't really understand why 0008 and 0009 are separate patches and 
aren't part of one of the other patches. But otherwise good job on 
splitting the functionality into patchset.

-- 
   Petr Jelinek                  http://www.2ndQuadrant.com/
   PostgreSQL Development, 24x7 Support, Training & Services


Commits

  1. Collect and use multi-column dependency stats

  2. Implement SortSupport for macaddr data type

  3. Implement multivariate n-distinct coefficients

  4. Generate fmgr prototypes automatically