2011/1/5 KaiGai Kohei <kaigai@ak.jp.nec.com>:
> How about feasibility to merge this 4KL chunks during the rest of
> 45 days? I think we should decide this general direction at first.
I read through this code tonight and it looks pretty straightforward.
I don't see much reason not to accept this more or less as-is. I'm a
bit suspicious of this line:
{ "translation", SEPG_PROCESS__TRANSITION },
I can't help wondering based on the rest of the table whether you
intend to have the same word on that line twice, but you don't. On a
related note, would it make sense to pare down this table to the
entries that are actually used at the moment? And how about adding a
ProcessUtility_hook to trap evil non-DML statements that some
nefarious user might issues?
One other problem is that you need to work on your whitespace a bit.
I believe in a few places you have a mixture of tabs and spaces. More
seriously, pgindent is going to completely mangle things like this:
/** sepgsql_mode** SEPGSQL_MODE_DISABLED : Disabled on runtime* SEPGSQL_MODE_DEFAULT : Same as system
settings*SEPGSQL_MODE_PERMISSIVE : Always permissive mode* SEPGSQL_MODE_INTERNAL : Same as
SEPGSQL_MODE_PERMISSIVE,* except for
no audit prints*/
You have to write it with a line of dashes on the first and last
lines, if you don't want it reformatted as a paragraph. It might be
worth actually running pgindent over contrib/selinux and then check
over the results.
Finally, we need to work on the documentation.
But overall, it looks pretty good, IMHO.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
