| From: | Emre Hasegeli <emre(at)hasegeli(dot)com> |
|---|---|
| To: | Dagfinn Ilmari Mannsåker <ilmari(at)ilmari(dot)org> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Matthias Kurz <m(dot)kurz(at)irregular(dot)at>, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com> |
| Subject: | Re: [PATCH] Alter or rename enum value |
| Date: | 2016-08-21 13:49:16 |
| Message-ID: | CAE2gYzxo4bB_uQDafQYoGx=WmUrO-NFUbKBYfu4j_=LfaaUzFg@mail.gmail.com |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Thread: | |
| Lists: | 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().
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2016-08-21 14:28:18 | Re: [PATCH] Alter or rename enum value |
| Previous Message | Christoph Berg | 2016-08-21 12:16:31 | Re: synchronous_commit = remote_flush |