Обсуждение: COMMENT ON mega patch

This patch does the following:

1. Comment on 5 new objects:

COMMENT ON CONVERSION my_conv IS 'Conversion to Unicode';
COMMENT ON LANGUAGE plpython IS 'Python support for stored procedures';
COMMENT ON OPERATOR CLASS int4ops USING btree IS '4 byte integer
operators for btrees';
COMMENT ON LARGE OBJECT 346344 IS 'Planning document';
COMMENT ON CAST (text AS int4) IS 'Allow casts from text to int4';

2. Dump conversions with pg_dump

3. Dump comments on conversions, languages, opclasses and casts with pg_dump

4. Upgrade psql's lob interface to use COMMENT ON instead of hacky insert.

5. Make large object comments get automatically removed when the object
is lo_unlink'd, so clients don't need any special code.

6. Documentation updates for (1)

7. Regression tests for commenting on all objects (currently COMMENT ON
  is strangely absent from the regression tests, as this shows:
http://www.alcove.com.au/pgregress/commands/index.html )

Chris

Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
> This patch does the following:
> 1. Comment on 5 new objects:
> [ etc ]

Reviewed and applied.  Couple things I didn't like:

* You were using a bare C string as the amname argument in COMMENT ON
OPERATOR CLASS.  This won't do because the parse tree is not a valid
Node structure; copyObject will fail on it.  I inserted makeString()
and strVal() calls to fix it.

BTW, a simple test to detect uncopiable-parsetree problems is to compile
with COPY_PARSE_PLAN_TREES defined.  Doing so revealed that you're not
the only person to have made this mistake lately --- ALTER SEQUENCE is
broken too.

* I made the macros LARGE and OBJECT be LARGE_P and OBJECT_P; they
seemed just a little too ripe for conflicts as-is ...

* The pg_dump code for COMMENT ON OPCLASS pretty obviously had not been
tested :-(

            regards, tom lane

> * You were using a bare C string as the amname argument in COMMENT ON
> OPERATOR CLASS.  This won't do because the parse tree is not a valid
> Node structure; copyObject will fail on it.  I inserted makeString()
> and strVal() calls to fix it.
>
> BTW, a simple test to detect uncopiable-parsetree problems is to compile
> with COPY_PARSE_PLAN_TREES defined.  Doing so revealed that you're not
> the only person to have made this mistake lately --- ALTER SEQUENCE is
> broken too.

Ok.

> * I made the macros LARGE and OBJECT be LARGE_P and OBJECT_P; they
> seemed just a little too ripe for conflicts as-is ...
 >
> * The pg_dump code for COMMENT ON OPCLASS pretty obviously had not been
> tested :-(

Hrm.  Yeah, weird.  I think I just forgot, because I learned in this
patch that you can't do two of those %s in the append function, and I
had fixed it in all the other dumps. *sigh*

The other thing I was concerned about was a bit of code duplication,
especially for the comment on opclass function.

Out of interest, I notice you didn't commit my inv_api.c change to
delete comments on LOBs - where did you put it instead?

Chris

> Out of interest, I notice you didn't commit my inv_api.c change to
> delete comments on LOBs - where did you put it instead?

Doh - just answered my own question - elsewhere in the same file.

Chris

Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
>> Out of interest, I notice you didn't commit my inv_api.c change to
>> delete comments on LOBs - where did you put it instead?

> Doh - just answered my own question - elsewhere in the same file.

Just editorializing --- the order of operations you'd chosen seemed a
tad strange to me.

            regards, tom lane