From 3477957b44baf2b53f95207ff8824a0361301a2e Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 11 Jun 2000 20:08:01 +0000 Subject: [PATCH] Update sequence-related functions to new fmgr style. Remove downcasing, quote-stripping, and acl-checking tasks for these functions from the parser, and do them at function execution time instead. This fixes the failure of pg_dump to produce correct output for nextval(Foo) used in a rule, and also eliminates the restriction that the argument of these functions must be a parse-time constant. --- contrib/spi/autoinc.c | 10 +-- src/backend/commands/sequence.c | 106 +++++++++++++++++++++++++------- src/backend/parser/parse_func.c | 58 +++-------------- src/include/catalog/pg_proc.h | 10 +-- src/include/commands/sequence.h | 10 +-- src/test/regress/regress.c | 10 +-- 6 files changed, 113 insertions(+), 91 deletions(-) diff --git a/contrib/spi/autoinc.c b/contrib/spi/autoinc.c index 33cce45723..804e1d6383 100644 --- a/contrib/spi/autoinc.c +++ b/contrib/spi/autoinc.c @@ -1,9 +1,9 @@ #include "executor/spi.h" /* this is what you need to work with SPI */ #include "commands/trigger.h" /* -"- and triggers */ +#include "commands/sequence.h" /* for nextval() */ extern Datum autoinc(PG_FUNCTION_ARGS); -extern int4 nextval(struct varlena * seqin); Datum autoinc(PG_FUNCTION_ARGS) @@ -53,7 +53,7 @@ autoinc(PG_FUNCTION_ARGS) for (i = 0; i < nargs;) { - struct varlena *seqname; + text *seqname; int attnum = SPI_fnumber(tupdesc, args[i]); int32 val; @@ -74,9 +74,11 @@ autoinc(PG_FUNCTION_ARGS) i++; chattrs[chnattrs] = attnum; seqname = textin(args[i]); - newvals[chnattrs] = Int32GetDatum(nextval(seqname)); + newvals[chnattrs] = DirectFunctionCall1(nextval, + PointerGetDatum(seqname)); if (DatumGetInt32(newvals[chnattrs]) == 0) - newvals[chnattrs] = Int32GetDatum(nextval(seqname)); + newvals[chnattrs] = DirectFunctionCall1(nextval, + PointerGetDatum(seqname)); pfree(seqname); chnattrs++; i++; diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c index 0667297d76..f95d6fbb6c 100644 --- a/src/backend/commands/sequence.c +++ b/src/backend/commands/sequence.c @@ -6,6 +6,8 @@ *------------------------------------------------------------------------- */ +#include + #include "postgres.h" #include "access/heapam.h" @@ -54,6 +56,7 @@ typedef SeqTableData *SeqTable; static SeqTable seqtab = NULL; +static char *get_seq_name(text *seqin); static SeqTable init_sequence(char *caller, char *name); static Form_pg_sequence read_info(char *caller, SeqTable elm, Buffer *buf); static void init_params(CreateSeqStmt *seq, Form_pg_sequence new); @@ -181,29 +184,37 @@ DefineSequence(CreateSeqStmt *seq) } -int4 -nextval(struct varlena * seqin) +Datum +nextval(PG_FUNCTION_ARGS) { - char *seqname = textout(seqin); + text *seqin = PG_GETARG_TEXT_P(0); + char *seqname = get_seq_name(seqin); SeqTable elm; Buffer buf; Form_pg_sequence seq; - int4 incby, + int32 incby, maxv, minv, cache; - int4 result, + int32 result, next, rescnt = 0; +#ifndef NO_SECURITY + if (pg_aclcheck(seqname, getpgusername(), ACL_WR) != ACLCHECK_OK) + elog(ERROR, "%s.nextval: you don't have permissions to set sequence %s", + seqname, seqname); +#endif + /* open and AccessShareLock sequence */ elm = init_sequence("nextval", seqname); + pfree(seqname); if (elm->last != elm->cached) /* some numbers were cached */ { elm->last += elm->increment; - return elm->last; + PG_RETURN_INT32(elm->last); } seq = read_info("nextval", elm, &buf); /* lock page' buffer and @@ -225,8 +236,9 @@ nextval(struct varlena * seqin) * Check MAXVALUE for ascending sequences and MINVALUE for * descending sequences */ - if (incby > 0) /* ascending sequence */ + if (incby > 0) { + /* ascending sequence */ if ((maxv >= 0 && next > maxv - incby) || (maxv < 0 && next + incby > maxv)) { @@ -241,8 +253,8 @@ nextval(struct varlena * seqin) next += incby; } else -/* descending sequence */ { + /* descending sequence */ if ((minv < 0 && next < minv - incby) || (minv >= 0 && next + incby < minv)) { @@ -274,35 +286,43 @@ nextval(struct varlena * seqin) if (WriteBuffer(buf) == STATUS_ERROR) elog(ERROR, "%s.nextval: WriteBuffer failed", elm->name); - return result; - + PG_RETURN_INT32(result); } - -int4 -currval(struct varlena * seqin) +Datum +currval(PG_FUNCTION_ARGS) { - char *seqname = textout(seqin); + text *seqin = PG_GETARG_TEXT_P(0); + char *seqname = get_seq_name(seqin); SeqTable elm; - int4 result; + int32 result; + +#ifndef NO_SECURITY + if (pg_aclcheck(seqname, getpgusername(), ACL_RD) != ACLCHECK_OK) + elog(ERROR, "%s.currval: you don't have permissions to read sequence %s", + seqname, seqname); +#endif /* open and AccessShareLock sequence */ elm = init_sequence("currval", seqname); - pfree(seqname); if (elm->increment == 0) /* nextval/read_info were not called */ - elog(ERROR, "%s.currval is not yet defined in this session", elm->name); + elog(ERROR, "%s.currval is not yet defined in this session", + seqname); result = elm->last; - return result; + pfree(seqname); + PG_RETURN_INT32(result); } -int4 -setval(struct varlena * seqin, int4 next) +Datum +setval(PG_FUNCTION_ARGS) { - char *seqname = textout(seqin); + text *seqin = PG_GETARG_TEXT_P(0); + int32 next = PG_GETARG_INT32(1); + char *seqname = get_seq_name(seqin); SeqTable elm; Buffer buf; Form_pg_sequence seq; @@ -341,9 +361,49 @@ setval(struct varlena * seqin, int4 next) LockBuffer(buf, BUFFER_LOCK_UNLOCK); if (WriteBuffer(buf) == STATUS_ERROR) - elog(ERROR, "%s.settval: WriteBuffer failed", seqname); + elog(ERROR, "%s.setval: WriteBuffer failed", seqname); - return next; + pfree(seqname); + + PG_RETURN_INT32(next); +} + +/* + * Given a 'text' parameter to a sequence function, extract the actual + * sequence name. We downcase the name if it's not double-quoted. + * + * This is a kluge, really --- should be able to write nextval(seqrel). + */ +static char * +get_seq_name(text *seqin) +{ + char *rawname = textout(seqin); + int rawlen = strlen(rawname); + char *seqname; + + if (rawlen >= 2 && + rawname[0] == '\"' && rawname[rawlen - 1] == '\"') + { + /* strip off quotes, keep case */ + rawname[rawlen - 1] = '\0'; + seqname = pstrdup(rawname + 1); + pfree(rawname); + } + else + { + seqname = rawname; + /* + * It's important that this match the identifier downcasing code + * used by backend/parser/scan.l. + */ + for (; *rawname; rawname++) + { + if (isascii((unsigned char) *rawname) && + isupper(*rawname)) + *rawname = tolower(*rawname); + } + } + return seqname; } static Form_pg_sequence diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c index 878c553005..541211af64 100644 --- a/src/backend/parser/parse_func.c +++ b/src/backend/parser/parse_func.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/parser/parse_func.c,v 1.82 2000/06/03 04:41:32 momjian Exp $ + * $Header: /cvsroot/pgsql/src/backend/parser/parse_func.c,v 1.83 2000/06/11 20:08:00 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -709,56 +709,14 @@ ParseFuncOrColumn(ParseState *pstate, char *funcname, List *fargs, } /* - * Sequence handling. + * Special checks to disallow sequence functions with side-effects + * in WHERE clauses. This is pretty much of a hack; why disallow these + * when we have no way to check for side-effects of user-defined fns? */ - if (funcid == F_NEXTVAL || - funcid == F_CURRVAL || - funcid == F_SETVAL) - { - Const *seq; - char *seqrel; - text *seqname; - int32 aclcheck_result = -1; - - Assert(nargs == ((funcid == F_SETVAL) ? 2 : 1)); - seq = (Const *) lfirst(fargs); - if (!IsA((Node *) seq, Const)) - elog(ERROR, "Only constant sequence names are acceptable for function '%s'", funcname); - - seqrel = textout((text *) DatumGetPointer(seq->constvalue)); - /* Do we have nextval('"Aa"')? */ - if (strlen(seqrel) >= 2 && - seqrel[0] == '\"' && seqrel[strlen(seqrel) - 1] == '\"') - { - /* strip off quotes, keep case */ - seqrel = pstrdup(seqrel + 1); - seqrel[strlen(seqrel) - 1] = '\0'; - pfree(DatumGetPointer(seq->constvalue)); - seq->constvalue = (Datum) textin(seqrel); - } - else - { - pfree(seqrel); - seqname = lower((text *) DatumGetPointer(seq->constvalue)); - pfree(DatumGetPointer(seq->constvalue)); - seq->constvalue = PointerGetDatum(seqname); - seqrel = textout(seqname); - } - - if ((aclcheck_result = pg_aclcheck(seqrel, GetPgUserName(), - (((funcid == F_NEXTVAL) || (funcid == F_SETVAL)) ? - ACL_WR : ACL_RD))) - != ACLCHECK_OK) - elog(ERROR, "%s.%s: %s", - seqrel, funcname, aclcheck_error_strings[aclcheck_result]); - - pfree(seqrel); - - if (funcid == F_NEXTVAL && pstate->p_in_where_clause) - elog(ERROR, "Sequence function nextval is not allowed in WHERE clauses"); - if (funcid == F_SETVAL && pstate->p_in_where_clause) - elog(ERROR, "Sequence function setval is not allowed in WHERE clauses"); - } + if (funcid == F_NEXTVAL && pstate->p_in_where_clause) + elog(ERROR, "Sequence function nextval is not allowed in WHERE clauses"); + if (funcid == F_SETVAL && pstate->p_in_where_clause) + elog(ERROR, "Sequence function setval is not allowed in WHERE clauses"); expr = makeNode(Expr); expr->typeOid = rettype; diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h index 99b164c44c..e870737bf2 100644 --- a/src/include/catalog/pg_proc.h +++ b/src/include/catalog/pg_proc.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2000, PostgreSQL, Inc * Portions Copyright (c) 1994, Regents of the University of California * - * $Id: pg_proc.h,v 1.137 2000/06/09 01:11:10 tgl Exp $ + * $Id: pg_proc.h,v 1.138 2000/06/11 20:07:51 tgl Exp $ * * NOTES * The script catalog/genbki.sh reads this file and generates .bki @@ -2003,12 +2003,12 @@ DESCR("convert int8 to int8 (no-op)"); /* SEQUENCEs nextval & currval functions */ -DATA(insert OID = 1574 ( nextval PGUID 11 f t f t 1 f 23 "25" 100 0 0 100 nextval - )); +DATA(insert OID = 1574 ( nextval PGUID 12 f t f t 1 f 23 "25" 100 0 0 100 nextval - )); DESCR("sequence next value"); -DATA(insert OID = 1575 ( currval PGUID 11 f t f t 1 f 23 "25" 100 0 0 100 currval - )); +DATA(insert OID = 1575 ( currval PGUID 12 f t f t 1 f 23 "25" 100 0 0 100 currval - )); DESCR("sequence current value"); -DATA(insert OID = 1576 ( setval PGUID 11 f t f t 2 f 23 "25 23" 100 0 0 100 setval - )); -DESCR("sequence set value"); +DATA(insert OID = 1576 ( setval PGUID 12 f t f t 2 f 23 "25 23" 100 0 0 100 setval - )); +DESCR("set sequence value"); DATA(insert OID = 1579 ( varbit_in PGUID 11 f t t t 1 f 1562 "0" 100 0 0 100 varbit_in - )); DESCR("(internal)"); diff --git a/src/include/commands/sequence.h b/src/include/commands/sequence.h index f602a6cb39..d695caa2df 100644 --- a/src/include/commands/sequence.h +++ b/src/include/commands/sequence.h @@ -9,10 +9,11 @@ #ifndef SEQUENCE_H #define SEQUENCE_H +#include "fmgr.h" #include "nodes/parsenodes.h" /* - * Columns of a sequnece relation + * Columns of a sequence relation */ #define SEQ_COL_NAME 1 @@ -27,10 +28,11 @@ #define SEQ_COL_FIRSTCOL SEQ_COL_NAME #define SEQ_COL_LASTCOL SEQ_COL_CALLED +extern Datum nextval(PG_FUNCTION_ARGS); +extern Datum currval(PG_FUNCTION_ARGS); +extern Datum setval(PG_FUNCTION_ARGS); + extern void DefineSequence(CreateSeqStmt *stmt); -extern int4 nextval(struct varlena * seqname); -extern int4 currval(struct varlena * seqname); -extern int4 setval(struct varlena * seqname, int4 next); extern void CloseSequences(void); #endif /* SEQUENCE_H */ diff --git a/src/test/regress/regress.c b/src/test/regress/regress.c index 18fb2d1996..c2ffb81ba2 100644 --- a/src/test/regress/regress.c +++ b/src/test/regress/regress.c @@ -1,5 +1,5 @@ /* - * $Header: /cvsroot/pgsql/src/test/regress/regress.c,v 1.38 2000/06/05 07:29:22 tgl Exp $ + * $Header: /cvsroot/pgsql/src/test/regress/regress.c,v 1.39 2000/06/11 20:07:44 tgl Exp $ */ #include /* faked on sunos */ @@ -8,6 +8,7 @@ #include "utils/geo_decls.h" /* includes */ #include "executor/executor.h" /* For GetAttributeByName */ +#include "commands/sequence.h" /* for nextval() */ #define P_MAXDIG 12 #define LDELIM '(' @@ -420,8 +421,6 @@ funny_dup17(PG_FUNCTION_ARGS) extern Datum ttdummy(PG_FUNCTION_ARGS); int32 set_ttdummy(int32 on); -extern int4 nextval(struct varlena * seqin); - #define TTDUMMY_INFINITY 999999 static void *splan = NULL; @@ -528,9 +527,10 @@ ttdummy(PG_FUNCTION_ARGS) } { - struct varlena *seqname = textin("ttdummy_seq"); + text *seqname = textin("ttdummy_seq"); - newoff = nextval(seqname); + newoff = DirectFunctionCall1(nextval, + PointerGetDatum(seqname)); pfree(seqname); }