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 via email to