Hi.
I found a TODO item "Add dumping of comments on index columns and
composite type columns" for pg_dump
and want to write a patch for it.

But I'm not sure if I understand the problem correctly.
Does "Add dumping of comments on index columns" mean that
pg_dump should dump out COMMENT statements like following?

COMMENT ON COLUMN some_index.index_column1 IS 'Hello column1';

Can anybody give me some advice on this?

Best regards,

-----
Taro Minowa(Higepon)

http://www.monaos.org/
http://code.google.com/p/mosh-scheme/

Search Discussions

  • Higepon at Mar 23, 2009 at 8:28 am
    Hi.
    I found a TODO item "Add dumping of comments on index columns and
    composite type columns" for pg_dump
    and want to write a patch for it.

    But I'm not sure if I understand the problem correctly.
    Does "Add dumping of comments on index columns" mean that
    pg_dump should dump out COMMENT statements like following?

    COMMENT ON COLUMN some_index.index_column1 IS 'Hello column1';

    Can anybody give me some advice on this?

    Best regards,

    -----
    Taro Minowa(Higepon)

    http://www.monaos.org/
    http://code.google.com/p/mosh-scheme/
  • Bruce Momjian at Mar 24, 2009 at 2:11 pm

    higepon wrote:
    Hi.
    I found a TODO item "Add dumping of comments on index columns and
    composite type columns" for pg_dump
    and want to write a patch for it.

    But I'm not sure if I understand the problem correctly.
    Does "Add dumping of comments on index columns" mean that
    pg_dump should dump out COMMENT statements like following?

    COMMENT ON COLUMN some_index.index_column1 IS 'Hello column1';

    Can anybody give me some advice on this?
    Wow, I have no idea what that means. I am wondering if we should just
    remove this TODO item. We don't even support comments on indexed
    columns, so why would pg_dump need to dump it?

    The full text is:

    Add dumping of comments on index columns and composite type columns

    Do we support comments on composite types?

    This item first appeared on the TODO list in Postgres 8.0.

    --
    Bruce Momjian <bruce@momjian.us> http://momjian.us
    EnterpriseDB http://enterprisedb.com

    + If your life is a hard drive, Christ can be your backup. +
  • Higepon at Mar 24, 2009 at 2:35 pm
    Hi.
    Wow, I have no idea what that means. I am wondering if we should just
    remove this TODO item. We don't even support comments on indexed
    columns, so why would pg_dump need to dump it?
    Oh I see.
    But we still can comment on indexed columns like following on 8.3.7,
    is it unsupported feature?

    create table person (social_no integer, name text, age integer, uri
    text, PRIMARY KEY (social_no));
    create index person_age on person using BTREE (age);
    comment on column person_age.age IS 'hello index person_age.age';

    And we can find the comment in pg_description table.
    Do we support comments on composite types
    If we do, I will also write a patch for it.

    Cheers.

    -----
    Taro Minowa(Higepon)

    http://www.monaos.org/
    http://code.google.com/p/mosh-scheme/

    On Tue, Mar 24, 2009 at 11:10 PM, Bruce Momjian wrote:
    higepon wrote:
    Hi.
    I found a TODO item "Add dumping of comments on index columns and
    composite type columns" for pg_dump
    and want to write a patch for it.

    But I'm not sure if I understand the problem correctly.
    Does "Add dumping of comments on index columns" mean that
    pg_dump should dump out COMMENT statements like following?

    COMMENT ON COLUMN some_index.index_column1 IS 'Hello column1';

    Can anybody give me some advice on this?
    Wow, I have no idea what that means.  I am wondering if we should just
    remove this TODO item.  We don't even support comments on indexed
    columns, so why would pg_dump need to dump it?

    The full text is:

    Add dumping of comments on index columns and composite type columns

    Do we support comments on composite types?

    This item first appeared on the TODO list in Postgres 8.0.

    --
    Bruce Momjian  <bruce@momjian.us>        http://momjian.us
    EnterpriseDB                             http://enterprisedb.com

    + If your life is a hard drive, Christ can be your backup. +
  • Tom Lane at Mar 24, 2009 at 2:56 pm

    Bruce Momjian writes:
    Wow, I have no idea what that means. I am wondering if we should just
    remove this TODO item. We don't even support comments on indexed
    columns, so why would pg_dump need to dump it?
    The system will let you do it, both cases:

    regression=# create type foo as (f1 int, f2 text);
    CREATE TYPE
    regression=# comment on column foo.f2 is 'column of a composite type';
    COMMENT
    regression=# create table tt (f1 int primary key);
    NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "tt_pkey" for table "tt"
    CREATE TABLE
    regression=# comment on column tt_pkey.f1 is 'column of an index';
    COMMENT

    and pg_dump fails to dump both cases.

    Commenting on a composite-type column seems reasonable. I'm less happy
    about the other because it depends on the names assigned to index
    columns, which are implementation artifacts. I'd rather see us forbid
    the case.

    regards, tom lane
  • Higepon at Mar 26, 2009 at 7:39 am
    Hi.

    Here is a patch for pg_dump "Commenting on a composite-type column".
    This patch is for Todo item named "Add dumping of comments on index
    columns and composite type columns".
    As Tom Lane said, this patch is not for dumping "comments on index columns",
    but only for "comment on composite-type column".

    With this patch, pg_dump can dump comments on composite-type column.

    --
    -- Name: COLUMN bar.b1; Type: COMMENT; Schema: public; Owner: taro
    --

    COMMENT ON COLUMN bar.b1 IS 'column of a composite type b1';


    --
    -- Name: COLUMN bar.b3; Type: COMMENT; Schema: public; Owner: taro
    --

    COMMENT ON COLUMN bar.b3 IS 'column of a composite type b3';

    Would someone please review this?

    Cheers.

    -----
    Taro Minowa(Higepon)

    Cybozu Labs, Inc.

    http://www.monaos.org/
    http://code.google.com/p/mosh-scheme/

    On Tue, Mar 24, 2009 at 11:56 PM, Tom Lane wrote:
    Bruce Momjian <bruce@momjian.us> writes:
    Wow, I have no idea what that means.  I am wondering if we should just
    remove this TODO item.  We don't even support comments on indexed
    columns, so why would pg_dump need to dump it?
    The system will let you do it, both cases:

    regression=# create type foo as (f1 int, f2 text);
    CREATE TYPE
    regression=# comment on column foo.f2 is 'column of a composite type';
    COMMENT
    regression=# create table tt (f1 int primary key);
    NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index "tt_pkey" for table "tt"
    CREATE TABLE
    regression=# comment on column tt_pkey.f1 is 'column of an index';
    COMMENT

    and pg_dump fails to dump both cases.

    Commenting on a composite-type column seems reasonable.  I'm less happy
    about the other because it depends on the names assigned to index
    columns, which are implementation artifacts.  I'd rather see us forbid
    the case.

    regards, tom lane
  • Robert Haas at Mar 26, 2009 at 2:37 pm
    Would someone please review this?
    Since we are about to go to beta, it may be that no one is up for
    reviewing it right now. But I've added it to the CommitFest page for
    the next CommitFest.

    http://wiki.postgresql.org/wiki/CommitFest_2009-First

    ...Robert
  • Higepon at Mar 26, 2009 at 2:40 pm
    Hi.
    Since we are about to go to beta, it may be that no one is up for
    reviewing it right now.  But I've added it to the CommitFest page for
    the next CommitFest.
    Thank you.
    I wait until the next CommitFest.

    -----
    Taro Minowa(Higepon)

    http://www.monaos.org/
    http://code.google.com/p/mosh-scheme/
  • Jaime Casanova at Jul 14, 2009 at 7:34 am

    On Thu, Mar 26, 2009 at 2:39 AM, higeponwrote:
    Hi.

    Here is a patch for pg_dump "Commenting on a composite-type column".
    This patch is for Todo item named "Add dumping of comments on index
    columns and composite type columns".
    this one looks good to me, the only adjust i made to the patch is
    change the name for the function that dump the comments from the
    composite types columns for: dumpCompositeTypeColsComment that seems
    more clearer to me...

    the patch works just fine...

    --
    Atentamente,
    Jaime Casanova
    Soporte y capacitación de PostgreSQL
    Asesoría y desarrollo de sistemas
    Guayaquil - Ecuador
    Cel. +59387171157
  • Higepon at Jul 15, 2009 at 4:42 am

    Jaime Casanova wrote:

    this one looks good to me, the only adjust i made to the patch is
    Thank you for your review!

    ---
    Taro Minowa(Higepon)

    http://www.monaos.org/
    http://code.google.com/p/mosh-scheme/


    On Tue, Jul 14, 2009 at 4:34 PM, Jaime
    Casanovawrote:
    On Thu, Mar 26, 2009 at 2:39 AM, higeponwrote:
    Hi.

    Here is a patch for pg_dump "Commenting on a composite-type column".
    This patch is for Todo item named "Add dumping of comments on index
    columns and composite type columns".
    this one looks good to me, the only adjust i made to the patch is
    change the name for the function that dump the comments from the
    composite types columns for: dumpCompositeTypeColsComment that seems
    more clearer to me...

    the patch works just fine...

    --
    Atentamente,
    Jaime Casanova
    Soporte y capacitación de PostgreSQL
    Asesoría y desarrollo de sistemas
    Guayaquil - Ecuador
    Cel. +59387171157
  • Brendan Jurd at Jul 22, 2009 at 5:54 am

    2009/7/14 Jaime Casanova <jcasanov@systemguards.com.ec>:
    On Thu, Mar 26, 2009 at 2:39 AM, higeponwrote:
    Here is a patch for pg_dump "Commenting on a composite-type column".
    This patch is for Todo item named "Add dumping of comments on index
    columns and composite type columns".
    this one looks good to me, the only adjust i made to the patch is
    change the name for the function that dump the comments from the
    composite types columns for: dumpCompositeTypeColsComment that seems
    more clearer to me...

    the patch works just fine...
    Oops. I picked this patch up from the commitfest queue because it was
    still marked as "Needs Review", not realising that Jaime had already
    done an initial review of the patch.

    Seems like this one should have been "Ready for Committer", but no matter.

    I've also done an initial review of the patch. Everything looks sane
    and the patch works as advertised. I made a couple of minor tweaks
    for code-style and comment consistency, and my version 3 is attached.

    I'm marking this patch Ready for Committer.

    I've also added a TODO item "forbid COMMENT on columns of an index",
    per Tom's comments upthread.

    Cheers,
    BJ
  • Tom Lane at Jul 23, 2009 at 11:02 pm

    Brendan Jurd writes:
    I've also done an initial review of the patch. Everything looks sane
    and the patch works as advertised. I made a couple of minor tweaks
    for code-style and comment consistency, and my version 3 is attached.
    I'm marking this patch Ready for Committer.
    Applied with minor revisions --- mostly, it leaked memory in the case
    of no comments, and the query wasn't very schema-safe.

    regards, tom lane
  • Bruce Momjian at Aug 8, 2009 at 6:10 am

    Tom Lane wrote:
    Brendan Jurd <direvus@gmail.com> writes:
    I've also done an initial review of the patch. Everything looks sane
    and the patch works as advertised. I made a couple of minor tweaks
    for code-style and comment consistency, and my version 3 is attached.
    I'm marking this patch Ready for Committer.
    Applied with minor revisions --- mostly, it leaked memory in the case
    of no comments, and the query wasn't very schema-safe.
    Just to verify, this patch was about comments on composite columns, not
    about dumping comments on index columns (as the subject states), right?
    We do have a TODO for index column comments:

    Forbid COMMENT on columns of an index

    Postgres currently allows comments to be placed on the columns of an
    index, but pg_dump doesn't handle them and the column names themselves
    are implementation-dependent.

    --
    Bruce Momjian <bruce@momjian.us> http://momjian.us
    EnterpriseDB http://enterprisedb.com

    + If your life is a hard drive, Christ can be your backup. +
  • Brendan Jurd at Aug 8, 2009 at 6:14 am

    2009/8/8 Bruce Momjian <bruce@momjian.us>:
    Just to verify, this patch was about comments on composite columns, not
    about dumping comments on index columns (as the subject states), right?
    We do have a TODO for index column comments:
    Correct.

    If you scroll up a couple of messages [1] you'll see that I added that
    TODO after I reviewed the patch.

    Cheers,
    BJ

    [1] 37ed240d0907212253peaab8abtf8fd58fbde44cfc2@mail.gmail.com

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
grouppgsql-hackers @
categoriespostgresql
postedMar 23, '09 at 8:15a
activeAug 8, '09 at 6:14a
posts14
users6
websitepostgresql.org...
irc#postgresql

People

Translate

site design / logo © 2022 Grokbase