Hi,
I gave a very quick skim to patch 0002. Not a real review yet. But
there are a few trivial points to fix:
* You still have empty sections in the SGML docs (such as the EXAMPLES).
I suppose the syntax is now firm enough that we can get some. (I looked
at the other patches to see whether it was filled in, but couldn't find
any additional text there.)
* check_object_ownership() needs to be filled in
* Since you're adding a new object type, please add a case to cover it
in the object_address.sql pg_regress test.
* in analyze.c (and elsewhere), please put new #include lines sorted.
* I think the AT_PASS_ADD_STATS is a leftover which should be removed.
* The XXX comment in get_relation_info should probably be handled
differently (namely, in a way that makes the syscache not contain OIDs
of dropped stats)
* The README.dependencies has a lot of TODOs. Do we need to get them
done during the first cut? If not, I suggest creating a new section
"Future work" in the file.
* Please put the common.h header in src/include. Make sure not to
include "postgres.h" in it -- our policy is that postgres.h goes at the
top of every .c file and never in any .h file. Also please find a
better name for it; even mvstats_common.h would be a lot more
convincing. However:
* ISTM that the code in common.c properly belongs in
src/backend/catalog/pg_mvstats.c instead (or more properly
catalog/pg_mv_statistics.c), which probably means the common.h file
should be named something else; perhaps some of it could become
pg_mv_statistic_fn.h, while the rest continues to be
src/include/utils/mvstats_common.h? Not sure.
* The version check in psql/describe.c uses 90500; should probably be
updated to 90600.
* _copyCreateStatsStmt is missing if_not_exists
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
