Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
Bruce Momjian writes: > Tom Lane wrote: >> Just ignore the inapplicable permissions during pg_dump. I think you're >> making this harder than it needs to be... > I don't think we should allow GRANT DELETE ON seq in 8.2 for invalid > permission. That's fine, but pg_dump has to continue to work against old servers, so it's going to have to be coded to ignore inapplicable permissions anyway. Contorting the server-side code to avoid that is pointless. > Ignoring your insult, the code is structured this way: > check all permission bits > call object-type-specific routine > loop over each object and set permission bits > so, to fix this, I would need to move the permission bit checks into > object-type-specific routines so that I could check the permission bits > for each object, rather than once in a single place. You'd have to allow the union of relation and sequence rights during the conversion to bitmask form in ExecuteGrantStmt, and then check more closely inside the per-object loop in ExecGrant_Relation, but that doesn't seem like a showstopper to me. It certainly seems more pleasant than exposing bizarre restrictions to users because we're sharing code between the cases. regards, tom lane ---(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
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
Tom Lane wrote:
> Bruce Momjian writes:
> > At first I was just going to continue allowing table-like permissions
> > for sequences if a GRANT [TABLE] was used, and add the new
> > USAGE/SELECT/UPDATE capability only for GRANT SEQUENCE. The problem was
> > that you could create a non-dumpable permission setup if you added
> > DELETE permission to a sequence using GRANT TABLE, and USAGE permission
> > using GRANT SEQUENCE. That couldn't be dumped with TABLE or with
> > SEQUENCE, and I didn't want to do a double-dump of GRANT to fit that,
> > nor did I want to throw an warning during the dump run.
>
> Just ignore the inapplicable permissions during pg_dump. I think you're
> making this harder than it needs to be...
I don't think we should allow GRANT DELETE ON seq in 8.2 for invalid
permission. If we are going to say what permissions are supported by
sequences, we should enforce that rather than silently accept it, and
once we decide that, we need infrastructure to treat sequences and
non-sequences differently.
The dump illustration is just another example of why we have to tighten
this. I would be OK to allow it and preserve it, but not to allow it
and ignore it in a dump, basically throwing it away from dump to reload.
The fact we can't preserve it drove me in this direction.
> > test=> REVOKE DELETE ON seq, tab FROM PUBLIC;
> > WARNING: invalid privilege type DELETE for sequence
> > ERROR: DELETE privilege invalid for command mixing sequences and
> > non-sequences
>
> This is just plain silly. If you're going to go to that length, why not
> rearrange the code to avoid the problem instead?
Ignoring your insult, the code is structured this way:
check all permission bits
call object-type-specific routine
loop over each object and set permission bits
so, to fix this, I would need to move the permission bit checks into
object-type-specific routines so that I could check the permission bits
for each object, rather than once in a single place. You could still do
that single permission check in each object-type-specific routine and
just do the loop for the GRANT TABLE case. Does that seem worth it?
Another solution would be to save the permission bits in a List one per
object rather than as a single value for all objects, but that mucks up
the API for all objects just to catch the sequence case of GRANT TABLE.
> > Would someone look at the change in src/backend/catalog/pg_shdepend.c
> > for shared dependencies? We don't have any system catalog sequences let
> > alone any shared catalog sequences, so I assume we are OK with assuming
> > it is a relation.
>
> We might have such in the future though.
The problem is that the case statement triggers off of
RelationRelationId:
switch (sdepForm->classid)
{
case RelationRelationId:
/* could be a sequence? */
istmt.objtype = ACL_OBJECT_RELATION;
The problem is that pg_class holds both sequences and non-sequences, so
I am suspecting the fix will require another field in the pg_shdepend
table, which seems wasteful considering we have no shared catalog
sequences.
--
Bruce Momjian| http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] TODO-item: Add sleep() function, remove from regress.c
BTW, a serious problem with just passing it off to pg_usleep like that is that the sleep can't be aborted by a cancel request; doesn't seem like a good idea to me. I'd suggest writing a loop that sleeps for at most a second at a time, with a CHECK_FOR_INTERRUPTS() before each sleep. This would also allow you to eliminate the arbitrary restriction on length of sleep. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] TODO-item: Add sleep() function, remove from regress.c
Joachim Wieland <[EMAIL PROTECTED]> writes: > I'd personally prefer to call the function pg_sleep(), but since it is > called sleep() on the TODO list and in previous discussions, I kept the > name. The internal function is called pg_sleep() however. pg_sleep seems like a better idea to me too. Why is the function defined to take numeric rather than float8? float8 is a whole lot easier to work with internally. (Performance doesn't seem like an issue here, but length and readability of the code are worth worrying about.) Further, you could avoid assuming that the machine has working int64 arithmetic, which is an assumption I still think we should avoid everywhere that it's not absolutely essential. The proposed regression test seems unacceptably fragile, as well as rather pointless. regards, tom lane ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
Bruce Momjian writes: > At first I was just going to continue allowing table-like permissions > for sequences if a GRANT [TABLE] was used, and add the new > USAGE/SELECT/UPDATE capability only for GRANT SEQUENCE. The problem was > that you could create a non-dumpable permission setup if you added > DELETE permission to a sequence using GRANT TABLE, and USAGE permission > using GRANT SEQUENCE. That couldn't be dumped with TABLE or with > SEQUENCE, and I didn't want to do a double-dump of GRANT to fit that, > nor did I want to throw an warning during the dump run. Just ignore the inapplicable permissions during pg_dump. I think you're making this harder than it needs to be... > test=> REVOKE DELETE ON seq, tab FROM PUBLIC; > WARNING: invalid privilege type DELETE for sequence > ERROR: DELETE privilege invalid for command mixing sequences and > non-sequences This is just plain silly. If you're going to go to that length, why not rearrange the code to avoid the problem instead? > Would someone look at the change in src/backend/catalog/pg_shdepend.c > for shared dependencies? We don't have any system catalog sequences let > alone any shared catalog sequences, so I assume we are OK with assuming > it is a relation. We might have such in the future though. regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] Proposed patch to change "missing FROM" messages
Michael Glaesemann <[EMAIL PROTECTED]> writes: > For clarity, I'd rewrite this hint as > There is an entry for table "a", ... Done. Anyone have any other suggestions for wording improvements? regards, tom lane ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
Bruce Momjian wrote: > Tom Lane wrote: > > I wrote: > > > Bruce Momjian writes: > > >> Does the standard require USAGE to support currval? > > > > > currval isn't in the standard (unless I missed something), so it has > > > nothing to say one way or the other on the point. > > > > Wait, I take that back. Remember our previous discussions about this > > point: the spec's NEXT VALUE FOR construct is *not* equivalent to > > nextval, because they specify that the sequence advances just once per > > command even if the command says NEXT VALUE FOR in multiple places. > > This means that NEXT VALUE FOR is effectively both nextval and currval; > > the first one in a command does nextval and the rest do currval. > > > > Accordingly, I think it's reasonable to read the spec as saying that > > USAGE privilege encompasses both nextval and currval. > > Here's a patch that more closely matches the ideas proposed. Here is an updated patch. I hit a few issues. At first I was just going to continue allowing table-like permissions for sequences if a GRANT [TABLE] was used, and add the new USAGE/SELECT/UPDATE capability only for GRANT SEQUENCE. The problem was that you could create a non-dumpable permission setup if you added DELETE permission to a sequence using GRANT TABLE, and USAGE permission using GRANT SEQUENCE. That couldn't be dumped with TABLE or with SEQUENCE, and I didn't want to do a double-dump of GRANT to fit that, nor did I want to throw an warning during the dump run. What I did was to throw a warning if an invalid permission is specified for a sequence in GRANT TABLE. By doing this, un-dumpable permission combinations will not be loaded into an 8.2 database. (GRANT ALL ON TABLE sets the sequence-only permissions.) test=> GRANT DELETE ON seq TO PUBLIC; WARNING: invalid privilege type DELETE for sequence WARNING: no privileges were granted GRANT test=> GRANT DELETE,SELECT ON seq TO PUBLIC; WARNING: invalid privilege type DELETE for sequence GRANT This seemed the safest backward-compatible setup. It will have to be mentioned in the release notes so users know they might get warnings from loading sequences into 8.2. You might think that it is unlikely for a DELETE permission to be assigned to a sequences, but a simple GRANT ALL and REVOKE INSERT in 8.1 will cause: test=> CREATE TABLE tab(x INTEGER); CREATE TABLE test=> GRANT ALL ON tab TO PUBLIC; GRANT test=> REVOKE INSERT ON tab FROM PUBLIC; REVOKE yields in pg_dump output: GRANT SELECT,RULE,UPDATE,DELETE,REFERENCES,TRIGGER ON TABLE tab TO PUBLIC; This test was done on a table, but in 8.1 the same would appear for a sequence with these warnings on load into 8.2: WARNING: invalid privilege type RULE for sequence WARNING: invalid privilege type DELETE for sequence WARNING: invalid privilege type REFERENCES for sequence WARNING: invalid privilege type TRIGGER for sequence GRANT Another tricky case was this: test=> GRANT DELETE ON tab, seq TO PUBLIC; GRANT allows multiple objects to be listed, as illustrated above. The current code checks for valid permissions in one place because it assumes all listed objects are of the same type and accept the same permissions. Because GRANT TABLE must allow only valid permissions for sequences (to avoid un-dumpable output) I had to throw an error if a sequence is mixed with a non-sequence, and the permission did not apply to both sequences and non-sequences, rather than throw a warning like I usually do for invalid sequence permissions: test=> REVOKE DELETE ON seq, tab FROM PUBLIC; WARNING: invalid privilege type DELETE for sequence ERROR: DELETE privilege invalid for command mixing sequences and non-sequences test=> REVOKE SELECT ON tab, seq FROM PUBLIC; REVOKE Because allowing sequences to use GRANT TABLE is only for backward compatibility, I think this is fine. If not, we would have to split apart the permission checking for tables from the existing routine, and lose modularity in the code. This patch also contains Marko's documentation adjustments. Would someone look at the change in src/backend/catalog/pg_shdepend.c for shared dependencies? We don't have any system catalog sequences let alone any shared catalog sequences, so I assume we are OK with assuming it is a relation. I added a comment just in case. -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 Index: doc/src/sgml/ref/grant.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/ref/grant.sgml,v retrieving revision 1.50 diff -c -c -r
Re: [PATCHES] pl/python refcount bug
On Mon, 2006-01-09 at 13:07 -0500, Neil Conway wrote: > The fix is simple: set the local pointer to the current argument to NULL > immediately after adding it to the argument list. This ensures that the > Py_XDECREF() in the PG_CATCH() block doesn't double-decrement. I'd like > to apply this to HEAD and back branches (as far back as PG_CATCH > exists). Applied to HEAD, REL8_1_STABLE, and REL8_0_STABLE. -Neil ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Proposed patch to change "missing FROM" messages
On Jan 10, 2006, at 8:32 , Tom Lane wrote: Attached is a proposed change to create hopefully-more-useful error messages in the cases where we currently say "missing FROM-clause entry". It's good to have these hints for users. Thanks, Tom. Patch: regression=# select * from a,b join c on (a.aa = c.cc); ERROR: invalid reference to FROM-clause entry for table "a" HINT: There is an entry for "a", but it cannot be referenced from this part of the query. For clarity, I'd rewrite this hint as There is an entry for table "a", ... Michael Glaesemann grzm myrealbox com ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
[PATCHES] TODO-item: Add sleep() function, remove from regress.c
I propose the appended patch for the todo item:
* Add sleep() function, remove from regress.c
The patch also removes do_sleep() from the regression and substitues this
with the call to the new function.
I'd personally prefer to call the function pg_sleep(), but since it is
called sleep() on the TODO list and in previous discussions, I kept the
name. The internal function is called pg_sleep() however.
There was a discussion about this item in August of last year
http://archives.postgresql.org/pgsql-hackers/2005-08/msg00633.php
A few people liked the idea, Tom Lane pointed out that it's bad to have a
sleeping backend that can hold locks for a long time. Other developers
answered that this should just be documented since the user can do that
anyways very easily if he is allowed to create functions in one of the
traditional scripting languages or with busy waiting also in plpgsql.
Finally the item got added to the TODO list and nobody objected.
In the mysqlcompat project there is a sleep function that does busy waiting,
this function could profit from a sleep() function in the backend.
To the docs I introduced a new paragraph such that it is easy to find this
function and labeled the note about the locks with the "warning" tag.
Tom, can you live with that?
Other Comments?
Joachim
diff -cr cvs/pgsql/doc/src/sgml/func.sgml cvs.build/pgsql/doc/src/sgml/func.sgml
*** cvs/pgsql/doc/src/sgml/func.sgml2005-12-28 02:29:58.0 +0100
--- cvs.build/pgsql/doc/src/sgml/func.sgml 2006-01-10 00:40:05.0
+0100
***
*** 6170,6175
--- 6170,6223
+
+
+Delaying the execution with sleep
+
+
+ sleep
+
+
+ delay
+
+
+
+ The following function is available to delay the execution of the
+ backend:
+
+ sleep (seconds)
+
+
+ sleep(seconds) makes the
+ current backend sleep until seconds seconds
have
+ elapsed. Note that the argument seconds is
given
+ in the numeric type. Thus you can also sleep for fractions of
+ seconds.
+
+
+ SELECT sleep(0.5);
+
+
+
+
+
+ The exact time that the process sleeps depends on the operating system.
+ On a busy system the backend process might get suspended for a longer
+ time than specified, especially if the priority of the backend process
is
+ low compared to other processes.
+
+
+
+
+ Make sure that your backend does not hold more locks than necessary
+ before calling
+ sleep(seconds), since
+ this could slow down your whole system. Other backends might have to
wait
+ for locks your sleeping backend still holds.
+
+
+
+
diff -cr cvs/pgsql/src/backend/utils/adt/misc.c
cvs.build/pgsql/src/backend/utils/adt/misc.c
*** cvs/pgsql/src/backend/utils/adt/misc.c 2005-10-15 04:49:29.0
+0200
--- cvs.build/pgsql/src/backend/utils/adt/misc.c2006-01-10
00:34:36.0 +0100
***
*** 28,33
--- 28,34
#include "storage/pmsignal.h"
#include "storage/procarray.h"
#include "utils/builtins.h"
+ #include "utils/numeric.h"
#define atooid(x) ((Oid) strtoul((x), NULL, 10))
***
*** 259,261
--- 260,315
FreeDir(fctx->dirdesc);
SRF_RETURN_DONE(funcctx);
}
+
+ /*
+ * pg_sleep - delay for N seconds (N is numeric)
+ *
+ * This function gets exposed to the user as "sleep()"
+ *
+ * Note that pg_usleep() will abort when receiving a SIGHUP for example since
+ * it is implemented by means of select().
+ */
+ Datum
+ pg_sleep(PG_FUNCTION_ARGS)
+ {
+ Numeric secs;
+ Numeric usecs;
+ int64 to_sleep;
+
+ if (PG_ARGISNULL(0))
+ /* have to return NULL here to comply with the strictness
property */
+ PG_RETURN_NULL();
+
+ secs = PG_GETARG_NUMERIC(0);
+
+ /* if we should sleep for example 2.5 seconds, we first calculate:
+* 2.5 * 1,000,000 = 2,500,000
+* Then we calculate the ceiling of this and transform it to an int64
for
+* easier calculations.
+*/
+ usecs = DatumGetNumeric(DirectFunctionCall1(int8_numeric,
+
Int64GetDatum(INT64CONST(100;
+ usecs = DatumGetNumeric(DirectFunctionCall2(numeric_mul,
+
NumericGetDatum(secs),
+
NumericGetDatum(usecs)));
+ usecs = DatumGetNumeric(DirectFunctionCall1(numeric_ceil,
+
NumericGetDatum(usecs)));
+ to_sleep = DatumGetInt64(DirectFunctionCall1(numeric_int8,
+
[PATCHES] Proposed patch to change "missing FROM" messages
Attached is a proposed change to create hopefully-more-useful error messages in the cases where we currently say "missing FROM-clause entry". Some examples of what it does: Patch: regression=# select * from a,b join c on (a.aa = c.cc); ERROR: invalid reference to FROM-clause entry for table "a" HINT: There is an entry for "a", but it cannot be referenced from this part of the query. 8.1: regression=# select * from a,b join c on (a.aa = c.cc); ERROR: missing FROM-clause entry for table "a" 8.0: regression=# select * from a,b join c on (a.aa = c.cc); NOTICE: adding missing FROM-clause entry for table "a" ERROR: JOIN/ON clause refers to "a", which is not part of JOIN Patch: regression=# select a.* from a b; ERROR: invalid reference to FROM-clause entry for table "a" HINT: Perhaps you meant to reference the table alias "b". 8.1: regression=# select a.* from a b; ERROR: missing FROM-clause entry for table "a" The above happens only because there is a table "a"; 8.1 behaves differently when there isn't such a table: Patch: regression=# select nosuch.* from a b; ERROR: missing FROM-clause entry for table "nosuch" 8.1: regression=# select nosuch.* from a b; ERROR: relation "nosuch" does not exist (This happens because 8.1 tries to create the RTE before it does warnAutoRange. This change in behavior is a side-effect of having to reorder the operations, but it doesn't seem worse to me.) The same hints are issued as part of the NOTICE when add_missing_from is ON, but I did not change the main text of the NOTICE messages. Also, the error message or notice is unchanged from 8.1 if the code can't find any RTE that the reference plausibly could have been meant to match. I'd like to apply this to 8.1 branch as well as HEAD. This would create a need for some new message translations, but there should be time for translators to do that before 8.1.3, if they are so inclined. Comments? regards, tom lane binIs3bSfiRpk.bin Description: missing-from.patch ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] CREATEUSER == SUPERUSER?
Yoshiyuki Asaba <[EMAIL PROTECTED]> writes: > The following command makes a superuser. Is this correct? > template1=# CREATE USER xyz CREATEUSER; Yes, read the CREATE ROLE documentation: CREATEUSER NOCREATEUSER These clauses are an obsolete, but still accepted, spelling of SUPERUSER and NOSUPERUSER. Note that they are not equivalent to CREATEROLE as one might naively expect! > I think CREATEUSER keyword is equal to CREATEROLE. The proposed patch breaks backward compatibility, which is the only reason we still allow these keywords at all. regards, tom lane ---(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
Re: [PATCHES] plpgsql: check domain constraints
Neil Conway <[EMAIL PROTECTED]> writes: > GetDomainConstraints() looks fairly expensive, so it would be nice to do > some caching. What would the best way to implement this be? I had > thought that perhaps the typcache would work, but there seems to be no > method to flush stale typcache data. Perhaps we could add support for > typcache invalidation (via a new sinval message), and then add the > domain constraint information to the typcache. Or is there an easier > way? Yeah, I had been thinking of exactly the same thing a few months ago after noting that GetDomainConstraints() can be pretty dang slow --- it seemed to be a major bottleneck for Kevin Grittner here: http://archives.postgresql.org/pgsql-hackers/2005-09/msg01135.php Unfortunately the rest of that conversation was unintentionally off-list, but we identified heavy use of pg_constraint_contypid_index as the source of his issue, and I said : Hmm. The only commonly-used code path I can see that would touch : pg_constraint_contypid_index is GetDomainConstraints(), which would be : called (once) during startup of a command that involves a CoerceToDomain : expression node. So if the heavily-hit table has any domain-type : columns, it's possible that a steady stream of inserts or updates : could have kept that index tied up. : : It might be worth introducing a system cache that could be used to : extract the constraints for a domain without going to the catalog for : every single command. There's been very little work done to date on : optimizing operations on domains :-( The lack of typcache invalidation is something that will eventually bite us in other ways, so we need to add the facility anyway. We don't really need a "new sinval message", as an inval on the pg_type row will serve perfectly well --- what we need is something comparable to CacheInvalidateRelcache() to cause such a message to be sent when we haven't actually changed the pg_type row itself. Do you want to work on this? I can if you don't. BTW, in connection with the lookup_rowtype_tupdesc fiasco, it's pretty obvious that any data structure returned by this function will need to be either copied or reference-counted. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
[PATCHES] pl/python refcount bug
In PLy_function_build_args(), the code loops repeatedly, constructing one argument at a time and then inserting the argument into a Python list via PyList_SetItem(). This "steals" the reference to the argument: that is, the reference to the new list member is now held by the Python list itself. This works fine, except if an elog occurs. This causes the function's PG_CATCH() block to be invoked, which decrements the reference counts on both the current argument and the list of arguments. If the elog happens to occur during the second or subsequent iteration of the loop, the reference count on the current argument will be decremented twice. The fix is simple: set the local pointer to the current argument to NULL immediately after adding it to the argument list. This ensures that the Py_XDECREF() in the PG_CATCH() block doesn't double-decrement. I'd like to apply this to HEAD and back branches (as far back as PG_CATCH exists). The broader point is that the current approach to handling reference counting and exceptions in PL/Python seems terribly error-prone. I briefly skimmed the code for other instances of the problem -- while I didn't find any, I don't have a lot of confidence that similar issues don't exist. Any thoughts on how to improve that? (I wonder if we could adapt ResOwner...) -Neil *** src/pl/plpython/plpython.c caab6efbac99de55d61348c6467b72b169c72199 --- src/pl/plpython/plpython.c 29438f318ecb215d1af5aeddd8c4304352d432ac *** *** 895,900 --- 895,901 * FIXME -- error check this */ PyList_SetItem(args, i, arg); + arg = NULL; } } PG_CATCH(); ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
[PATCHES] CREATEUSER == SUPERUSER?
Hi,
The following command makes a superuser. Is this correct?
template1=# CREATE USER xyz CREATEUSER;
CREATE ROLE
template1=# select rolname,rolsuper from pg_roles where rolname = 'xyz';
rolname | rolsuper
-+--
xyz | t
(1 row)
I think CREATEUSER keyword is equal to CREATEROLE.
Regards,
--
Yoshiyuki Asaba
[EMAIL PROTECTED]
Index: gram.y
===
RCS file: /projects/cvsroot/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.521
diff -c -r2.521 gram.y
*** gram.y 29 Dec 2005 04:53:18 - 2.521
--- gram.y 9 Jan 2006 15:18:51 -
***
*** 664,675
}
| CREATEUSER
{
! /* For backwards compatibility, synonym
for SUPERUSER */
! $$ = makeDefElem("superuser", (Node
*)makeInteger(TRUE));
}
| NOCREATEUSER
{
! $$ = makeDefElem("superuser", (Node
*)makeInteger(FALSE));
}
| LOGIN_P
{
--- 664,675
}
| CREATEUSER
{
! /* For backwards compatibility, synonym
for CREATEROLE */
! $$ = makeDefElem("createrole", (Node
*)makeInteger(TRUE));
}
| NOCREATEUSER
{
! $$ = makeDefElem("createrole", (Node
*)makeInteger(FALSE));
}
| LOGIN_P
{
---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly
Re: [PATCHES] plpgsql: check domain constraints
On Sun, 2006-01-08 at 23:56 -0500, Tom Lane wrote: > We have gone out of our way to make sure that domain constraint checking > responds promptly (ie, within one query) to updates of the domain > definition. Caching at the session level in plpgsql would be a > significant regression from that, and I don't think it's acceptable. > If you had a way of invalidating the cache when needed, it'd be great > ... but not without that. GetDomainConstraints() looks fairly expensive, so it would be nice to do some caching. What would the best way to implement this be? I had thought that perhaps the typcache would work, but there seems to be no method to flush stale typcache data. Perhaps we could add support for typcache invalidation (via a new sinval message), and then add the domain constraint information to the typcache. Or is there an easier way? > > I also made a few semi-related cleanups. In pl_exec.c, it seems to me > > that estate.eval_econtext MUST be non-NULL during the guts of both > > plpgsql_exec_trigger() and plpgsql_exec_function(). Therefore checking > > that estate.eval_econtext is non-NULL when cleaning up is unnecessary > > (line 649 and 417 in current sources, respectively), so I've removed the > > checks. Am I missing something? > > The code doesn't currently assume that, and it doesn't seem to me that > saving one simple if-test is a reason to add the assumption ... It's not a matter of "saving an if-test", it's a matter of code clarity. Code ought to be consistent about whether any given pointer variable might be NULL. This makes it easier for a programmer to tell if new code ought to check for NULL-ness before using the pointer. (In fact, when modifying plpgsql_exec_function() for this patch, I had to spend a few minutes checking for the situations in which estate.eval_econtext might be NULL.) Some languages (e.g. ML) actually distinguish in the type system between "references that might be NULL" and those that cannot be. That's not possible in C, but consistently treating NULL-ness makes it easier to determine this by hand. -Neil ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
