From: Petr Jelinek <petr@2ndquadrant.com>
To: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Tatsuo Ishii <ishii@postgresql.org>
Cc: jeff.janes@gmail.com
Cc: alvherre@2ndquadrant.com
Cc: pgsql-hackers@postgresql.org
Subject: Re: multivariate statistics v14
Date: Wed, 23 Mar 2016 19:23:40 +0100
Message-ID: <56F2DF2C.6070202@2ndquadrant.com> (raw)
In-Reply-To: <eb8522d2-f1ee-7401-2118-e7d6b8990483@2ndquadrant.com>
References: <20160323.110125.1808526964660429411.t-ishii@sraoss.co.jp>
<20160323.115758.938003466026607400.t-ishii@sraoss.co.jp>
<20160323.120101.207302357952079425.t-ishii@sraoss.co.jp>
<20160323.142004.624106180171245380.t-ishii@sraoss.co.jp>
<eb8522d2-f1ee-7401-2118-e7d6b8990483@2ndquadrant.com>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: petr@2ndquadrant.com, tomas.vondra@2ndquadrant.com, ishii@postgresql.org, jeff.janes@gmail.com, alvherre@2ndquadrant.com
Subject: Re: multivariate statistics v14
In-Reply-To: <56F2DF2C.6070202@2ndquadrant.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox