PL/PGSQL: Dynamic Record Introspection

Lists: pgsql-patches
From: Titus von Boxberg <ut(at)bhi-hamburg(dot)de>
To: pgsql-patches(at)postgresql(dot)org
Subject: PL/PGSQL: Dynamic Record Introspection
Date: 2005-07-14 00:32:23
Message-ID: 42D5B297.7090804@bhi-hamburg.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches

Hi all,

I needed introspection capabilities for record types to write more generic
trigger procedures in PL/PGSQL.

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).

The syntax is
- record%NFIELDS gives the number of fields in the record
- record%FIELDNAMES gives the array of the field names
- record%scalarvariable extracts the field whose name
is equal to the contents of scalarvariable

------------

The patch is nonintrusive in the sense that it only adds things
with one exception:
In function exec_eval_datum(), file pl_exec.c, line 3557 (after the patch)
I chose to convert the record field values to TEXT if the caller does
not require a certain type (expectedtypeid == InvalidOid).
Additionally, I cast the value from the record field type if
the destination Datum is of different type.

As far as I can see, this does no harm because in most cases
the expectedtypeid is correctly set. I just wanted to avoid that
if it is not set, the returned datum is of a more restrictive type
than TEXT.

------------

The patch is against a HEAD checkout from 07/12/05
The output comes from difforig.

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

------------

Here is a summary of things that get patched by the file:
- add three new special parsing functions to pl_comp.c
(plpgsql_parse_wordpercentword, plpgsql_parse_wordnfields,
plpgsql_parse_wordfieldnames).
- modify PLpgSQL_recfield in plpgsql.h to either hold
a conventional field name (record.fieldname) or a dno
for the variable (record%variable).
- add two PLPGSQL_DTYPEs for the two new % notations
- modify "case PLPGSQL_DTYPE_RECFIELD:"
in exec_eval_datum() and exec_assign_value()
to deal with index strings from a variable
- add "case PLPGSQL_DTYPE_RECFIELDNAMES"
and "case PLPGSQL_DTYPE_NRECFIELD"
to exec_eval_datum() to evaluate %FIELDNAMES
and %NFIELDS expressions.
- update plpgsql.sgml in the docs directory

------------

Please notify me if I can be of further assistance.

Regards
Titus

Attachment Content-Type Size
assocrec-current.patch text/plain 20.7 KB

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
Views: Raw Message | Whole Thread | Download mbox | Resend email
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


From: Pavel Stehule <stehule(at)kix(dot)fsv(dot)cvut(dot)cz>
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:38:32
Message-ID: Pine.LNX.4.44.0507140836300.31492-100000@kix.fsv.cvut.cz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches

good idea. it's what can eliminate not neccessery using plperl. I would to
see it in plpgsql.

regards
Pavel

On Thu, 14 Jul 2005, Titus von Boxberg wrote:

> Hi all,
>
> I needed introspection capabilities for record types to write more generic
> trigger procedures in PL/PGSQL.
>
> 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).
>
> The syntax is
> - record%NFIELDS gives the number of fields in the record
> - record%FIELDNAMES gives the array of the field names
> - record%scalarvariable extracts the field whose name
> is equal to the contents of scalarvariable
>
> ------------
>
> The patch is nonintrusive in the sense that it only adds things
> with one exception:
> In function exec_eval_datum(), file pl_exec.c, line 3557 (after the patch)
> I chose to convert the record field values to TEXT if the caller does
> not require a certain type (expectedtypeid == InvalidOid).
> Additionally, I cast the value from the record field type if
> the destination Datum is of different type.
>
> As far as I can see, this does no harm because in most cases
> the expectedtypeid is correctly set. I just wanted to avoid that
> if it is not set, the returned datum is of a more restrictive type
> than TEXT.
>
> ------------
>
> The patch is against a HEAD checkout from 07/12/05
> The output comes from difforig.
>
> Test code for the patch can be extracted from an example I put into
> plpgsql.sgml
>
> ------------
>
> Here is a summary of things that get patched by the file:
> - add three new special parsing functions to pl_comp.c
> (plpgsql_parse_wordpercentword, plpgsql_parse_wordnfields,
> plpgsql_parse_wordfieldnames).
> - modify PLpgSQL_recfield in plpgsql.h to either hold
> a conventional field name (record.fieldname) or a dno
> for the variable (record%variable).
> - add two PLPGSQL_DTYPEs for the two new % notations
> - modify "case PLPGSQL_DTYPE_RECFIELD:"
> in exec_eval_datum() and exec_assign_value()
> to deal with index strings from a variable
> - add "case PLPGSQL_DTYPE_RECFIELDNAMES"
> and "case PLPGSQL_DTYPE_NRECFIELD"
> to exec_eval_datum() to evaluate %FIELDNAMES
> and %NFIELDS expressions.
> - update plpgsql.sgml in the docs directory
>
> ------------
>
> Please notify me if I can be of further assistance.
>
> Regards
> Titus
>


From: Titus von Boxberg <ut(at)bhi-hamburg(dot)de>
To: Neil Conway <neilc(at)samurai(dot)com>
Cc: pgsql-patches(at)postgresql(dot)org
Subject: Re: PL/PGSQL: Dynamic Record Introspection
Date: 2005-07-14 14:54:07
Message-ID: 42D67C8F.6060502@bhi-hamburg.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches

Neil Conway schrieb:
> 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... :)
What do you mean by "right syntax". There are hundreds of examples
in programming languages where a "small" syntactic difference leads to totally
different semantics.
I chose % because
- it's already used in the var%TYPE notation. Here %xxx stands for
a kind of extracting operator, too. In this sense, %TYPE's operation is similar
to variables what %NFIELDS, %FIELDNAMES and %variable do to records.
- I could not think of any other operator that would not be harmful
for the parser (or cause much work) and also would not lead to
other ambiguities in the sense not being "right".
- % strongly differs optically from .

-------------------

>
> Can you supply some proper regression tests, please? i.e. patch
> sql/plpgsql.sql and expected/plpgsql.out in src/test/regress
I'll do that.
>
> A few minor comments from skimming the patch:
I'll implement your suggestions.

I'll also remove the changes to the RECFIELD value evaluation semantics
completely that I introduced.

--------------------

Are there any general objections to implement this feature?
Personally, I would very much like to see it implemented because
I do want to use plpgsql and without this feature (trigger)
procedures cannot be held general if dealing with structural
similarities between tables they are invoked upon.

I'd send the patched patch when I'm done.

Regards
Titus


From: Titus von Boxberg <boxberg(at)pleach(dot)de>
To: Neil Conway <neilc(at)samurai(dot)com>
Cc: pgsql-patches(at)postgresql(dot)org
Subject: Re: PL/PGSQL: Dynamic Record Introspection
Date: 2005-07-14 22:04:25
Message-ID: 42D6E169.8030909@pleach.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches

Neil Conway schrieb:
> Titus von Boxberg wrote:
>
> Can you supply some proper regression tests, please? i.e. patch
> sql/plpgsql.sql and expected/plpgsql.out in src/test/regress
In sql/plpgsql.sql I have added a function testing the new features
and altered expected/plpgsql.out

>
> A few minor comments from skimming the patch:
Done.

------------------

The modifications to the first one are the following:
- restored type evaluation in RECFIELD value evaluation
in exec_eval_datum to the previous semantics.
The addition should now be completely invisible to existing code.
- instead do a check in exec_eval_expr() if the expression
contains datums of the type record%variable. For these
the type cannot be stored in a plan because the type might change
between evaluations of this expr. --> free and reprepare the plan.
- storage allocated by exec_eval_expr() for the expression
record%FIELDNAMES (the array containing the names)
is freed in exec_eval_cleanup

------------------

regression tests are ok on my system.

Do you agree with the mods and the patch?

Regards
Titus

Attachment Content-Type Size
assocrec-current3.patch text/plain 24.7 KB

From: Titus von Boxberg <ut(at)bhi-hamburg(dot)de>
To: pgsql-patches(at)postgresql(dot)org
Subject: Re: PL/PGSQL: Dynamic Record Introspection
Date: 2005-07-14 22:26:43
Message-ID: 42D6E6A3.3050108@bhi-hamburg.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches

Neil Conway schrieb:

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

In sql/plpgsql.sql I have added a function testing the new features
and altered expected/plpgsql.out

>
> A few minor comments from skimming the patch:

Done.

------------------

The modifications to the first one are the following:
- restored type evaluation in RECFIELD value evaluation
in exec_eval_datum to the previous semantics.
The addition should now be completely invisible to existing code.
- instead do a check in exec_eval_expr() if the expression
contains datums of the type record%variable. For these
the type cannot be stored in a plan because the type might change
between evaluations of this expr. --> free and reprepare the plan.
- storage allocated by exec_eval_expr() for the expression
record%FIELDNAMES (the array containing the names)
is freed in exec_eval_cleanup

------------------

regression tests are ok on my system.

Do you agree with the mods and the patch?

Regards
Titus

Attachment Content-Type Size
assocrec-current3.patch text/plain 24.7 KB

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-15 09:06:08
Message-ID: 42D77C80.9060207@samurai.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches

Titus von Boxberg wrote:
> What do you mean by "right syntax". There are hundreds of examples
> in programming languages where a "small" syntactic difference leads to
> totally different semantics.

Well, I'm just not happy that "foo.bar" means something completely
different than "foo%bar" -- foo%bar isn't even fetching something called
"bar" from "foo". Anyway, I can't see a better syntax, so I suppose this
will do.

> Are there any general objections to implement this feature?

Not from me. You missed the 8.1 freeze by about two weeks, so I'm not
sure if this is a candidate for 8.1 though.

-Neil


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Neil Conway <neilc(at)samurai(dot)com>
Cc: Titus von Boxberg <ut(at)bhi-hamburg(dot)de>, pgsql-patches(at)postgresql(dot)org
Subject: Re: PL/PGSQL: Dynamic Record Introspection
Date: 2005-07-15 14:20:01
Message-ID: 11161.1121437201@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches

Neil Conway <neilc(at)samurai(dot)com> writes:
> Not from me. You missed the 8.1 freeze by about two weeks, so I'm not
> sure if this is a candidate for 8.1 though.

Not a chance.

regards, tom lane


From: "Titus von Boxberg" <ut(at)bhi-hamburg(dot)de>
To: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Neil Conway" <neilc(at)samurai(dot)com>
Cc: <pgsql-patches(at)postgresql(dot)org>
Subject: Re: PL/PGSQL: Dynamic Record Introspection
Date: 2005-07-15 14:53:31
Message-ID: CCEMIPDGKNLDOCEIDHEFKEBPCAAA.ut@bhi-hamburg.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches

> -----Ursprüngliche Nachricht-----
> Von: Tom Lane [mailto:tgl(at)sss(dot)pgh(dot)pa(dot)us]
> Neil Conway <neilc(at)samurai(dot)com> writes:
> > Not from me. You missed the 8.1 freeze by about two weeks, so I'm not
> > sure if this is a candidate for 8.1 though.
>
> Not a chance.
>
> regards, tom lane

I'd like to participate in postgresql development on a time to time basis
as I continue using it and as my knowledge (hopefully) grows.

Having read the FAQ, it's not clear to me, where the patches go (and when)
that I would send into this list.
These are my open questions:

Are the patches applied to HEAD?
And only Very Important Patches are merged to the current or
even an older release branch?

Main background questions are:
What else (besides creating and testing the patch myself) can I do to
alleviate applying (or deciding to apply) the patches?
Could I expect (and if so in what time frame) the patches to appear in HEAD?
So at some time I would find my code in a cvs up?
How do I know that?

Thanks for answering newbie questions.

Regards
Titus


From: Neil Conway <neilc(at)samurai(dot)com>
To: Titus von Boxberg <ut(at)bhi-hamburg(dot)de>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-patches(at)postgresql(dot)org
Subject: Re: PL/PGSQL: Dynamic Record Introspection
Date: 2005-07-15 15:01:11
Message-ID: 42D7CFB7.2020003@samurai.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches

Titus von Boxberg wrote:
> Having read the FAQ, it's not clear to me, where the patches go (and when)
> that I would send into this list.
> These are my open questions:
>
> Are the patches applied to HEAD?

It depends on the patch. New features are almost exclusively only
applied to HEAD, whereas patches that fix bugs (with a low chance of
introducing additional regressions) or fix typos in the documentation
are applied to back branches.

> And only Very Important Patches are merged to the current or
> even an older release branch?

Right.

> Main background questions are:
> What else (besides creating and testing the patch myself) can I do to
> alleviate applying (or deciding to apply) the patches?
> Could I expect (and if so in what time frame) the patches to appear in HEAD?
> So at some time I would find my code in a cvs up?

Assuming the patch is accepted, it will eventually be reviewed and
applied by one of the committers. The time frame for doing that depends
on various factors: the size and complexity of the patch, the amount of
review / fixing it needs, the workload of the committers, and the
current stage of the release cycle (we just passed feature freeze for
8.1, so I think most committers are busy with work for the upcoming 8.1
beta release). Unfortunately we can be a bit tardy in applying patches
at times, IMHO.

> How do I know that?

It is standard practice to send email to patch submitters when their
patch has been applied (or alternatively rejected, or a resubmission of
the patch has been requested). You can also read the pgsql-committers
mailing list, which logs all the CVS commits.

-Neil


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Neil Conway <neilc(at)samurai(dot)com>
Cc: Titus von Boxberg <ut(at)bhi-hamburg(dot)de>, pgsql-patches(at)postgresql(dot)org
Subject: Re: PL/PGSQL: Dynamic Record Introspection
Date: 2005-07-18 05:32:38
Message-ID: 25146.1121664758@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches

Neil Conway <neilc(at)samurai(dot)com> writes:
> Titus von Boxberg wrote:
>> What do you mean by "right syntax". There are hundreds of examples
>> in programming languages where a "small" syntactic difference leads to
>> totally different semantics.

> Well, I'm just not happy that "foo.bar" means something completely
> different than "foo%bar" -- foo%bar isn't even fetching something called
> "bar" from "foo".

There's a worse objection, which is that % is a perfectly valid operator
name (typically defined as modulo). The patch is therefore usurping
syntax of an existing feature, which is surely Right Out.

Another small problem with the patch is the untenable assumption that
neither FIELDNAMES nor NFIELDS is ever used by anyone as an actual field
name.

Assuming that we're inventing in a green field, I'd suggest a syntax
along this line:

recordvalue.(stringexpression)

which is not confusable with any currently-valid syntax. I'm not sure
what to do about FIELDNAMES --- perhaps this would work:

recordvalue.(*)

As for NFIELDS, I don't think we need it -- you can always measure the
length of the FIELDNAMES array.

But the real $64 question for plpgsql stuff is "what does Oracle do?"
Is there any remotely comparable functionality in PL/SQL, and if so
what does it look like?

regards, tom lane


From: "Titus von Boxberg" <ut(at)bhi-hamburg(dot)de>
To: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Neil Conway" <neilc(at)samurai(dot)com>
Cc: <pgsql-patches(at)postgresql(dot)org>
Subject: Re: PL/PGSQL: Dynamic Record Introspection
Date: 2005-07-18 22:11:13
Message-ID: FGEHIMPCMPIPCBNNFENLAEMHCDAA.ut@bhi-hamburg.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches

> -----Ursprüngliche Nachricht-----
> Von: Tom Lane
> An: Neil Conway
> Cc: Titus von Boxberg
> Betreff: Re: [PATCHES] PL/PGSQL: Dynamic Record Introspection
>
-------------------

> There's a worse objection, which is that % is a perfectly valid operator
OK. I did not recognize that.

> Another small problem with the patch is the untenable assumption that
> neither FIELDNAMES nor NFIELDS is ever used by anyone as an actual field
> name.
No. You cannot name a variable "NFIELDS" or "FIELDNAMES" that you want to
use
in this expression. The record field names themselves are unaffected.

> As for NFIELDS, I don't think we need it -- you can always measure the
> length of the FIELDNAMES array.
I would like to leave it there. As far as I can see it's much faster
than other builtins for evaluating array dimensions and I think it does
not hurt.
What about
recordvalue.(#)
staying with your syntax?

>
> But the real $64 question for plpgsql stuff is "what does Oracle do?"
I don't have much experience with Oracle. From what I know, Oracle PL/SQL
lacks the concept of records with dynamically assigned structure.
This would make this construct meaningless.
Same for MS SQL. Don't know anything about DB2.

-------------------------

Questions:
- could anyone who knows Oracle better than me confirm
that with Oracle there are no RECORD variables of varying
dynamically assigned type? And thus there is no
construct dyamically acessing the fields of a RECORD type?
- is the syntax RECORD.(identifier), RECORD.(*), RECORD.(#)
still acceptable? I don't have any objections, but
as I already overlooked the modulo stuff
I'd assume my jugdement will not be authoritative :-)
- do you agree with my approach that "identifier"
is restricted to be a variable and cannot be an arbitrary
expression evaluating to a string?
- do you accept the NFIELDS construct, whatever it's
syntax finally would look like? Or do you want it
finally to be removed.

I'd then modify the code to the new syntax.

Regards
Titus


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "Titus von Boxberg" <ut(at)bhi-hamburg(dot)de>
Cc: "Neil Conway" <neilc(at)samurai(dot)com>, pgsql-patches(at)postgresql(dot)org
Subject: Re: PL/PGSQL: Dynamic Record Introspection
Date: 2005-07-19 03:10:37
Message-ID: 18889.1121742637@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches

"Titus von Boxberg" <ut(at)bhi-hamburg(dot)de> writes:
> Questions:
> - could anyone who knows Oracle better than me confirm
> that with Oracle there are no RECORD variables of varying
> dynamically assigned type?

Anyone?

> - is the syntax RECORD.(identifier), RECORD.(*), RECORD.(#)
> still acceptable?

It works for me if we want to have an "NFIELDS" construct. Personally
I'm still not convinced that we need one --- what's the use-case?

> - do you agree with my approach that "identifier"
> is restricted to be a variable and cannot be an arbitrary
> expression evaluating to a string?

I'd prefer arbitrary expression, but I suppose there's no harm in doing
the simple case first and generalizing if there's demand.

regards, tom lane


From: Neil Conway <neilc(at)samurai(dot)com>
To: Titus von Boxberg <ut(at)bhi-hamburg(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: <pgsql-patches(at)postgresql(dot)org>
Subject: Re: PL/PGSQL: Dynamic Record Introspection
Date: 2005-07-19 04:15:35
Message-ID: BF02BB87.14DF%neilc@samurai.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches

On 7/19/05 8:11 AM, "Titus von Boxberg" <ut(at)bhi-hamburg(dot)de> wrote:
>> As for NFIELDS, I don't think we need it -- you can always measure the
>> length of the FIELDNAMES array.
> I would like to leave it there. As far as I can see it's much faster
> than other builtins for evaluating array dimensions and I think it does
> not hurt.

Is performance a concern here? (Considering that you needn't repeatedly get
the length of the array as you can store it in a local variable.) I agree
with Tom, we can probably do without this.

-Neil


From: Titus von Boxberg <ut(at)bhi-hamburg(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Neil Conway <neilc(at)samurai(dot)com>, pgsql-patches(at)postgresql(dot)org
Subject: Re: PL/PGSQL: Dynamic Record Introspection
Date: 2005-07-24 10:06:39
Message-ID: 42E3682F.1080307@bhi-hamburg.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches

Tom Lane schrieb:
> "Titus von Boxberg" <ut(at)bhi-hamburg(dot)de> writes:
> It works for me if we want to have an "NFIELDS" construct. Personally
> I'm still not convinced that we need one --- what's the use-case?
I have removed the NFIELDS construct
>
>
> I'd prefer arbitrary expression, but I suppose there's no harm in doing
> the simple case first and generalizing if there's demand.
I took the "no harm" way.

Attached please find the updated patch.
The patch is against HEAD of 050721.

I switched the syntax to your proposal, renamed the functions
in pl_comp.c and updated the sgml doc source and regression
test files accordingly.

Regards
Titus

Attachment Content-Type Size
ar-current050720.patch text/plain 22.5 KB

From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Titus von Boxberg <ut(at)bhi-hamburg(dot)de>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Neil Conway <neilc(at)samurai(dot)com>, pgsql-patches(at)postgresql(dot)org
Subject: Re: PL/PGSQL: Dynamic Record Introspection
Date: 2005-07-31 02:36:05
Message-ID: 200507310236.j6V2a5g04335@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches


This has been saved for the 8.2 release:

http://momjian.postgresql.org/cgi-bin/pgpatches_hold

---------------------------------------------------------------------------

Titus von Boxberg wrote:
> Tom Lane schrieb:
> > "Titus von Boxberg" <ut(at)bhi-hamburg(dot)de> writes:
> > It works for me if we want to have an "NFIELDS" construct. Personally
> > I'm still not convinced that we need one --- what's the use-case?
> I have removed the NFIELDS construct
> >
> >
> > I'd prefer arbitrary expression, but I suppose there's no harm in doing
> > the simple case first and generalizing if there's demand.
> I took the "no harm" way.
>
> Attached please find the updated patch.
> The patch is against HEAD of 050721.
>
> I switched the syntax to your proposal, renamed the functions
> in pl_comp.c and updated the sgml doc source and regression
> test files accordingly.
>
> Regards
> Titus
>

> *** ./doc/src/sgml/plpgsql.sgml.orig Sat Jul 2 08:59:47 2005
> --- ./doc/src/sgml/plpgsql.sgml Sat Jul 23 17:24:54 2005
> ***************
> *** 867,872 ****
> --- 867,921 ----
> </para>
>
> <para>
> + To obtain the values of the fields the record is made up of,
> + the record variable can be qualified with the column or field
> + name. This can be done either by literally using the column name
> + or the column name for indexing the record can be taken out of a scalar
> + variable. The syntax for this notation is Record_variable.(IndexVariable).
> + To get information about the column field names of the record,
> + a
> special expression exists that returns all column names as an array:
> + RecordVariable.(*) .
> + Thus, the RECORD can be viewed
> + as an associative array that allows for introspection of it's contents.
> + This feature is especially useful for writing generic triggers that
> + operate on records with unknown structure.
> + Here is an example procedure that shows column names and values
> + of the predefined record NEW in a trigger procedure:
> + <programlisting>
> +
> + CREATE OR REPLACE FUNCTION show_associative_records() RETURNS TRIGGER AS $$
> + DECLARE
> + colname TEXT;
> + colcontent TEXT;
> + colnames TEXT[];
> + coln INT4;
> + coli INT4;
> + BEGIN
> + -- obtain an array with all field names of the record
> + colnames := NEW.(*);
> + RAISE NOTICE 'All column names of test record: %', colnames;
> + -- show field names and contents of record
> + coli := 1;
> + coln := array_upper(colnames,1);
> + RAISE NOTICE 'Number of columns in NEW: %', coln;
> + FOR coli IN 1 .. coln LOOP
> + colname := colnames[coli];
> + colcontent := NEW.(colname);
> + RAISE NOTICE 'column % of NEW: %', quote_ident(colname), quote_literal(colcontent);
> + END LOOP;
> + -- Do it with a fixed field name:
> + -- will have to know the column name
> + RAISE NOTICE 'column someint of NEW: %', quote_literal(NEW.someint);
> + RETURN NULL;
> + END;
> + $$ LANGUAGE plpgsql;
> + --CREATE TABLE test_records (someint INT8, somestring TEXT);
> + --CREATE TRIGGER tr_test_record BEFORE INSERT ON test_records FOR EACH ROW EXECUTE PROCEDURE show_associative_records();
> +
> + </programlisting>
> + </para>
> +
> + <para>
> Note that <literal>RECORD</> is not a true data type, only a placeholder.
> One should also realize that when a <application>PL/pgSQL</application>
> function is declared to return type <type>record</>, this is not quite the
> *** ./src/pl/plpgsql/src/pl_comp.c.orig Wed Jul 6 16:42:10 2005
> --- ./src/pl/plpgsql/src/pl_comp.c Thu Jul 21 21:28:15 2005
> ***************
> *** 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 = pstrdup(cp[1]);
> ! new->fieldindex_flag = RECFIELD_USE_FIELDNAME;
> new->recparentno = ns->itemno;
>
> plpgsql_adddatum((PLpgSQL_datum *) new);
> ***************
> *** 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 = pstrdup(cp[2]);
> ! new->fieldindex_flag = RECFIELD_USE_FIELDNAME;
> new->recparentno = ns->itemno;
>
> plpgsql_adddatum((PLpgSQL_datum *) new);
> ***************
> *** 1550,1555 ****
> --- 1552,1683 ----
> MemoryContextSwitchTo(oldCxt);
> return T_DTYPE;
> }
> +
> + /* ----------
> + * plpgsql_parse_recindex
> + * lookup associative index into record
> + * ----------
> + */
> + int
> + plpgsql_parse_recindex(char *word)
> + {
> + PLpgSQL_nsitem *ns1, *ns2;
> + char *cp[2];
> + int ret = T_ERROR;
> + char *fieldvar;
> + int fl;
> +
> + /* Do case conversion and word separation */
> + plpgsql_convert_ident(word, cp, 2);
> + Assert(cp[1] != NULL);
> +
> + /* cleanup the "(identifier)" string to "identifier" */
> + fieldvar = cp[1];
> + Assert(*fieldvar == '(');
> + ++fieldvar; /* get rid of ( */
> +
> + fl = strlen(fieldvar);
> + Assert(fieldvar[fl-1] == ')');
> + fieldvar[fl-1] = 0; /* get rid of ) */
> +
> + /*
> + * Lookup the first word
> + */
> + ns1 = plpgsql_ns_lookup(cp[0], NULL);
> + if ( ns1 == NULL )
> + {
> + pfree(cp[0]);
> + pfree(cp[1]);
> + return T_ERROR;
> + }
> +
> + ns2 = plpgsql_ns_lookup(fieldvar, NULL);
> + pfree(cp[0]);
> + pfree(cp[1]);
> + if ( ns2 == NULL ) /* name lookup failed */
> + return T_ERROR;
> +
> + switch (ns1->itemtype)
> + {
> + case PLPGSQL_NSTYPE_REC:
> + {
> + /*
> + * First word is a record name, so second word must be an
> + * variable holding the field name in this record.
> + */
> + if ( ns2->itemtype == PLPGSQL_NSTYPE_VAR ) {
> + PLpgSQL_recfield *new;
> +
> + new = palloc(sizeof(PLpgSQL_recfield));
> + new->dtype = PLPGSQL_DTYPE_RECFIELD;
> + new->fieldindex.indexvar_no = ns2->itemno;
> + new->fieldindex_flag = RECFIELD_USE_INDEX_VAR;
> + new->recparentno = ns1->itemno;
> +
> + plpgsql_adddatum((PLpgSQL_datum *) new);
> +
> + plpgsql_yylval.scalar = (PLpgSQL_datum *) new;
> + ret = T_SCALAR;
> + }
> + break;
> + }
> + default:
> + break;
> + }
> + return ret;
> + }
> +
> +
> + /* ----------
> + * plpgsql_parse_recfieldnames
> + * create fieldnames of a record
> + * ----------
> + */
> + int
> + plpgsql_parse_recfieldnames(char *word)
> + {
> + PLpgSQL_nsitem *ns1;
> + char *cp[2];
> + int ret = T_ERROR;
> +
> + /* Do case conversion and word separation */
> + plpgsql_convert_ident(word, cp, 2);
> +
> + /*
> + * Lookup the first word
> + */
> + ns1 = plpgsql_ns_lookup(cp[0], NULL);
> + if ( ns1 == NULL )
> + {
> + pfree(cp[0]);
> + pfree(cp[1]);
> + return T_ERROR;
> + }
> +
> + pfree(cp[0]);
> + pfree(cp[1]);
> +
> + switch (ns1->itemtype)
> + {
> + case PLPGSQL_NSTYPE_REC:
> + {
> + PLpgSQL_recfieldproperties *new;
> +
> + new = palloc(sizeof(PLpgSQL_recfieldproperties));
> + new->dtype = PLPGSQL_DTYPE_RECFIELDNAMES;
> + new->recparentno = ns1->itemno;
> + new->save_fieldnames = NULL;
> + plpgsql_adddatum((PLpgSQL_datum *) new);
> + plpgsql_yylval.scalar = (PLpgSQL_datum *) new;
> + ret = T_SCALAR; /* ??? */
> + break;
> + }
> + default:
> + break;
> + }
> + return ret;
> + }
> +
>
> /*
> * plpgsql_build_variable - build a datum-array entry of a given
> *** ./src/pl/plpgsql/src/pl_exec.c.orig Sun Jun 26 22:05:42 2005
> --- ./src/pl/plpgsql/src/pl_exec.c Sat Jul 23 17:13:32 2005
> ***************
> *** 716,721 ****
> --- 716,722 ----
> case PLPGSQL_DTYPE_RECFIELD:
> case PLPGSQL_DTYPE_ARRAYELEM:
> case PLPGSQL_DTYPE_TRIGARG:
> + case PLPGSQL_DTYPE_RECFIELDNAMES:
> /*
> * These datum records are read-only at runtime, so no need
> * to copy them
> ***************
> *** 825,830 ****
> --- 826,832 ----
>
> case PLPGSQL_DTYPE_RECFIELD:
> case PLPGSQL_DTYPE_ARRAYELEM:
> + case PLPGSQL_DTYPE_RECFIELDNAMES:
> break;
>
> default:
> ***************
> *** 2146,2151 ****
> --- 2148,2155 ----
> static void
> exec_eval_cleanup(PLpgSQL_execstate *estate)
> {
> + int i;
> + ArrayType *a;
> /* Clear result of a full SPI_execute */
> if (estate->eval_tuptable != NULL)
> SPI_freetuptable(estate->eval_tuptable);
> ***************
> *** 2154,2159 ****
> --- 2158,2171 ----
> /* Clear result of exec_eval_simple_expr (but keep the econtext) */
> if (estate->eval_econtext != NULL)
> ResetExprContext(estate->eval_econtext);
> + for ( i = 0; i < estate->ndatums; ++i ) {
> + if ( estate->datums[i]->dtype == PLPGSQL_DTYPE_RECFIELDNAMES ) {
> + a = ((PLpgSQL_recfieldproperties *)(estate->datums[i]))->save_fieldnames;
> + if ( a )
> + pfree(a);
> + ((PLpgSQL_recfieldproperties *)(estate->datums[i]))->save_fieldnames = NULL;
> + }
> + }
> }
>
>
> ***************
> *** 3154,3165 ****
> * Get the number of the records field to change and the
> * number of attributes in the tuple.
> */
> ! fno = SPI_fnumber(rec->tupdesc, recfield->fieldname);
> ! if (fno == SPI_ERROR_NOATTRIBUTE)
> ! ereport(ERROR,
> ! (errcode(ERRCODE_UNDEFINED_COLUMN),
> ! errmsg("record \"%s\" has no field \"%s\"",
> ! rec->refname, recfield->fieldname)));
> fno--;
> natts = rec->tupdesc->natts;
>
> --- 3166,3200 ----
> * Get the number of the records field to change and the
> * number of attributes in the tuple.
> */
> ! if ( recfield->fieldindex_flag == RECFIELD_USE_FIELDNAME ) {
> ! fno = SPI_fnumber(rec->tupdesc, recfield->fieldindex.fieldname);
> ! if (fno == SPI_ERROR_NOATTRIBUTE)
> ! ereport(ERROR,
> ! (errcode(ERRCODE_UNDEFINED_COLUMN),
> ! errmsg("record \"%s\" has no field \"%s\"",
> ! rec->refname, recfield->fieldindex.fieldname)));
> ! }
> ! else if ( recfield->fieldindex_flag == RECFIELD_USE_INDEX_VAR ) {
> ! PLpgSQL_var * idxvar = (PLpgSQL_var *) (estate->datums[recfield->fieldindex.indexvar_no]);
> ! char * fname = convert_value_to_string(idxvar->value, idxvar->datatype->typoid);
> ! if ( fname == NULL )
> ! ereport(ERROR,
> ! (errcode(ERRCODE_UNDEFINED_COLUMN),
> ! errmsg("record \"%s\": cannot evaluate variable to record index string",
> ! rec->refname)));
> ! fno = SPI_fnumber(rec->tupdesc, fname);
> ! pfree(fname);
> ! if (fno == SPI_ERROR_NOATTRIBUTE)
> ! ereport(ERROR,
> ! (errcode(ERRCODE_UNDEFINED_COLUMN),
> ! errmsg("record \"%s\" has no field \"%s\"",
> ! rec->refname, fname)));
> ! }
> ! else
> ! ereport(ERROR,
> ! (errcode(ERRCODE_UNDEFINED_COLUMN),
> ! errmsg("record \"%s\": internal error",
> ! rec->refname)));
> fno--;
> natts = rec->tupdesc->natts;
>
> ***************
> *** 3497,3518 ****
> errmsg("record \"%s\" is not assigned yet",
> rec->refname),
> errdetail("The tuple structure of a not-yet-assigned record is indeterminate.")));
> ! fno = SPI_fnumber(rec->tupdesc, recfield->fieldname);
> ! if (fno == SPI_ERROR_NOATTRIBUTE)
> ! ereport(ERROR,
> ! (errcode(ERRCODE_UNDEFINED_COLUMN),
> ! errmsg("record \"%s\" has no field \"%s\"",
> ! rec->refname, recfield->fieldname)));
> ! *typeid = SPI_gettypeid(rec->tupdesc, fno);
> ! *value = SPI_getbinval(rec->tup, rec->tupdesc, fno, isnull);
> ! if (expectedtypeid != InvalidOid && expectedtypeid != *typeid)
> ! ereport(ERROR,
> ! (errcode(ERRCODE_DATATYPE_MISMATCH),
> ! errmsg("type of \"%s.%s\" does not match that when preparing the plan",
> ! rec->refname, recfield->fieldname)));
> ! break;
> ! }
> !
> case PLPGSQL_DTYPE_TRIGARG:
> {
> PLpgSQL_trigarg *trigarg = (PLpgSQL_trigarg *) datum;
> --- 3532,3656 ----
> errmsg("record \"%s\" is not assigned yet",
> rec->refname),
> errdetail("The tuple structure of a not-yet-assigned record is indeterminate.")));
> ! if ( recfield->fieldindex_flag == RECFIELD_USE_FIELDNAME ) {
> ! fno = SPI_fnumber(rec->tupdesc, recfield->fieldindex.fieldname);
> ! if (fno == SPI_ERROR_NOATTRIBUTE)
> ! ereport(ERROR,
> ! (errcode(ERRCODE_UNDEFINED_COLUMN),
> ! errmsg("record \"%s\" has no field \"%s\"",
> ! rec->refname, recfield->fieldindex.fieldname)));
> ! }
> ! else if ( recfield->fieldindex_flag == RECFIELD_USE_INDEX_VAR ) {
> ! PLpgSQL_var * idxvar = (PLpgSQL_var *) (estate->datums[recfield->fieldindex.indexvar_no]);
> ! char * fname = convert_value_to_string(idxvar->value, idxvar->datatype->typoid);
> ! if ( fname == NULL )
> ! ereport(ERROR,
> ! (errcode(ERRCODE_UNDEFINED_COLUMN),
> ! errmsg("record \"%s\": cannot evaluate variable to record index string",
> ! rec->refname)));
> ! fno = SPI_fnumber(rec->tupdesc, fname);
> ! pfree(fname);
> ! if (fno == SPI_ERROR_NOATTRIBUTE)
> ! ereport(ERROR,
> ! (errcode(ERRCODE_UNDEFINED_COLUMN),
> ! errmsg("record \"%s\" has no field \"%s\"",
> ! rec->refname, fname)));
> ! }
> ! else
> ! ereport(ERROR,
> ! (errcode(ERRCODE_UNDEFINED_COLUMN),
> ! errmsg("record \"%s\": internal error",
> ! rec->refname)));
> !
> ! /* Do not allow typeids to become "narrowed" by InvalidOids
> ! causing specialized typeids from the tuple restricting the destination */
> ! if ( expectedtypeid != InvalidOid && expectedtypeid != SPI_gettypeid(rec->tupdesc, fno) ) {
> ! Datum cval = SPI_getbinval(rec->tup, rec->tupdesc, fno, isnull);
> ! cval = exec_simple_cast_value(cval,
> ! SPI_gettypeid(rec->tupdesc, fno),
> ! expectedtypeid,
> ! -1,
> ! isnull);
> !
> ! *value = cval;
> ! *typeid = expectedtypeid;
> ! /* ereport(ERROR,
> ! (errcode(ERRCODE_DATATYPE_MISMATCH),
> ! errmsg("type of \"%s\" does not match that when preparing the plan",
> ! rec->refname)));
> ! */
> ! }
> ! else { /* expected typeid matches */
> ! *value = SPI_getbinval(rec->tup, rec->tupdesc, fno, isnull);
> ! *typeid = SPI_gettypeid(rec->tupdesc, fno);
> ! }
> ! break;
> ! }
> !
> ! case PLPGSQL_DTYPE_RECFIELDNAMES:
> ! /* Construct array datum from record field names */
> ! {
> ! Oid arraytypeid,
> ! arrayelemtypeid = TEXTOID;
> ! int16 arraytyplen,
> ! elemtyplen;
> ! bool elemtypbyval;
> ! char elemtypalign;
> ! ArrayType *arrayval;
> ! PLpgSQL_recfieldproperties * recfp = (PLpgSQL_recfieldproperties *) datum;
> ! PLpgSQL_rec *rec = (PLpgSQL_rec *) (estate->datums[recfp->recparentno]);
> ! int fc, tfc = 0;
> ! Datum *arrayelems;
> ! char *fieldname;
> !
> ! if (!HeapTupleIsValid(rec->tup))
> ! ereport(ERROR,
> ! (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> ! errmsg("record \"%s\" is not assigned yet",
> ! rec->refname),
> ! errdetail("The tuple structure of a not-yet-assigned record is indeterminate.")));
> ! arrayelems = palloc(sizeof(Datum) * rec->tupdesc->natts);
> ! arraytypeid = get_array_type(arrayelemtypeid);
> ! arraytyplen = get_typlen(arraytypeid);
> ! get_typlenbyvalalign(arrayelemtypeid,
> ! &elemtyplen,
> ! &elemtypbyval,
> ! &elemtypalign);
> !
> ! if ( expectedtypeid != InvalidOid && expectedtypeid != arraytypeid )
> ! ereport(ERROR,
> ! (errcode(ERRCODE_DATATYPE_MISMATCH),
> ! errmsg("type of \"%s\" does not match array type when preparing the plan",
> ! rec->refname)));
> ! for ( fc = 0; fc < rec->tupdesc->natts; ++fc ) {
> ! fieldname = SPI_fname(rec->tupdesc, fc+1);
> ! if ( fieldname ) {
> ! arrayelems[fc] = DirectFunctionCall1(textin, CStringGetDatum(fieldname));
> ! pfree(fieldname);
> ! ++tfc;
> ! }
> ! }
> ! arrayval = construct_array(arrayelems, tfc,
> ! arrayelemtypeid,
> ! elemtyplen,
> ! elemtypbyval,
> ! elemtypalign);
> !
> !
> ! /* construct_array copies data; free temp elem array */
> ! for ( fc = 0; fc < tfc; ++fc )
> ! pfree(DatumGetPointer(arrayelems[fc]));
> ! pfree(arrayelems);
> ! *value = PointerGetDatum(arrayval);
> ! *typeid = arraytypeid;
> ! *isnull = false;
> ! /* need to save the pointer because otherwise it does not get freed */
> ! if ( recfp->save_fieldnames )
> ! pfree(recfp->save_fieldnames);
> ! recfp->save_fieldnames = arrayval;
> ! break;
> ! }
> !
> case PLPGSQL_DTYPE_TRIGARG:
> {
> PLpgSQL_trigarg *trigarg = (PLpgSQL_trigarg *) datum;
> ***************
> *** 3610,3616 ****
> --- 3748,3790 ----
> */
> if (expr->plan == NULL)
> exec_prepare_plan(estate, expr);
> + else {
> + /*
> + * check for any subexpressions with varying type in the expression
> + * currently (July 05), this is a record field of a record indexed by a variable
> + */
> + int i;
> + PLpgSQL_datum *d;
> + PLpgSQL_recfield *rf;
> + for ( i = 0; i < expr->nparams; ++i ) {
> + d = estate->datums[expr->params[i]];
> + if ( d->dtype == PLPGSQL_DTYPE_RECFIELD ) {
> + rf = (PLpgSQL_recfield *)d;
> + if ( rf->fieldindex_flag == RECFIELD_USE_INDEX_VAR )
> + break;
> + }
> + }
> + if ( i < expr->nparams ) { /* expr may change it's type */
> + /* Make sure expr is not in the list of active simple expressions
> + because exec_prepare_plan()/exec_simple_check_plan()...
> + will destroy the link to the next simple expression */
> + PLpgSQL_expr *e, *enext;
> + PLpgSQL_expr **eprevnext = &active_simple_exprs;
> + for (e = active_simple_exprs; e; e = enext)
> + {
> + enext = e->expr_simple_next;
> + if ( e == expr ) /* found us in the list */
> + *eprevnext = enext; /* discard us */
> + else
> + eprevnext = &(e->expr_simple_next);
> + }
>
> + /* now discard the plan and get new one */
> + SPI_freeplan(expr->plan);
> + expr->plan = NULL;
> + exec_prepare_plan(estate, expr);
> + }
> + }
> /*
> * If this is a simple expression, bypass SPI and use the executor
> * directly
> *** ./src/pl/plpgsql/src/pl_funcs.c.orig Wed Jun 22 01:35:02 2005
> --- ./src/pl/plpgsql/src/pl_funcs.c Thu Jul 21 21:03:40 2005
> ***************
> *** 1066,1074 ****
> printf("REC %s\n", ((PLpgSQL_rec *) d)->refname);
> break;
> case PLPGSQL_DTYPE_RECFIELD:
> ! printf("RECFIELD %-16s of REC %d\n",
> ! ((PLpgSQL_recfield *) d)->fieldname,
> ! ((PLpgSQL_recfield *) d)->recparentno);
> break;
> case PLPGSQL_DTYPE_ARRAYELEM:
> printf("ARRAYELEM of VAR %d subscript ",
> --- 1066,1078 ----
> printf("REC %s\n", ((PLpgSQL_rec *) d)->refname);
> break;
> case PLPGSQL_DTYPE_RECFIELD:
> ! if ( ((PLpgSQL_recfield *) d)->fieldindex_flag == RECFIELD_USE_FIELDNAME )
> ! printf("RECFIELD %-16s of REC %d\n",
> ! ((PLpgSQL_recfield *) d)->fieldindex.fieldname,
> ! ((PLpgSQL_recfield *) d)->recparentno);
> ! else
> ! printf("RECFIELD Variable of REC %d\n",
> ! ((PLpgSQL_recfield *) d)->recparentno);
> break;
> case PLPGSQL_DTYPE_ARRAYELEM:
> printf("ARRAYELEM of VAR %d subscript ",
> *** ./src/pl/plpgsql/src/plpgsql.h.orig Thu Jul 21 21:00:57 2005
> --- ./src/pl/plpgsql/src/plpgsql.h Thu Jul 21 21:15:42 2005
> ***************
> *** 73,79 ****
> PLPGSQL_DTYPE_RECFIELD,
> PLPGSQL_DTYPE_ARRAYELEM,
> PLPGSQL_DTYPE_EXPR,
> ! PLPGSQL_DTYPE_TRIGARG
> };
>
> /* ----------
> --- 73,80 ----
> PLPGSQL_DTYPE_RECFIELD,
> PLPGSQL_DTYPE_ARRAYELEM,
> PLPGSQL_DTYPE_EXPR,
> ! PLPGSQL_DTYPE_TRIGARG,
> ! PLPGSQL_DTYPE_RECFIELDNAMES
> };
>
> /* ----------
> ***************
> *** 269,278 ****
> { /* Field in record */
> int dtype;
> int rfno;
> ! char *fieldname;
> int recparentno; /* dno of parent record */
> } PLpgSQL_recfield;
>
>
> typedef struct
> { /* Element of array variable */
> --- 270,294 ----
> { /* Field in record */
> int dtype;
> int rfno;
> ! union {
> ! char *fieldname;
> ! int indexvar_no; /* dno of variable holding index string */
> ! } fieldindex;
> ! enum {
> ! RECFIELD_USE_FIELDNAME,
> ! RECFIELD_USE_INDEX_VAR,
> ! } fieldindex_flag;
> int recparentno; /* dno of parent record */
> } PLpgSQL_recfield;
>
> + typedef struct
> + { /* Field in record */
> + int dtype;
> + int rfno;
> + int recparentno; /* dno of parent record */
> + ArrayType * save_fieldnames;
> + } PLpgSQL_recfieldproperties;
> +
>
> typedef struct
> { /* Element of array variable */
> ***************
> *** 678,683 ****
> --- 694,701 ----
> extern int plpgsql_parse_tripwordtype(char *word);
> extern int plpgsql_parse_wordrowtype(char *word);
> extern int plpgsql_parse_dblwordrowtype(char *word);
> + extern int plpgsql_parse_recfieldnames(char *word);
> + extern int plpgsql_parse_recindex(char *word);
> extern PLpgSQL_type *plpgsql_parse_datatype(const char *string);
> extern PLpgSQL_type *plpgsql_build_datatype(Oid typeOid, int32 typmod);
> extern PLpgSQL_variable *plpgsql_build_variable(const char *refname, int lineno,
> *** ./src/pl/plpgsql/src/scan.l.orig Sun Jun 26 19:16:07 2005
> --- ./src/pl/plpgsql/src/scan.l Thu Jul 21 21:28:20 2005
> ***************
> *** 243,248 ****
> --- 243,254 ----
> {param}{space}*\.{space}*{identifier}{space}*%ROWTYPE {
> plpgsql_error_lineno = plpgsql_scanner_lineno();
> return plpgsql_parse_dblwordrowtype(yytext); }
> + {identifier}{space}*\.\(\*\) {
> + plpgsql_error_lineno = plpgsql_scanner_lineno();
> + return plpgsql_parse_recfieldnames(yytext); }
> + {identifier}{space}*\.\({identifier}\) {
> + plpgsql_error_lineno = plpgsql_scanner_lineno();
> + return plpgsql_parse_recindex(yytext); }
>
> {digit}+ { return T_NUMBER; }
>
> *** ./src/test/regress/expected/plpgsql.out.orig Sat Jul 2 08:59:48 2005
> --- ./src/test/regress/expected/plpgsql.out Thu Jul 21 22:32:52 2005
> ***************
> *** 2721,2723 ****
> --- 2721,2761 ----
> $$ language plpgsql;
> ERROR: end label "outer_label" specified for unlabelled block
> CONTEXT: compile of PL/pgSQL function "end_label4" near line 5
> + -- check introspective records
> + create table ritest (i INT4, t TEXT);
> + insert into ritest (i, t) VALUES (1, 'sometext');
> + create function test_record() returns void as $$
> + declare
> + cname text;
> + tval text;
> + ival int4;
> + tval2 text;
> + ival2 int4;
> + columns text[];
> + r RECORD;
> + begin
> + SELECT INTO r * FROM ritest WHERE i = 1;
> + ival := r.i;
> + tval := r.t;
> + RAISE NOTICE 'ival=%, tval=%', ival, tval;
> + cname := 'i';
> + ival2 := r.(cname);
> + cname :='t';
> + tval2 := r.(cname);
> + RAISE NOTICE 'ival2=%, tval2=%', ival2, tval2;
> + columns := r.(*);
> + RAISE NOTICE 'fieldnames=%', columns;
> + RETURN;
> + end;
> + $$ language plpgsql;
> + select test_record();
> + NOTICE: ival=1, tval=sometext
> + NOTICE: ival2=1, tval2=sometext
> + NOTICE: fieldnames={i,t}
> + test_record
> + -------------
> +
> + (1 row)
> +
> + drop table ritest;
> + drop function test_record();
> *** ./src/test/regress/sql/plpgsql.sql.orig Thu Jul 21 21:04:43 2005
> --- ./src/test/regress/sql/plpgsql.sql Thu Jul 21 21:36:11 2005
> ***************
> *** 2280,2282 ****
> --- 2280,2314 ----
> end loop outer_label;
> end;
> $$ language plpgsql;
> +
> + -- check introspective records
> + create table ritest (i INT4, t TEXT);
> + insert into ritest (i, t) VALUES (1, 'sometext');
> + create function test_record() returns void as $$
> + declare
> + cname text;
> + tval text;
> + ival int4;
> + tval2 text;
> + ival2 int4;
> + columns text[];
> + r RECORD;
> + begin
> + SELECT INTO r * FROM ritest WHERE i = 1;
> + ival := r.i;
> + tval := r.t;
> + RAISE NOTICE 'ival=%, tval=%', ival, tval;
> + cname := 'i';
> + ival2 := r.(cname);
> + cname :='t';
> + tval2 := r.(cname);
> + RAISE NOTICE 'ival2=%, tval2=%', ival2, tval2;
> + columns := r.(*);
> + RAISE NOTICE 'fieldnames=%', columns;
> + RETURN;
> + end;
> + $$ language plpgsql;
> + select test_record();
> + drop table ritest;
> + drop function test_record();
> +

>
> ---------------------------(end of broadcast)---------------------------
> TIP 9: In versions below 8.0, the planner will ignore your desire to
> choose an index scan if your joining column's datatypes do not
> match

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073


From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Titus von Boxberg <ut(at)bhi-hamburg(dot)de>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Neil Conway <neilc(at)samurai(dot)com>, pgsql-patches(at)postgresql(dot)org
Subject: Re: PL/PGSQL: Dynamic Record Introspection
Date: 2006-04-25 20:47:42
Message-ID: 200604252047.k3PKlgJ00368@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches


This patch cannot be applied. 'active_simple_exprs' is referenced but
not defined. I think the new variable name is 'simple_eval_estate',
which used to be a structure member of 'active_simple_exprs'.

Would you please update it against current CVS and resubmit? Thanks.

---------------------------------------------------------------------------

Titus von Boxberg wrote:
> Tom Lane schrieb:
> > "Titus von Boxberg" <ut(at)bhi-hamburg(dot)de> writes:
> > It works for me if we want to have an "NFIELDS" construct. Personally
> > I'm still not convinced that we need one --- what's the use-case?
> I have removed the NFIELDS construct
> >
> >
> > I'd prefer arbitrary expression, but I suppose there's no harm in doing
> > the simple case first and generalizing if there's demand.
> I took the "no harm" way.
>
> Attached please find the updated patch.
> The patch is against HEAD of 050721.
>
> I switched the syntax to your proposal, renamed the functions
> in pl_comp.c and updated the sgml doc source and regression
> test files accordingly.
>
> Regards
> Titus
>

> *** ./doc/src/sgml/plpgsql.sgml.orig Sat Jul 2 08:59:47 2005
> --- ./doc/src/sgml/plpgsql.sgml Sat Jul 23 17:24:54 2005
> ***************
> *** 867,872 ****
> --- 867,921 ----
> </para>
>
> <para>
> + To obtain the values of the fields the record is made up of,
> + the record variable can be qualified with the column or field
> + name. This can be done either by literally using the column name
> + or the column name for indexing the record can be taken out of a scalar
> + variable. The syntax for this notation is Record_variable.(IndexVariable).
> + To get information about the column field names of the record,
> + a
> special expression exists that returns all column names as an array:
> + RecordVariable.(*) .
> + Thus, the RECORD can be viewed
> + as an associative array that allows for introspection of it's contents.
> + This feature is especially useful for writing generic triggers that
> + operate on records with unknown structure.
> + Here is an example procedure that shows column names and values
> + of the predefined record NEW in a trigger procedure:
> + <programlisting>
> +
> + CREATE OR REPLACE FUNCTION show_associative_records() RETURNS TRIGGER AS $$
> + DECLARE
> + colname TEXT;
> + colcontent TEXT;
> + colnames TEXT[];
> + coln INT4;
> + coli INT4;
> + BEGIN
> + -- obtain an array with all field names of the record
> + colnames := NEW.(*);
> + RAISE NOTICE 'All column names of test record: %', colnames;
> + -- show field names and contents of record
> + coli := 1;
> + coln := array_upper(colnames,1);
> + RAISE NOTICE 'Number of columns in NEW: %', coln;
> + FOR coli IN 1 .. coln LOOP
> + colname := colnames[coli];
> + colcontent := NEW.(colname);
> + RAISE NOTICE 'column % of NEW: %', quote_ident(colname), quote_literal(colcontent);
> + END LOOP;
> + -- Do it with a fixed field name:
> + -- will have to know the column name
> + RAISE NOTICE 'column someint of NEW: %', quote_literal(NEW.someint);
> + RETURN NULL;
> + END;
> + $$ LANGUAGE plpgsql;
> + --CREATE TABLE test_records (someint INT8, somestring TEXT);
> + --CREATE TRIGGER tr_test_record BEFORE INSERT ON test_records FOR EACH ROW EXECUTE PROCEDURE show_associative_records();
> +
> + </programlisting>
> + </para>
> +
> + <para>
> Note that <literal>RECORD</> is not a true data type, only a placeholder.
> One should also realize that when a <application>PL/pgSQL</application>
> function is declared to return type <type>record</>, this is not quite the
> *** ./src/pl/plpgsql/src/pl_comp.c.orig Wed Jul 6 16:42:10 2005
> --- ./src/pl/plpgsql/src/pl_comp.c Thu Jul 21 21:28:15 2005
> ***************
> *** 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 = pstrdup(cp[1]);
> ! new->fieldindex_flag = RECFIELD_USE_FIELDNAME;
> new->recparentno = ns->itemno;
>
> plpgsql_adddatum((PLpgSQL_datum *) new);
> ***************
> *** 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 = pstrdup(cp[2]);
> ! new->fieldindex_flag = RECFIELD_USE_FIELDNAME;
> new->recparentno = ns->itemno;
>
> plpgsql_adddatum((PLpgSQL_datum *) new);
> ***************
> *** 1550,1555 ****
> --- 1552,1683 ----
> MemoryContextSwitchTo(oldCxt);
> return T_DTYPE;
> }
> +
> + /* ----------
> + * plpgsql_parse_recindex
> + * lookup associative index into record
> + * ----------
> + */
> + int
> + plpgsql_parse_recindex(char *word)
> + {
> + PLpgSQL_nsitem *ns1, *ns2;
> + char *cp[2];
> + int ret = T_ERROR;
> + char *fieldvar;
> + int fl;
> +
> + /* Do case conversion and word separation */
> + plpgsql_convert_ident(word, cp, 2);
> + Assert(cp[1] != NULL);
> +
> + /* cleanup the "(identifier)" string to "identifier" */
> + fieldvar = cp[1];
> + Assert(*fieldvar == '(');
> + ++fieldvar; /* get rid of ( */
> +
> + fl = strlen(fieldvar);
> + Assert(fieldvar[fl-1] == ')');
> + fieldvar[fl-1] = 0; /* get rid of ) */
> +
> + /*
> + * Lookup the first word
> + */
> + ns1 = plpgsql_ns_lookup(cp[0], NULL);
> + if ( ns1 == NULL )
> + {
> + pfree(cp[0]);
> + pfree(cp[1]);
> + return T_ERROR;
> + }
> +
> + ns2 = plpgsql_ns_lookup(fieldvar, NULL);
> + pfree(cp[0]);
> + pfree(cp[1]);
> + if ( ns2 == NULL ) /* name lookup failed */
> + return T_ERROR;
> +
> + switch (ns1->itemtype)
> + {
> + case PLPGSQL_NSTYPE_REC:
> + {
> + /*
> + * First word is a record name, so second word must be an
> + * variable holding the field name in this record.
> + */
> + if ( ns2->itemtype == PLPGSQL_NSTYPE_VAR ) {
> + PLpgSQL_recfield *new;
> +
> + new = palloc(sizeof(PLpgSQL_recfield));
> + new->dtype = PLPGSQL_DTYPE_RECFIELD;
> + new->fieldindex.indexvar_no = ns2->itemno;
> + new->fieldindex_flag = RECFIELD_USE_INDEX_VAR;
> + new->recparentno = ns1->itemno;
> +
> + plpgsql_adddatum((PLpgSQL_datum *) new);
> +
> + plpgsql_yylval.scalar = (PLpgSQL_datum *) new;
> + ret = T_SCALAR;
> + }
> + break;
> + }
> + default:
> + break;
> + }
> + return ret;
> + }
> +
> +
> + /* ----------
> + * plpgsql_parse_recfieldnames
> + * create fieldnames of a record
> + * ----------
> + */
> + int
> + plpgsql_parse_recfieldnames(char *word)
> + {
> + PLpgSQL_nsitem *ns1;
> + char *cp[2];
> + int ret = T_ERROR;
> +
> + /* Do case conversion and word separation */
> + plpgsql_convert_ident(word, cp, 2);
> +
> + /*
> + * Lookup the first word
> + */
> + ns1 = plpgsql_ns_lookup(cp[0], NULL);
> + if ( ns1 == NULL )
> + {
> + pfree(cp[0]);
> + pfree(cp[1]);
> + return T_ERROR;
> + }
> +
> + pfree(cp[0]);
> + pfree(cp[1]);
> +
> + switch (ns1->itemtype)
> + {
> + case PLPGSQL_NSTYPE_REC:
> + {
> + PLpgSQL_recfieldproperties *new;
> +
> + new = palloc(sizeof(PLpgSQL_recfieldproperties));
> + new->dtype = PLPGSQL_DTYPE_RECFIELDNAMES;
> + new->recparentno = ns1->itemno;
> + new->save_fieldnames = NULL;
> + plpgsql_adddatum((PLpgSQL_datum *) new);
> + plpgsql_yylval.scalar = (PLpgSQL_datum *) new;
> + ret = T_SCALAR; /* ??? */
> + break;
> + }
> + default:
> + break;
> + }
> + return ret;
> + }
> +
>
> /*
> * plpgsql_build_variable - build a datum-array entry of a given
> *** ./src/pl/plpgsql/src/pl_exec.c.orig Sun Jun 26 22:05:42 2005
> --- ./src/pl/plpgsql/src/pl_exec.c Sat Jul 23 17:13:32 2005
> ***************
> *** 716,721 ****
> --- 716,722 ----
> case PLPGSQL_DTYPE_RECFIELD:
> case PLPGSQL_DTYPE_ARRAYELEM:
> case PLPGSQL_DTYPE_TRIGARG:
> + case PLPGSQL_DTYPE_RECFIELDNAMES:
> /*
> * These datum records are read-only at runtime, so no need
> * to copy them
> ***************
> *** 825,830 ****
> --- 826,832 ----
>
> case PLPGSQL_DTYPE_RECFIELD:
> case PLPGSQL_DTYPE_ARRAYELEM:
> + case PLPGSQL_DTYPE_RECFIELDNAMES:
> break;
>
> default:
> ***************
> *** 2146,2151 ****
> --- 2148,2155 ----
> static void
> exec_eval_cleanup(PLpgSQL_execstate *estate)
> {
> + int i;
> + ArrayType *a;
> /* Clear result of a full SPI_execute */
> if (estate->eval_tuptable != NULL)
> SPI_freetuptable(estate->eval_tuptable);
> ***************
> *** 2154,2159 ****
> --- 2158,2171 ----
> /* Clear result of exec_eval_simple_expr (but keep the econtext) */
> if (estate->eval_econtext != NULL)
> ResetExprContext(estate->eval_econtext);
> + for ( i = 0; i < estate->ndatums; ++i ) {
> + if ( estate->datums[i]->dtype == PLPGSQL_DTYPE_RECFIELDNAMES ) {
> + a = ((PLpgSQL_recfieldproperties *)(estate->datums[i]))->save_fieldnames;
> + if ( a )
> + pfree(a);
> + ((PLpgSQL_recfieldproperties *)(estate->datums[i]))->save_fieldnames = NULL;
> + }
> + }
> }
>
>
> ***************
> *** 3154,3165 ****
> * Get the number of the records field to change and the
> * number of attributes in the tuple.
> */
> ! fno = SPI_fnumber(rec->tupdesc, recfield->fieldname);
> ! if (fno == SPI_ERROR_NOATTRIBUTE)
> ! ereport(ERROR,
> ! (errcode(ERRCODE_UNDEFINED_COLUMN),
> ! errmsg("record \"%s\" has no field \"%s\"",
> ! rec->refname, recfield->fieldname)));
> fno--;
> natts = rec->tupdesc->natts;
>
> --- 3166,3200 ----
> * Get the number of the records field to change and the
> * number of attributes in the tuple.
> */
> ! if ( recfield->fieldindex_flag == RECFIELD_USE_FIELDNAME ) {
> ! fno = SPI_fnumber(rec->tupdesc, recfield->fieldindex.fieldname);
> ! if (fno == SPI_ERROR_NOATTRIBUTE)
> ! ereport(ERROR,
> ! (errcode(ERRCODE_UNDEFINED_COLUMN),
> ! errmsg("record \"%s\" has no field \"%s\"",
> ! rec->refname, recfield->fieldindex.fieldname)));
> ! }
> ! else if ( recfield->fieldindex_flag == RECFIELD_USE_INDEX_VAR ) {
> ! PLpgSQL_var * idxvar = (PLpgSQL_var *) (estate->datums[recfield->fieldindex.indexvar_no]);
> ! char * fname = convert_value_to_string(idxvar->value, idxvar->datatype->typoid);
> ! if ( fname == NULL )
> ! ereport(ERROR,
> ! (errcode(ERRCODE_UNDEFINED_COLUMN),
> ! errmsg("record \"%s\": cannot evaluate variable to record index string",
> ! rec->refname)));
> ! fno = SPI_fnumber(rec->tupdesc, fname);
> ! pfree(fname);
> ! if (fno == SPI_ERROR_NOATTRIBUTE)
> ! ereport(ERROR,
> ! (errcode(ERRCODE_UNDEFINED_COLUMN),
> ! errmsg("record \"%s\" has no field \"%s\"",
> ! rec->refname, fname)));
> ! }
> ! else
> ! ereport(ERROR,
> ! (errcode(ERRCODE_UNDEFINED_COLUMN),
> ! errmsg("record \"%s\": internal error",
> ! rec->refname)));
> fno--;
> natts = rec->tupdesc->natts;
>
> ***************
> *** 3497,3518 ****
> errmsg("record \"%s\" is not assigned yet",
> rec->refname),
> errdetail("The tuple structure of a not-yet-assigned record is indeterminate.")));
> ! fno = SPI_fnumber(rec->tupdesc, recfield->fieldname);
> ! if (fno == SPI_ERROR_NOATTRIBUTE)
> ! ereport(ERROR,
> ! (errcode(ERRCODE_UNDEFINED_COLUMN),
> ! errmsg("record \"%s\" has no field \"%s\"",
> ! rec->refname, recfield->fieldname)));
> ! *typeid = SPI_gettypeid(rec->tupdesc, fno);
> ! *value = SPI_getbinval(rec->tup, rec->tupdesc, fno, isnull);
> ! if (expectedtypeid != InvalidOid && expectedtypeid != *typeid)
> ! ereport(ERROR,
> ! (errcode(ERRCODE_DATATYPE_MISMATCH),
> ! errmsg("type of \"%s.%s\" does not match that when preparing the plan",
> ! rec->refname, recfield->fieldname)));
> ! break;
> ! }
> !
> case PLPGSQL_DTYPE_TRIGARG:
> {
> PLpgSQL_trigarg *trigarg = (PLpgSQL_trigarg *) datum;
> --- 3532,3656 ----
> errmsg("record \"%s\" is not assigned yet",
> rec->refname),
> errdetail("The tuple structure of a not-yet-assigned record is indeterminate.")));
> ! if ( recfield->fieldindex_flag == RECFIELD_USE_FIELDNAME ) {
> ! fno = SPI_fnumber(rec->tupdesc, recfield->fieldindex.fieldname);
> ! if (fno == SPI_ERROR_NOATTRIBUTE)
> ! ereport(ERROR,
> ! (errcode(ERRCODE_UNDEFINED_COLUMN),
> ! errmsg("record \"%s\" has no field \"%s\"",
> ! rec->refname, recfield->fieldindex.fieldname)));
> ! }
> ! else if ( recfield->fieldindex_flag == RECFIELD_USE_INDEX_VAR ) {
> ! PLpgSQL_var * idxvar = (PLpgSQL_var *) (estate->datums[recfield->fieldindex.indexvar_no]);
> ! char * fname = convert_value_to_string(idxvar->value, idxvar->datatype->typoid);
> ! if ( fname == NULL )
> ! ereport(ERROR,
> ! (errcode(ERRCODE_UNDEFINED_COLUMN),
> ! errmsg("record \"%s\": cannot evaluate variable to record index string",
> ! rec->refname)));
> ! fno = SPI_fnumber(rec->tupdesc, fname);
> ! pfree(fname);
> ! if (fno == SPI_ERROR_NOATTRIBUTE)
> ! ereport(ERROR,
> ! (errcode(ERRCODE_UNDEFINED_COLUMN),
> ! errmsg("record \"%s\" has no field \"%s\"",
> ! rec->refname, fname)));
> ! }
> ! else
> ! ereport(ERROR,
> ! (errcode(ERRCODE_UNDEFINED_COLUMN),
> ! errmsg("record \"%s\": internal error",
> ! rec->refname)));
> !
> ! /* Do not allow typeids to become "narrowed" by InvalidOids
> ! causing specialized typeids from the tuple restricting the destination */
> ! if ( expectedtypeid != InvalidOid && expectedtypeid != SPI_gettypeid(rec->tupdesc, fno) ) {
> ! Datum cval = SPI_getbinval(rec->tup, rec->tupdesc, fno, isnull);
> ! cval = exec_simple_cast_value(cval,
> ! SPI_gettypeid(rec->tupdesc, fno),
> ! expectedtypeid,
> ! -1,
> ! isnull);
> !
> ! *value = cval;
> ! *typeid = expectedtypeid;
> ! /* ereport(ERROR,
> ! (errcode(ERRCODE_DATATYPE_MISMATCH),
> ! errmsg("type of \"%s\" does not match that when preparing the plan",
> ! rec->refname)));
> ! */
> ! }
> ! else { /* expected typeid matches */
> ! *value = SPI_getbinval(rec->tup, rec->tupdesc, fno, isnull);
> ! *typeid = SPI_gettypeid(rec->tupdesc, fno);
> ! }
> ! break;
> ! }
> !
> ! case PLPGSQL_DTYPE_RECFIELDNAMES:
> ! /* Construct array datum from record field names */
> ! {
> ! Oid arraytypeid,
> ! arrayelemtypeid = TEXTOID;
> ! int16 arraytyplen,
> ! elemtyplen;
> ! bool elemtypbyval;
> ! char elemtypalign;
> ! ArrayType *arrayval;
> ! PLpgSQL_recfieldproperties * recfp = (PLpgSQL_recfieldproperties *) datum;
> ! PLpgSQL_rec *rec = (PLpgSQL_rec *) (estate->datums[recfp->recparentno]);
> ! int fc, tfc = 0;
> ! Datum *arrayelems;
> ! char *fieldname;
> !
> ! if (!HeapTupleIsValid(rec->tup))
> ! ereport(ERROR,
> ! (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> ! errmsg("record \"%s\" is not assigned yet",
> ! rec->refname),
> ! errdetail("The tuple structure of a not-yet-assigned record is indeterminate.")));
> ! arrayelems = palloc(sizeof(Datum) * rec->tupdesc->natts);
> ! arraytypeid = get_array_type(arrayelemtypeid);
> ! arraytyplen = get_typlen(arraytypeid);
> ! get_typlenbyvalalign(arrayelemtypeid,
> ! &elemtyplen,
> ! &elemtypbyval,
> ! &elemtypalign);
> !
> ! if ( expectedtypeid != InvalidOid && expectedtypeid != arraytypeid )
> ! ereport(ERROR,
> ! (errcode(ERRCODE_DATATYPE_MISMATCH),
> ! errmsg("type of \"%s\" does not match array type when preparing the plan",
> ! rec->refname)));
> ! for ( fc = 0; fc < rec->tupdesc->natts; ++fc ) {
> ! fieldname = SPI_fname(rec->tupdesc, fc+1);
> ! if ( fieldname ) {
> ! arrayelems[fc] = DirectFunctionCall1(textin, CStringGetDatum(fieldname));
> ! pfree(fieldname);
> ! ++tfc;
> ! }
> ! }
> ! arrayval = construct_array(arrayelems, tfc,
> ! arrayelemtypeid,
> ! elemtyplen,
> ! elemtypbyval,
> ! elemtypalign);
> !
> !
> ! /* construct_array copies data; free temp elem array */
> ! for ( fc = 0; fc < tfc; ++fc )
> ! pfree(DatumGetPointer(arrayelems[fc]));
> ! pfree(arrayelems);
> ! *value = PointerGetDatum(arrayval);
> ! *typeid = arraytypeid;
> ! *isnull = false;
> ! /* need to save the pointer because otherwise it does not get freed */
> ! if ( recfp->save_fieldnames )
> ! pfree(recfp->save_fieldnames);
> ! recfp->save_fieldnames = arrayval;
> ! break;
> ! }
> !
> case PLPGSQL_DTYPE_TRIGARG:
> {
> PLpgSQL_trigarg *trigarg = (PLpgSQL_trigarg *) datum;
> ***************
> *** 3610,3616 ****
> --- 3748,3790 ----
> */
> if (expr->plan == NULL)
> exec_prepare_plan(estate, expr);
> + else {
> + /*
> + * check for any subexpressions with varying type in the expression
> + * currently (July 05), this is a record field of a record indexed by a variable
> + */
> + int i;
> + PLpgSQL_datum *d;
> + PLpgSQL_recfield *rf;
> + for ( i = 0; i < expr->nparams; ++i ) {
> + d = estate->datums[expr->params[i]];
> + if ( d->dtype == PLPGSQL_DTYPE_RECFIELD ) {
> + rf = (PLpgSQL_recfield *)d;
> + if ( rf->fieldindex_flag == RECFIELD_USE_INDEX_VAR )
> + break;
> + }
> + }
> + if ( i < expr->nparams ) { /* expr may change it's type */
> + /* Make sure expr is not in the list of active simple expressions
> + because exec_prepare_plan()/exec_simple_check_plan()...
> + will destroy the link to the next simple expression */
> + PLpgSQL_expr *e, *enext;
> + PLpgSQL_expr **eprevnext = &active_simple_exprs;
> + for (e = active_simple_exprs; e; e = enext)
> + {
> + enext = e->expr_simple_next;
> + if ( e == expr ) /* found us in the list */
> + *eprevnext = enext; /* discard us */
> + else
> + eprevnext = &(e->expr_simple_next);
> + }
>
> + /* now discard the plan and get new one */
> + SPI_freeplan(expr->plan);
> + expr->plan = NULL;
> + exec_prepare_plan(estate, expr);
> + }
> + }
> /*
> * If this is a simple expression, bypass SPI and use the executor
> * directly
> *** ./src/pl/plpgsql/src/pl_funcs.c.orig Wed Jun 22 01:35:02 2005
> --- ./src/pl/plpgsql/src/pl_funcs.c Thu Jul 21 21:03:40 2005
> ***************
> *** 1066,1074 ****
> printf("REC %s\n", ((PLpgSQL_rec *) d)->refname);
> break;
> case PLPGSQL_DTYPE_RECFIELD:
> ! printf("RECFIELD %-16s of REC %d\n",
> ! ((PLpgSQL_recfield *) d)->fieldname,
> ! ((PLpgSQL_recfield *) d)->recparentno);
> break;
> case PLPGSQL_DTYPE_ARRAYELEM:
> printf("ARRAYELEM of VAR %d subscript ",
> --- 1066,1078 ----
> printf("REC %s\n", ((PLpgSQL_rec *) d)->refname);
> break;
> case PLPGSQL_DTYPE_RECFIELD:
> ! if ( ((PLpgSQL_recfield *) d)->fieldindex_flag == RECFIELD_USE_FIELDNAME )
> ! printf("RECFIELD %-16s of REC %d\n",
> ! ((PLpgSQL_recfield *) d)->fieldindex.fieldname,
> ! ((PLpgSQL_recfield *) d)->recparentno);
> ! else
> ! printf("RECFIELD Variable of REC %d\n",
> ! ((PLpgSQL_recfield *) d)->recparentno);
> break;
> case PLPGSQL_DTYPE_ARRAYELEM:
> printf("ARRAYELEM of VAR %d subscript ",
> *** ./src/pl/plpgsql/src/plpgsql.h.orig Thu Jul 21 21:00:57 2005
> --- ./src/pl/plpgsql/src/plpgsql.h Thu Jul 21 21:15:42 2005
> ***************
> *** 73,79 ****
> PLPGSQL_DTYPE_RECFIELD,
> PLPGSQL_DTYPE_ARRAYELEM,
> PLPGSQL_DTYPE_EXPR,
> ! PLPGSQL_DTYPE_TRIGARG
> };
>
> /* ----------
> --- 73,80 ----
> PLPGSQL_DTYPE_RECFIELD,
> PLPGSQL_DTYPE_ARRAYELEM,
> PLPGSQL_DTYPE_EXPR,
> ! PLPGSQL_DTYPE_TRIGARG,
> ! PLPGSQL_DTYPE_RECFIELDNAMES
> };
>
> /* ----------
> ***************
> *** 269,278 ****
> { /* Field in record */
> int dtype;
> int rfno;
> ! char *fieldname;
> int recparentno; /* dno of parent record */
> } PLpgSQL_recfield;
>
>
> typedef struct
> { /* Element of array variable */
> --- 270,294 ----
> { /* Field in record */
> int dtype;
> int rfno;
> ! union {
> ! char *fieldname;
> ! int indexvar_no; /* dno of variable holding index string */
> ! } fieldindex;
> ! enum {
> ! RECFIELD_USE_FIELDNAME,
> ! RECFIELD_USE_INDEX_VAR,
> ! } fieldindex_flag;
> int recparentno; /* dno of parent record */
> } PLpgSQL_recfield;
>
> + typedef struct
> + { /* Field in record */
> + int dtype;
> + int rfno;
> + int recparentno; /* dno of parent record */
> + ArrayType * save_fieldnames;
> + } PLpgSQL_recfieldproperties;
> +
>
> typedef struct
> { /* Element of array variable */
> ***************
> *** 678,683 ****
> --- 694,701 ----
> extern int plpgsql_parse_tripwordtype(char *word);
> extern int plpgsql_parse_wordrowtype(char *word);
> extern int plpgsql_parse_dblwordrowtype(char *word);
> + extern int plpgsql_parse_recfieldnames(char *word);
> + extern int plpgsql_parse_recindex(char *word);
> extern PLpgSQL_type *plpgsql_parse_datatype(const char *string);
> extern PLpgSQL_type *plpgsql_build_datatype(Oid typeOid, int32 typmod);
> extern PLpgSQL_variable *plpgsql_build_variable(const char *refname, int lineno,
> *** ./src/pl/plpgsql/src/scan.l.orig Sun Jun 26 19:16:07 2005
> --- ./src/pl/plpgsql/src/scan.l Thu Jul 21 21:28:20 2005
> ***************
> *** 243,248 ****
> --- 243,254 ----
> {param}{space}*\.{space}*{identifier}{space}*%ROWTYPE {
> plpgsql_error_lineno = plpgsql_scanner_lineno();
> return plpgsql_parse_dblwordrowtype(yytext); }
> + {identifier}{space}*\.\(\*\) {
> + plpgsql_error_lineno = plpgsql_scanner_lineno();
> + return plpgsql_parse_recfieldnames(yytext); }
> + {identifier}{space}*\.\({identifier}\) {
> + plpgsql_error_lineno = plpgsql_scanner_lineno();
> + return plpgsql_parse_recindex(yytext); }
>
> {digit}+ { return T_NUMBER; }
>
> *** ./src/test/regress/expected/plpgsql.out.orig Sat Jul 2 08:59:48 2005
> --- ./src/test/regress/expected/plpgsql.out Thu Jul 21 22:32:52 2005
> ***************
> *** 2721,2723 ****
> --- 2721,2761 ----
> $$ language plpgsql;
> ERROR: end label "outer_label" specified for unlabelled block
> CONTEXT: compile of PL/pgSQL function "end_label4" near line 5
> + -- check introspective records
> + create table ritest (i INT4, t TEXT);
> + insert into ritest (i, t) VALUES (1, 'sometext');
> + create function test_record() returns void as $$
> + declare
> + cname text;
> + tval text;
> + ival int4;
> + tval2 text;
> + ival2 int4;
> + columns text[];
> + r RECORD;
> + begin
> + SELECT INTO r * FROM ritest WHERE i = 1;
> + ival := r.i;
> + tval := r.t;
> + RAISE NOTICE 'ival=%, tval=%', ival, tval;
> + cname := 'i';
> + ival2 := r.(cname);
> + cname :='t';
> + tval2 := r.(cname);
> + RAISE NOTICE 'ival2=%, tval2=%', ival2, tval2;
> + columns := r.(*);
> + RAISE NOTICE 'fieldnames=%', columns;
> + RETURN;
> + end;
> + $$ language plpgsql;
> + select test_record();
> + NOTICE: ival=1, tval=sometext
> + NOTICE: ival2=1, tval2=sometext
> + NOTICE: fieldnames={i,t}
> + test_record
> + -------------
> +
> + (1 row)
> +
> + drop table ritest;
> + drop function test_record();
> *** ./src/test/regress/sql/plpgsql.sql.orig Thu Jul 21 21:04:43 2005
> --- ./src/test/regress/sql/plpgsql.sql Thu Jul 21 21:36:11 2005
> ***************
> *** 2280,2282 ****
> --- 2280,2314 ----
> end loop outer_label;
> end;
> $$ language plpgsql;
> +
> + -- check introspective records
> + create table ritest (i INT4, t TEXT);
> + insert into ritest (i, t) VALUES (1, 'sometext');
> + create function test_record() returns void as $$
> + declare
> + cname text;
> + tval text;
> + ival int4;
> + tval2 text;
> + ival2 int4;
> + columns text[];
> + r RECORD;
> + begin
> + SELECT INTO r * FROM ritest WHERE i = 1;
> + ival := r.i;
> + tval := r.t;
> + RAISE NOTICE 'ival=%, tval=%', ival, tval;
> + cname := 'i';
> + ival2 := r.(cname);
> + cname :='t';
> + tval2 := r.(cname);
> + RAISE NOTICE 'ival2=%, tval2=%', ival2, tval2;
> + columns := r.(*);
> + RAISE NOTICE 'fieldnames=%', columns;
> + RETURN;
> + end;
> + $$ language plpgsql;
> + select test_record();
> + drop table ritest;
> + drop function test_record();
> +

>
> ---------------------------(end of broadcast)---------------------------
> TIP 9: In versions below 8.0, the planner will ignore your desire to
> choose an index scan if your joining column's datatypes do not
> match

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +