Skip site navigation (1) Skip section navigation (2)

Re: PL/PGSQL: Dynamic Record Introspection

From: Neil Conway <neilc(at)samurai(dot)com>
To: Titus von Boxberg <ut(at)bhi-hamburg(dot)de>
Cc: pgsql-patches(at)postgresql(dot)org
Subject: Re: PL/PGSQL: Dynamic Record Introspection
Date: 2005-07-14 06:12:11
Message-ID: 42D6023B.9050301@samurai.com (view raw or flat)
Thread:
Lists: pgsql-patches
Titus von Boxberg wrote:
> With the following patch it's possible to
> - extract all field names of a record into an array
> - extract field count of a record
> - address a single field of a record with a variable
>   containing the field name (additional to the usual record.fieldname
>   notation where the fieldname is hardcoded).

I wonder if this is the right syntax. record%identifier is doing 
something fundamentally different from record.identifier, but the syntax 
doesn't make that clear. I don't have any concrete suggestions for 
improvement, mind you... :)

> Test code for the patch can be extracted from an example I put into
> plpgsql.sgml

Can you supply some proper regression tests, please? i.e. patch 
sql/plpgsql.sql and expected/plpgsql.out in src/test/regress

A few minor comments from skimming the patch:

> ***************
> *** 995,1001 ****
>   
>   				new = palloc(sizeof(PLpgSQL_recfield));
>   				new->dtype = PLPGSQL_DTYPE_RECFIELD;
> ! 				new->fieldname = pstrdup(cp[1]);
>   				new->recparentno = ns->itemno;
>   
>   				plpgsql_adddatum((PLpgSQL_datum *) new);
> --- 995,1002 ----
>   
>   				new = palloc(sizeof(PLpgSQL_recfield));
>   				new->dtype = PLPGSQL_DTYPE_RECFIELD;
> ! 				new->fieldindex.fieldname = strdup(cp[1]);
> ! 				new->fieldindex_flag = RECFIELD_USE_FIELDNAME;
>   				new->recparentno = ns->itemno;
>   
>   				plpgsql_adddatum((PLpgSQL_datum *) new);

Should be pstrdup().

> ***************
> *** 1101,1107 ****
>   
>   				new = palloc(sizeof(PLpgSQL_recfield));
>   				new->dtype = PLPGSQL_DTYPE_RECFIELD;
> ! 				new->fieldname = pstrdup(cp[2]);
>   				new->recparentno = ns->itemno;
>   
>   				plpgsql_adddatum((PLpgSQL_datum *) new);
> --- 1102,1109 ----
>   
>   				new = palloc(sizeof(PLpgSQL_recfield));
>   				new->dtype = PLPGSQL_DTYPE_RECFIELD;
> ! 				new->fieldindex.fieldname = strdup(cp[2]);
> ! 				new->fieldindex_flag = RECFIELD_USE_FIELDNAME;
>   				new->recparentno = ns->itemno;
>   
>   				plpgsql_adddatum((PLpgSQL_datum *) new);

Ibid.

> + 	switch (ns1->itemtype)
> + 	{
> + 		case PLPGSQL_NSTYPE_REC:
> + 			{
> + 				PLpgSQL_recfieldproperties *new;
> + 
> + 				new = malloc(sizeof(PLpgSQL_recfieldproperties));
> + 				new->dtype = PLPGSQL_DTYPE_NRECFIELD;
> + 				new->recparentno = ns1->itemno;
> + 				plpgsql_adddatum((PLpgSQL_datum *) new);
> + 				plpgsql_yylval.scalar = (PLpgSQL_datum *) new;
> + 				ret =  T_SCALAR;	/* ??? */
> + 				break;

Should be palloc().

> + 	return ret;
> + } /* plpgsql_parse_wordnfields */

The Postgres convention is not to include comments like this.

> + 		case PLPGSQL_NSTYPE_REC:
> + 			{
> + 				PLpgSQL_recfieldproperties *new;
> + 
> + 				new = malloc(sizeof(PLpgSQL_recfieldproperties));
> + 				new->dtype = PLPGSQL_DTYPE_RECFIELDNAMES;
> + 				new->recparentno = ns1->itemno;
> + 				plpgsql_adddatum((PLpgSQL_datum *) new);
> + 				plpgsql_yylval.scalar = (PLpgSQL_datum *) new;
> + 				ret =  T_SCALAR;	/* ??? */
> + 				break;
> + 			}

Should be palloc().

> ! 				/* construct_array copies data; free temp elem array */
> ! #if	0
> ! 				for ( fc = 0; fc < tfc; ++fc )
> ! 					pfree(DatumGetPointer(arrayelems[fc]);
> ! 				pfree(arrayelems);
> ! #endif

Unexplained #if 0 blocks aren't good.

-Neil

In response to

Responses

pgsql-patches by date

Next:From: Neil ConwayDate: 2005-07-14 06:18:05
Subject: Re: Final cleanup of SQL:1999 references
Previous:From: Jeffrey W. BakerDate: 2005-07-14 00:33:45
Subject: Re: [PATCHES] O_DIRECT for WAL writes

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group