Re: [PATCH] Alter or rename enum value - Mailing list pgsql-hackers

From Emre Hasegeli
Subject Re: [PATCH] Alter or rename enum value
Date
Msg-id CAE2gYzxo4bB_uQDafQYoGx=WmUrO-NFUbKBYfu4j_=LfaaUzFg@mail.gmail.com
Whole thread Raw
In response to Re: [PATCH] Alter or rename enum value  (ilmari@ilmari.org (Dagfinn Ilmari Mannsåker))
Responses Re: [PATCH] Alter or rename enum value  (Tom Lane <tgl@sss.pgh.pa.us>)
Re: [PATCH] Alter or rename enum value  (ilmari@ilmari.org (Dagfinn Ilmari Mannsåker))
List pgsql-hackers
> Here is v4, which changes the command from ALTER VALUE to RENAME VALUE,
> for consistency with RENAME ATTRIBUTE.

It looks like we always use "ALTER ... RENAME ... old_name TO
new_name" syntax, so it is better that way.  I have noticed that all
the other RENAMEs go through ExecRenameStmt().  We better do the same.

> +    int         nbr_index;
> +    Form_pg_enum nbr_en;

What is "nbr"?  Why not calling them something like "i" and "val"?

> +   /* Locate the element to rename and check if the new label is already in

The project style for multi-line commands is to leave the first line
of the comment block empty.

> +        if (strcmp(NameStr(nbr_en->enumlabel), oldVal) == 0)

I found namestrcmp() for this.

> +    }
> +    if (!old_tup)

I would leave a space in between.

> +        ReleaseCatCacheList(list);
> +        heap_close(pg_enum, RowExclusiveLock);

Maybe we better release them before reporting error, too.  I would
release the list after the loop, close the heap before ereport().



pgsql-hackers by date:

Previous
From: Christoph Berg
Date:
Subject: Re: synchronous_commit = remote_flush
Next
From: Tom Lane
Date:
Subject: Re: [PATCH] Alter or rename enum value