Обсуждение: [PATCH] Cleanup of PLpgSQL_recfield
While looking to add some functionality to PL/pgSQL, I found that the rfno member of the PLpgSQL_recfield structure is unused. This patch is just a cleanup and doesn't seem along the same lines as the patches in CommitFest... should I add it to the wiki anyway? -- Jonah H. Harris, Senior DBA myYearbook.com
Вложения
"Jonah H. Harris" <jonah.harris@gmail.com> writes: > While looking to add some functionality to PL/pgSQL, I found that the > rfno member of the PLpgSQL_recfield structure is unused. This patch > is just a cleanup No, that'd be wrong. Note here: /** PLpgSQL_datum is the common supertype for PLpgSQL_expr, PLpgSQL_var,* PLpgSQL_row, PLpgSQL_rec, PLpgSQL_recfield, PLpgSQL_arrayelem,and* PLpgSQL_trigarg*/ typedef struct { /* Generic datum array item */ int dtype; int dno; } PLpgSQL_datum; I am not real sure why the code is inconsistent about spelling the second field's name differently in some of the structs, but it seems like a bad idea --- as you've demonstrated, it invites confusion. What would probably be better is a patch to rename exprno, rfno, etc to all be called dno to make this connection more obvious. regards, tom lane
Jonah H. Harris escribió: > While looking to add some functionality to PL/pgSQL, I found that the > rfno member of the PLpgSQL_recfield structure is unused. This patch > is just a cleanup and doesn't seem along the same lines as the patches > in CommitFest... should I add it to the wiki anyway? Nah -- I just applied it. Thanks. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Tom Lane escribió: > "Jonah H. Harris" <jonah.harris@gmail.com> writes: > > While looking to add some functionality to PL/pgSQL, I found that the > > rfno member of the PLpgSQL_recfield structure is unused. This patch > > is just a cleanup > > No, that'd be wrong. Oops. Reverting. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Tue, Nov 4, 2008 at 3:57 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > I am not real sure why the code is inconsistent about spelling the > second field's name differently in some of the structs, but it seems > like a bad idea --- as you've demonstrated, it invites confusion. > What would probably be better is a patch to rename exprno, rfno, etc > to all be called dno to make this connection more obvious. Attached. Passed regressions and basic testing. -- Jonah H. Harris, Senior DBA myYearbook.com
Вложения
"Jonah H. Harris" <jonah.harris@gmail.com> writes: > On Tue, Nov 4, 2008 at 3:57 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> I am not real sure why the code is inconsistent about spelling the >> second field's name differently in some of the structs, but it seems >> like a bad idea --- as you've demonstrated, it invites confusion. >> What would probably be better is a patch to rename exprno, rfno, etc >> to all be called dno to make this connection more obvious. > Attached. Passed regressions and basic testing. Looks good, applied. regards, tom lane