From: | Neil Conway <neilc(at)samurai(dot)com> |
---|---|
To: | Dennis Bjorklund <db(at)zigo(dot)dhs(dot)org> |
Cc: | pgsql-patches(at)postgresql(dot)org |
Subject: | Re: lastval() |
Date: | 2005-05-19 03:15:33 |
Message-ID: | 428C04D5.2060807@samurai.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers pgsql-patches |
Dennis Bjorklund wrote:
> + Datum
> + lastval(PG_FUNCTION_ARGS)
> + {
> + Relation seqrel;
> + int64 result;
> +
> + if (last_used_seq == NULL) {
> + ereport(ERROR,
> + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> + errmsg("nextval have not been used in the current session")));
> + }
"has not been" would be more better English.
> +
> + seqrel = relation_open(last_used_seq->relid, NoLock);
> +
> + acquire_share_lock (seqrel, last_used_seq);
> +
> + if (pg_class_aclcheck(last_used_seq->relid, GetUserId(), ACL_SELECT) != ACLCHECK_OK)
> + ereport(ERROR,
> + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
> + errmsg("permission denied for sequence with OID %d",
> + last_used_seq->relid)));
"%d" is always the wrong formatting sequence for OIDs (they are
unsigned, hence %u). But in any case user-visible error messages should
specify the name of the sequence, which you can get via
RelationGetRelationName(seqrel)
> +
> + if (last_used_seq->increment == 0) /* nextval/read_info were not called */
> + ereport(ERROR,
> + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> + errmsg("currval of sequence with OID %d is not yet defined in this session",
> + last_used_seq->relid)));
See above; however, when will this error actually be invoked? (The
comment is wrong, as last_used_seq won't be defined if nextval has not
been called.)
> /*
> + * If we haven't touched the sequence already in this transaction,
> + * we need to acquire AccessShareLock. We arrange for the lock to
> + * be owned by the top transaction, so that we don't need to do it
> + * more than once per xact.
> + */
> + static void
> + acquire_share_lock (Relation seqrel,
> + SeqTableData *data)
Confusing SeqTable and SeqTableData * is bad style. I personally don't
like putting pointers into typedefs, but since the PG code does this,
SeqTable should be used consistently rather than SeqTableData *. The
same applies to the definition of "last_used_seq".
Comments on behavior:
neilc=# select setval('foo', 500);
setval
--------
500
(1 row)
neilc=# select lastval();
lastval
---------
500
(1 row)
I'm not sure it's necessarily _wrong_ to update lastval() on both setval
and nextval, but if that's the behavior we're going to implement, it
should surely be documented.
neilc=# create sequence bar ; select nextval ('bar') ; drop sequence bar;
CREATE SEQUENCE
nextval
---------
1
(1 row)
DROP SEQUENCE
neilc=# select lastval();
ERROR: XX000: could not open relation with OID 16389
Needs a friendlier error message.
-Neil
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2005-05-19 05:37:29 | Re: Learning curves and such (was Re: pgFoundry) |
Previous Message | Tom Lane | 2005-05-19 03:00:00 | Re: Two-phase commit issues |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2005-05-19 03:31:27 | Re: md5(bytea) |
Previous Message | Abhijit Menon-Sen | 2005-05-19 02:58:10 | md5(bytea) |