[PATCH] Cleanup of PLpgSQL_recfield

"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