refactoring comment.c

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: refactoring comment.c
Date: 2010-08-06 15:02:58
Message-ID: AANLkTiktk8+dwyD6uGfyj=QXXFA_gLoDhaBbXUDSwvv6@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

At PGCon, we discussed the possibility that a minimal SE-PostgreSQL
implementation would need little more than a hook in
ExecCheckRTPerms() [which we've since added] and a security label
facility [for which KaiGai has submitted a patch]. I actually sat
down to write the security label patch myself while we were in Ottawa,
but quickly ran into difficulties: while the hook we have now can't do
anything useful with objects other than relations, it's pretty clear
from previous discussions on this topic that the demand for labels on
other kinds of objects is not going to go away. Rather than adding
additional syntax to every object type in the system (some of which
don't even have ALTER commands at present), I suggested basing the
syntax on the existing COMMENT syntax. After some discussion[1], we
seem to have settled on the following:

SECURITY LABEL [ FOR <provider> ] ON <object class> <object name> IS '<label>';

At present, there are some difficulties with generalizing this syntax
to other object types. As I found out when I initially set out to
write this patch, it'd basically require duplicating all of comment.c,
which is an unpleasant prospect, because that file is big and crufty;
it has a large amount of internal duplication. Furthermore, the
existing locking mechanism that we're using for comments is known to
be inadequate[2]. Dropping a comment while someone else is in the
midst of commenting on it leaves orphaned comments lying around in
pg_(sh)description that could later be inherited by a new object.
That's a minor nuisance for comments and would be nice to fix, but is
obviously a far larger problem for security labels, where even a small
chance of randomly mislabeling an object is no good.

So I wrote a patch. The attached patch factors out all of the code in
comment.c that is responsible for translating parser representations
into a new file parser/parse_object.c, leaving just the
comment-specific stuff in commands/comment.c. It also adds
appropriate locking, so that concurrent COMMENT/DROP scenarios don't
leave behind leftovers. It's a fairly large patch, but the changes
are extremely localized: comment.c gets a lot smaller, and
parse_object.c gets bigger by a slightly smaller amount.

Any comments? (ha ha ha...)

[1] http://archives.postgresql.org/pgsql-hackers/2010-07/msg01328.php
[2] http://archives.postgresql.org/pgsql-hackers/2010-07/msg00351.php

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

Attachment Content-Type Size
refactor_comment-v1.patch application/octet-stream 56.3 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Florian Pflug 2010-08-06 15:13:56 Re: Review: Row-level Locks & SERIALIZABLE transactions, postgres vs. Oracle
Previous Message Tom Lane 2010-08-06 14:53:06 Re: default of max_stack_depth