Hi,
I got extremely frustrated with having to create a temp table every time
I wanted to access an arbitrary column from a record plpgsql. After
seeing a note on the developers TODO about accessing plpgsql records
with a 'dot bracket' notation I started digging into the plpgsql source.
My diff (against 8beta4) is attached.
Warning: I Am Not a C Programmer! I haven't even written a hello world
in C before, and I knew nothing about Flex before yesterday. It was fun
figuring stuff out, I'm amazed it mostly works, but I'm really hoping
someone can point out my mistakes.
Goal:
Enable users to access fields in record variables using the following
syntax like the following:
rec.(1)
rec.('foo')
rec.(myvar::int)
rec.(myvar || '_id')
Files changed:
plpgsql.h
- added 'expr' member to PLpgSQL_recfield type for holding the
PLpgSQL_expr structure.
scan.l
- added match for {identifier}{space}*\. AFAIK this should only match
if a longer expression doesn't?
pl_comp.c
- added plpgsql_parse_wordexpr() function called by above match. Ripped
off code from plpgsql_parse_word that deals with arg_v[expr] to find our
expression. Prob a dumb name for the function!
pl_exec.c
- hacked exec_assign_value() and exec_eval_datum() to use the expression
to get the field name/number.
Stuff I need help with:
1. It should recognise OLD.(1) as a field number, not a column name. I
think I've got to check the returned type from exec_eval_expr() then
exec_simple_cast_value() on it, but that seems beyond me.
2. Freeing stuff. As I explained, this is all pretty new to me, and the
comments about it around exec_eval_expr() et al just confused me :(
Please give some hints about what needs freeing!
3. Would probably be good to add check in pl_comp.c to see if the
expression actually needs to be evaluated at runtime (ie isn't just a
field name/number). How?
4. Make this also work for row.(expr), label.record.(expr) and
label.row.(expr) - but want to get the basics working first!
5. Because of the way the expression is parsed (looking for closing
parenth), this will choke if you try and put a function in there. Would
it be better to use curly braces '{expr}' or another character to mark
the expression?
I hope at this eventually leads to some really useful extra
functionality, particularly for writing generic triggers. And it's a
tribute to the existing code that a complete newbie can cut-and-paste
their way to a halfarsed solution in a (rather long) night!
Regards,
Matt
diff -u src/pl_comp.c src.mk/pl_comp.c
--- src/pl_comp.c 2004-09-13 21:09:20.000000000 +0100
+++ src.mk/pl_comp.c 2004-11-18 12:59:25.825372489 +0000
@@ -3,7 +3,7 @@
* procedural language
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_comp.c,v 1.82 2004/09/13 20:09:20 tgl Exp $
+ * $PostgreSQL: pgsql-server/src/pl/plpgsql/src/pl_comp.c,v 1.82 2004/09/13 20:09:20 tgl Exp $
*
* This software is copyrighted by Jan Wieck - Hamburg.
*
@@ -783,6 +783,75 @@
return T_WORD;
}
+/* ----------
+ * plpgsql_parse_wordexpr Same lookup for word followed by dot.
+ * Should only get here if it wasn't followed by
+ * an identifier.
+ * ----------
+ */
+int
+plpgsql_parse_wordexpr(char *word)
+{
+ PLpgSQL_nsitem *ns;
+ char *cp[1];
+ int save_spacescanned = plpgsql_SpaceScanned;
+
+ /* Do case conversion and word separation */
+ /* add fake bit after dot to keep converter happy */
+ word[strlen(word) - 1] = '\0';
+ plpgsql_convert_ident(word, cp, 1);
+
+ /* Make sure we've got an open parenthesis */
+
+ /*
+ * Do a lookup on the compilers namestack
+ */
+ ns = plpgsql_ns_lookup(cp[0], NULL);
+ if (ns == NULL)
+ {
+ pfree(cp[0]);
+ return T_ERROR;
+ }
+ switch (ns->itemtype)
+ {
+ case PLPGSQL_NSTYPE_REC:
+ {
+ /*
+ * First word is a record name, so expression refers to
+ * field in this record.
+ */
+ PLpgSQL_recfield *new;
+
+ new = malloc(sizeof(PLpgSQL_recfield));
+ memset(new, 0, sizeof(PLpgSQL_recfield));
+
+ if (plpgsql_yylex() != '(')
+ plpgsql_yyerror("expected identifier or \"(\"");
+ new->expr = plpgsql_read_expression(')', ")");
+ new->recparentno = ns->itemno;
+ /* just to be sure - we'll test on this later */
+ new->fieldname = '\0';
+ new->dtype = PLPGSQL_DTYPE_RECFIELD;
+
+ plpgsql_adddatum((PLpgSQL_datum *) new);
+
+ plpgsql_yylval.scalar = (PLpgSQL_datum *) new;
+
+ plpgsql_SpaceScanned = save_spacescanned;
+
+ pfree(cp[0]);
+ return T_SCALAR;
+ }
+
+ /* TODO: deal with rows, too */
+
+ default:
+ break;
+
+ }
+ pfree(cp[0]);
+ return T_ERROR;
+}
/* ----------
* plpgsql_parse_dblword Same lookup for two words
diff -u src/pl_exec.c src.mk/pl_exec.c
--- src/pl_exec.c 2004-09-16 17:58:44.000000000 +0100
+++ src.mk/pl_exec.c 2004-11-18 12:59:25.825372489 +0000
@@ -3,7 +3,7 @@
* procedural language
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.120 2004/09/16 16:58:44 tgl Exp $
+ * $PostgreSQL: pgsql-server/src/pl/plpgsql/src/pl_exec.c,v 1.120 2004/09/16 16:58:44 tgl Exp $
*
* This software is copyrighted by Jan Wieck - Hamburg.
*
@@ -2965,7 +2965,8 @@
PLpgSQL_recfield *recfield = (PLpgSQL_recfield *) target;
PLpgSQL_rec *rec;
int fno;
- HeapTuple newtup;
+ char *fname;
+ HeapTuple newtup;
int natts;
int i;
Datum *values;
@@ -2988,20 +2989,48 @@
errmsg("record \"%s\" is not assigned yet",
rec->refname),
errdetail("The tuple structure of a not-yet-assigned record is indeterminate.")));
-
- /*
+
+ /* Have we got an expr to deal with? */
+ fno = 0;
+ if (recfield->fieldname == '\0')
+ {
+ Datum exprdatum;
+ Oid exprtype;
+ bool *isNull = false;
+
+ /* Evaluate expression */
+ exprdatum = exec_eval_expr(estate, recfield->expr,
+ isNull, &exprtype);
+
+ /* Get C-String representation */
+ fname = convert_value_to_string(
+ exprdatum, exprtype);
+ /* cols must be one-based... not ideal */
+ fno = atoi(fname);
+
+ /* do we need to free datum? */
+ }
+ else {
+ fname = recfield->fieldname;
+ }
+
+ /*
* 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;
-
+ if (fno < 1)
+ {
+ fno = SPI_fnumber(rec->tupdesc, fname);
+ if (fno == SPI_ERROR_NOATTRIBUTE)
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_COLUMN),
+ errmsg("record \"%s\" has no field \"%s\"",
+ rec->refname, fname)));
+ }
+ fno--;
+ natts = rec->tupdesc->natts;
+
+
/*
* Set up values/datums arrays for heap_formtuple. For
* all the attributes except the one we want to replace,
@@ -3297,7 +3326,8 @@
PLpgSQL_recfield *recfield = (PLpgSQL_recfield *) datum;
PLpgSQL_rec *rec;
int fno;
-
+ char *fname;
+
rec = (PLpgSQL_rec *) (estate->datums[recfield->recparentno]);
if (!HeapTupleIsValid(rec->tup))
ereport(ERROR,
@@ -3305,19 +3335,59 @@
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);
+
+ /* Have we got an expr to deal with? */
+ fno = 0;
+ if (recfield->fieldname == '\0')
+ {
+ Datum exprdatum;
+ Oid exprtype;
+
+ /* Evaluate expression */
+ exprdatum = exec_eval_expr(estate, recfield->expr,
+ isnull, &exprtype);
+
+
+ /* Get C-String representation */
+ fname = convert_value_to_string(
+ exprdatum, exprtype);
+
+ /* How do we test to see if it's an integer for fno?
+ this isn't working... */
+ fno = atoi(fname);
+
+ }
+ else {
+ fname = recfield->fieldname;
+ }
+
+ /*
+ * Get the number of the records field to change and the
+ * number of attributes in the tuple.
+ */
+ if (fno < 1)
+ {
+ fno = SPI_fnumber(rec->tupdesc, fname);
+ if (fno == SPI_ERROR_NOATTRIBUTE)
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_COLUMN),
+ errmsg("record \"%s\" has no field \"%s\"",
+ rec->refname, fname)));
+ }
+
+ *typeid = SPI_gettypeid(rec->tupdesc, fno);
*value = SPI_getbinval(rec->tup, rec->tupdesc, fno, isnull);
- if (expectedtypeid != InvalidOid && expectedtypeid != *typeid)
+ if (*value == SPI_ERROR_NOATTRIBUTE)
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_COLUMN),
+ errmsg("record \"%s\" has no field number \"%d\"",
+ rec->refname, &fno)));
+
+ 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)));
+ rec->refname, fname)));
break;
}
Only in src.mk: .plpgsql.h.swp
diff -u src/scan.l src.mk/scan.l
--- src/scan.l 2004-09-13 02:45:32.000000000 +0100
+++ src.mk/scan.l 2004-11-18 12:59:25.825372489 +0000
@@ -4,7 +4,7 @@
* procedural language
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/pl/plpgsql/src/scan.l,v 1.37 2004/09/13 01:45:32 neilc Exp $
+ * $PostgreSQL: pgsql-server/src/pl/plpgsql/src/scan.l,v 1.37 2004/09/13 01:45:32 neilc Exp $
*
* This software is copyrighted by Jan Wieck - Hamburg.
*
@@ -195,6 +195,9 @@
{identifier} {
plpgsql_error_lineno = plpgsql_scanner_lineno();
return plpgsql_parse_word(yytext); }
+{identifier}{space}*\. {
+ plpgsql_error_lineno = plpgsql_scanner_lineno();
+ return plpgsql_parse_wordexpr(yytext); }
{identifier}{space}*\.{space}*{identifier} {
plpgsql_error_lineno = plpgsql_scanner_lineno();
return plpgsql_parse_dblword(yytext); }
---------------------------(end of broadcast)---------------------------
TIP 9: the planner will ignore your desire to choose an index scan if your
joining column's datatypes do not match
