[PATCHES] enable/disable trigger (Re: Fwd: [HACKERS] Open items)
Hi all,
Here is a first patch to allow these commands.
ALTER TABLE table ENABLE TRIGGER trigname
ALTER TABLE table DISABLE TRIGGER trigname
Bruce said to allow them only super-user,
but currently this patch allows also the table owner.
If we need to restrict these operations,
we have to add more user checks.
From: Bruce Momjian pgman@candle.pha.pa.us
Date: 2005/06/29 20:49
Subject: Re: [HACKERS] Open items
To: Satoshi Nagayasu [EMAIL PROTECTED]
Cc: PostgreSQL-development pgsql-hackers@postgresql.org
Satoshi Nagayasu wrote:
How about enable/disable triggers?
From TODO:
Allow triggers to be disabled.
http://momjian.postgresql.org/cgi-bin/pgtodo?trigger
I think this is good for COPY performance improvement.
Now I have user functions to enable/disable triggers, not DDL.
It modifies system tables.
But I can rewrite this as a DDL. (ALTER TABLE?)
Yea, it is a TODO item, and should be pretty straight-forward to code,
so sure, go ahead.
It has to be something that is super-user-only.
--
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 4: Don't 'kill -9' the postmaster
--
NAGAYASU Satoshi [EMAIL PROTECTED]
diff -ru pgsql.orig/src/backend/commands/tablecmds.c
pgsql/src/backend/commands/tablecmds.c
--- pgsql.orig/src/backend/commands/tablecmds.c 2005-06-28 14:08:54.0
+0900
+++ pgsql/src/backend/commands/tablecmds.c 2005-07-01 15:50:27.0
+0900
@@ -236,6 +236,8 @@
Oid newOwnerId);
static void ATExecClusterOn(Relation rel, const char *indexName);
static void ATExecDropCluster(Relation rel);
+static void ATExecEnableDisableTrigger(Relation rel, char *trigname,
+ bool
enable);
static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
char *tablespacename);
static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace);
@@ -1993,6 +1995,11 @@
}
pass = AT_PASS_DROP;
break;
+ case AT_EnableTrig: /* ENABLE TRIGGER */
+ case AT_DisableTrig:/* DISABLE TRIGGER */
+ ATSimplePermissions(rel, false);
+ pass = AT_PASS_MISC;
+ break;
case AT_SetTableSpace: /* SET TABLESPACE */
/* This command never recurses */
ATPrepSetTableSpace(tab, rel, cmd-name);
@@ -2155,6 +2162,12 @@
* Nothing to do here; Phase 3 does the work
*/
break;
+ case AT_EnableTrig: /* ENABLE TRIGGER */
+ ATExecEnableDisableTrigger(rel, cmd-name, true);
+ break;
+ case AT_DisableTrig:/* DISABLE TRIGGER */
+ ATExecEnableDisableTrigger(rel, cmd-name, false);
+ break;
default:/* oops */
elog(ERROR, unrecognized alter table type: %d,
(int) cmd-subtype);
@@ -5465,6 +5478,15 @@
}
/*
+ * ALTER TABLE ENABLE/DISABLE TRIGGER
+ */
+static void
+ATExecEnableDisableTrigger(Relation rel, char *trigname, bool enable)
+{
+ EnableDisableTrigger(rel, trigname, enable);
+}
+
+/*
* ALTER TABLE SET TABLESPACE
*/
static void
diff -ru pgsql.orig/src/backend/commands/trigger.c
pgsql/src/backend/commands/trigger.c
--- pgsql.orig/src/backend/commands/trigger.c 2005-05-30 16:20:58.0
+0900
+++ pgsql/src/backend/commands/trigger.c2005-07-01 15:53:45.0
+0900
@@ -3063,3 +3063,74 @@
afterTriggerAddEvent(new_event);
}
}
+
+/* --
+ * EnableDisableTrigger()
+ *
+ * Called by ALTER TABLE ENABLE/DISABLE TRIGGER
+ * to change 'tgenabled' flag in the pg_trigger.
+ * --
+ */
+void
+EnableDisableTrigger(Relation rel, const char *tgname, bool enable)
+{
+ Relation tgrel;
+ SysScanDesc tgscan;
+ ScanKeyData keys[2];
+ HeapTuple tuple;
+
+ /* Permissions checks */
+ if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
+ aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
+ RelationGetRelationName(rel));
+
+ if (!allowSystemTableMods IsSystemRelation(rel))
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+
Re: [PATCHES] enable/disable trigger (Re: Fwd: [HACKERS] Open items)
ALTER TABLE table ENABLE TRIGGER trigname ALTER TABLE table DISABLE TRIGGER trigname Bruce said to allow them only super-user, but currently this patch allows also the table owner. The table owner can drop and create triggers - so why shouldn't they be able to enable and disable them? Chris ---(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] enable/disable trigger (Re: Fwd: [HACKERS] Open items)
The table owner can drop and create triggers - so why shouldn't they be able to enable and disable them? For convenience or easy operation. I believe the user doesn't like to create same triggers again and again. Christopher Kings-Lynne wrote: ALTER TABLE table ENABLE TRIGGER trigname ALTER TABLE table DISABLE TRIGGER trigname Bruce said to allow them only super-user, but currently this patch allows also the table owner. The table owner can drop and create triggers - so why shouldn't they be able to enable and disable them? Chris -- NAGAYASU Satoshi [EMAIL PROTECTED] ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] enable/disable trigger (Re: Fwd: [HACKERS] Open items)
Satoshi Nagayasu wrote: The table owner can drop and create triggers - so why shouldn't they be able to enable and disable them? For convenience or easy operation. I believe the user doesn't like to create same triggers again and again. I said why _shouldn't_. I was agreeing with you. Chris ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] enable/disable trigger (Re: Fwd: [HACKERS] Open items)
I said why _shouldn't_. I was agreeing with you. Oops. Sorry. I don't know why only super-user shold be able to enable/disable tirggers. I believe the table owner want to enable/disable triggers, because I always need it. Loading huge data set into the table with triggers(or fk) is very painful. Christopher Kings-Lynne wrote: Satoshi Nagayasu wrote: The table owner can drop and create triggers - so why shouldn't they be able to enable and disable them? For convenience or easy operation. I believe the user doesn't like to create same triggers again and again. I said why _shouldn't_. I was agreeing with you. Chris -- NAGAYASU Satoshi [EMAIL PROTECTED] ---(end of broadcast)--- TIP 3: 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] enable/disable trigger (Re: Fwd: [HACKERS] Open items)
Hi,
BTW, the Postgres convention is to submit patches in context diff format
(diff -c).
Satoshi Nagayasu wrote:
+ while (HeapTupleIsValid(tuple = systable_getnext(tgscan)))
+ {
+ HeapTuple newtup = heap_copytuple(tuple);
+ Form_pg_trigger pg_trigger = (Form_pg_trigger)
GETSTRUCT(newtup);
+
+ if ( pg_trigger-tgenabled==true enable==false )
+ {
+ pg_trigger-tgenabled = false;
+ }
+ else if ( pg_trigger-tgenabled==false enable==true )
+ {
+ pg_trigger-tgenabled = true;
+ }
+
+ simple_heap_update(tgrel, newtup-t_self, newtup);
Wouldn't it just be easier to set pg_trigger-tgenabled = enable ?
Also, you should probably skip the simple_heap_update() if the user
tries to disable an already-disabled trigger, to avoid pointless MVCC
bloat (and same for enabling an already-enabled trigger, of course).
--- pgsql.orig/src/backend/parser/gram.y2005-06-30 05:34:13.0
+0900
+++ pgsql/src/backend/parser/gram.y 2005-07-01 14:21:25.0 +0900
@@ -348,9 +348,9 @@
DATABASE DAY_P DEALLOCATE DEC DECIMAL_P DECLARE DEFAULT DEFAULTS
DEFERRABLE DEFERRED DEFINER DELETE_P DELIMITER DELIMITERS
- DESC DISTINCT DO DOMAIN_P DOUBLE_P DROP
+ DESC DISABLE DISTINCT DO DOMAIN_P DOUBLE_P DROP
- EACH ELSE ENCODING ENCRYPTED END_P ESCAPE EXCEPT EXCLUDING
+ EACH ELSE ENABLE ENCODING ENCRYPTED END_P ESCAPE EXCEPT EXCLUDING
EXCLUSIVE EXECUTE EXISTS EXPLAIN EXTERNAL EXTRACT
You also need to add these to unreserved_keywords (see gram.y:7903).
--- pgsql.orig/src/include/commands/trigger.h 2005-05-30 16:20:58.0
+0900
+++ pgsql/src/include/commands/trigger.h2005-07-01 15:50:09.0
+0900
@@ -164,6 +164,7 @@
extern void AfterTriggerSetState(ConstraintsSetStmt *stmt);
+void EnableDisableTrigger(Relation rel, const char *tgname, bool enable);
The Postgres convention is to use the extern keyword in declarations
of global functions.
If someone pg_dump's a table with a disabled trigger, should the dump
enable or disable the trigger? I'd be inclined to say pg_dump should
preserve the state of the database it is dumping, and so the trigger
should be disabled. In that case I believe pg_dump needs to be updated.
The patch needs to update the documentation and add regression tests.
psql tab completion might also be worth adding.
-Neil
---(end of broadcast)---
TIP 2: you can get off all lists at once with the unregister command
(send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [PATCHES] enable/disable trigger (Re: Fwd: [HACKERS] Open items)
Thanks for comments, Neil.
Some are fixed.
Neil Conway wrote:
Also, you should probably skip the simple_heap_update() if the user
tries to disable an already-disabled trigger, to avoid pointless MVCC
bloat (and same for enabling an already-enabled trigger, of course).
Do we need some log/warning message for the user in these cases?
If someone pg_dump's a table with a disabled trigger, should the dump
enable or disable the trigger? I'd be inclined to say pg_dump should
preserve the state of the database it is dumping, and so the trigger
should be disabled. In that case I believe pg_dump needs to be updated.
I think so too.
The patch needs to update the documentation and add regression tests.
psql tab completion might also be worth adding.
I'll dive into pg_dump and psql this weekend...
--
NAGAYASU Satoshi [EMAIL PROTECTED]
diff -cr pgsql.orig/src/backend/commands/tablecmds.c
pgsql/src/backend/commands/tablecmds.c
*** pgsql.orig/src/backend/commands/tablecmds.c 2005-06-28 14:08:54.0
+0900
--- pgsql/src/backend/commands/tablecmds.c 2005-07-01 15:50:27.0
+0900
***
*** 236,241
--- 236,243
Oid newOwnerId);
static void ATExecClusterOn(Relation rel, const char *indexName);
static void ATExecDropCluster(Relation rel);
+ static void ATExecEnableDisableTrigger(Relation rel, char *trigname,
+ bool
enable);
static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
char *tablespacename);
static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace);
***
*** 1993,1998
--- 1995,2005
}
pass = AT_PASS_DROP;
break;
+ case AT_EnableTrig: /* ENABLE TRIGGER */
+ case AT_DisableTrig:/* DISABLE TRIGGER */
+ ATSimplePermissions(rel, false);
+ pass = AT_PASS_MISC;
+ break;
case AT_SetTableSpace: /* SET TABLESPACE */
/* This command never recurses */
ATPrepSetTableSpace(tab, rel, cmd-name);
***
*** 2155,2160
--- 2162,2173
* Nothing to do here; Phase 3 does the work
*/
break;
+ case AT_EnableTrig: /* ENABLE TRIGGER */
+ ATExecEnableDisableTrigger(rel, cmd-name, true);
+ break;
+ case AT_DisableTrig:/* DISABLE TRIGGER */
+ ATExecEnableDisableTrigger(rel, cmd-name, false);
+ break;
default:/* oops */
elog(ERROR, unrecognized alter table type: %d,
(int) cmd-subtype);
***
*** 5465,5470
--- 5478,5492
}
/*
+ * ALTER TABLE ENABLE/DISABLE TRIGGER
+ */
+ static void
+ ATExecEnableDisableTrigger(Relation rel, char *trigname, bool enable)
+ {
+ EnableDisableTrigger(rel, trigname, enable);
+ }
+
+ /*
* ALTER TABLE SET TABLESPACE
*/
static void
diff -cr pgsql.orig/src/backend/commands/trigger.c
pgsql/src/backend/commands/trigger.c
*** pgsql.orig/src/backend/commands/trigger.c 2005-05-30 16:20:58.0
+0900
--- pgsql/src/backend/commands/trigger.c2005-07-01 17:21:44.0
+0900
***
*** 3063,3065
--- 3063,3132
afterTriggerAddEvent(new_event);
}
}
+
+ /* --
+ * EnableDisableTrigger()
+ *
+ *Called by ALTER TABLE ENABLE/DISABLE TRIGGER
+ * to change 'tgenabled' flag in the pg_trigger.
+ * --
+ */
+ void
+ EnableDisableTrigger(Relation rel, const char *tgname, bool enable)
+ {
+ Relation tgrel;
+ SysScanDesc tgscan;
+ ScanKeyData keys[2];
+ HeapTuple tuple;
+
+ /* Permissions checks */
+ if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
+ aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
+ RelationGetRelationName(rel));
+
+ if (!allowSystemTableMods IsSystemRelation(rel))
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+errmsg(permission denied: \%s\ is a system
catalog,
+ RelationGetRelationName(rel;
+
+ tgrel = heap_open(TriggerRelationId, RowExclusiveLock);
+
+ ScanKeyInit(keys[0],
+ Anum_pg_trigger_tgrelid,
+ BTEqualStrategyNumber, F_OIDEQ,
+
Re: [PATCHES] enable/disable trigger (Re: Fwd: [HACKERS] Open items)
There is one more fix...
+ tgscan = systable_beginscan(tgrel, TriggerRelidNameIndexId, true,
+ SnapshotNow, 1,
keys);
'1' was incorrect, must be '2'.
+ tgscan = systable_beginscan(tgrel, TriggerRelidNameIndexId, true,
+ SnapshotNow, 2,
keys);
--
NAGAYASU Satoshi [EMAIL PROTECTED]
diff -cr pgsql.orig/src/backend/commands/tablecmds.c
pgsql/src/backend/commands/tablecmds.c
*** pgsql.orig/src/backend/commands/tablecmds.c 2005-06-28 14:08:54.0
+0900
--- pgsql/src/backend/commands/tablecmds.c 2005-07-01 15:50:27.0
+0900
***
*** 236,241
--- 236,243
Oid newOwnerId);
static void ATExecClusterOn(Relation rel, const char *indexName);
static void ATExecDropCluster(Relation rel);
+ static void ATExecEnableDisableTrigger(Relation rel, char *trigname,
+ bool
enable);
static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
char *tablespacename);
static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace);
***
*** 1993,1998
--- 1995,2005
}
pass = AT_PASS_DROP;
break;
+ case AT_EnableTrig: /* ENABLE TRIGGER */
+ case AT_DisableTrig:/* DISABLE TRIGGER */
+ ATSimplePermissions(rel, false);
+ pass = AT_PASS_MISC;
+ break;
case AT_SetTableSpace: /* SET TABLESPACE */
/* This command never recurses */
ATPrepSetTableSpace(tab, rel, cmd-name);
***
*** 2155,2160
--- 2162,2173
* Nothing to do here; Phase 3 does the work
*/
break;
+ case AT_EnableTrig: /* ENABLE TRIGGER */
+ ATExecEnableDisableTrigger(rel, cmd-name, true);
+ break;
+ case AT_DisableTrig:/* DISABLE TRIGGER */
+ ATExecEnableDisableTrigger(rel, cmd-name, false);
+ break;
default:/* oops */
elog(ERROR, unrecognized alter table type: %d,
(int) cmd-subtype);
***
*** 5465,5470
--- 5478,5492
}
/*
+ * ALTER TABLE ENABLE/DISABLE TRIGGER
+ */
+ static void
+ ATExecEnableDisableTrigger(Relation rel, char *trigname, bool enable)
+ {
+ EnableDisableTrigger(rel, trigname, enable);
+ }
+
+ /*
* ALTER TABLE SET TABLESPACE
*/
static void
diff -cr pgsql.orig/src/backend/commands/trigger.c
pgsql/src/backend/commands/trigger.c
*** pgsql.orig/src/backend/commands/trigger.c 2005-05-30 16:20:58.0
+0900
--- pgsql/src/backend/commands/trigger.c2005-07-01 17:21:44.0
+0900
***
*** 3063,3065
--- 3063,3132
afterTriggerAddEvent(new_event);
}
}
+
+ /* --
+ * EnableDisableTrigger()
+ *
+ *Called by ALTER TABLE ENABLE/DISABLE TRIGGER
+ * to change 'tgenabled' flag in the pg_trigger.
+ * --
+ */
+ void
+ EnableDisableTrigger(Relation rel, const char *tgname, bool enable)
+ {
+ Relation tgrel;
+ SysScanDesc tgscan;
+ ScanKeyData keys[2];
+ HeapTuple tuple;
+
+ /* Permissions checks */
+ if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
+ aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
+ RelationGetRelationName(rel));
+
+ if (!allowSystemTableMods IsSystemRelation(rel))
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+errmsg(permission denied: \%s\ is a system
catalog,
+ RelationGetRelationName(rel;
+
+ tgrel = heap_open(TriggerRelationId, RowExclusiveLock);
+
+ ScanKeyInit(keys[0],
+ Anum_pg_trigger_tgrelid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(RelationGetRelid(rel)));
+ ScanKeyInit(keys[1],
+ Anum_pg_trigger_tgname,
+ BTEqualStrategyNumber, F_NAMEEQ,
+ CStringGetDatum(tgname));
+
+ tgscan = systable_beginscan(tgrel, TriggerRelidNameIndexId, true,
+ SnapshotNow, 2,
keys);
+
+ while (HeapTupleIsValid(tuple =
Re: [PATCHES] enable/disable trigger (Re: Fwd: [HACKERS] Open items)
On Fri, 1 Jul 2005, Satoshi Nagayasu wrote:
Hi all,
Here is a first patch to allow these commands.
ALTER TABLE table ENABLE TRIGGER trigname
ALTER TABLE table DISABLE TRIGGER trigname
There are three other areas which are worth looking at:
a) We may defer the execution of some triggers to the end of the
transaction. Do we execute those if a they were later disabled?
b) There is a bug in how we execute triggers. For example, in
ExecDelete():
booldodelete;
dodelete = ExecBRDeleteTriggers(estate, resultRelInfo, tupleid,
estate-es_snapshot-curcid);
if (!dodelete) /* do nothing */
return;
This means that if a before trigger returned NULL, we short circuit and do
not delete the tuple. Consider the following in ExecBRDeleteTriggers()
HeapTuple newtuple = NULL;
...
for (i = 0; i ntrigs; i++)
{
Trigger*trigger = trigdesc-triggers[tgindx[i]];
if (!trigger-tgenabled)
continue;
LocTriggerData.tg_trigtuple = trigtuple;
LocTriggerData.tg_trigtuplebuf = InvalidBuffer;
LocTriggerData.tg_trigger = trigger;
newtuple = ExecCallTriggerFunc(LocTriggerData,
tgindx[i],
relinfo-ri_TrigFunctions,
relinfo-ri_TrigInstrument,
GetPerTupleMemoryContext(estate));
if (newtuple == NULL)
break;
if (newtuple != trigtuple)
heap_freetuple(newtuple);
}
...
return (newtuple == NULL) ? false : true;
This means that if all triggers on a table are disabled, we tell the
caller that a trigger returned NULL and that we should short circuit. This
does not seem to be the case for the other DML statements.
c) There has been a push over previous releases to make dumps generated by
pg_dump look like ANSI SQL. Now, ALTER TABLE ... DISABLE trigger is useful
for pg_dump but not compliant. Others have suggested something like:
SET enable_triggers = off
This would turn all triggers off in the current session. It has the added
benefit that it does not affect other sessions. It does introduce some
issues with permissions -- I wouldn't want users turning off data
validation before triggers in my database, but that's me. I'm not
enamoured of the idea but it is worth discussing, I believe.
Also, a final patch will also need documentation and regression tests :-)
Thanks,
Gavin
---(end of broadcast)---
TIP 8: explain analyze is your friend
Re: [PATCHES] enable/disable trigger (Re: Fwd: [HACKERS] Open items)
On Fri, 1 Jul 2005, Satoshi Nagayasu wrote: Hi all, Here is a first patch to allow these commands. ALTER TABLE table ENABLE TRIGGER trigname ALTER TABLE table DISABLE TRIGGER trigname Hmmm.. just thinking about it for a second. I wonder if we should also support: ALTER TABLE DISABLE TRIGGERS which would disable all triggers on the table. We would have a complimentary ENABLE TRIGGERS as well, obviously. The reason I say this is that the common case will be that people are doing a bulk load and want to disable all triggers. However, this will be very useful for debugging interactions between triggers on a table so a user might want to disable only one of many triggers -- as your current grammar does. Perhaps a way of making the grammar a little less ambiguous would be to have the following to disable all triggers: ALTER TABLE table DISABLE TRIGGERS and the following to disable one: ALTER TRIGGER trigger DISABLE Just an idea. Thanks, Gavin ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] enable/disable trigger (Re: Fwd: [HACKERS] Open items)
Hi, Gavin Sherry wrote: Hmmm.. just thinking about it for a second. I wonder if we should also support: ALTER TABLE DISABLE TRIGGERS I found some RDBMSes are supporting 'DISABLE TRIGGER ALL TRIGGERS' command. Does anyone know about the SQL99 spec? -- NAGAYASU Satoshi [EMAIL PROTECTED] ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] enable/disable trigger (Re: Fwd: [HACKERS] Open items)
On Fri, 1 Jul 2005, Satoshi Nagayasu wrote: Hi, Gavin Sherry wrote: Hmmm.. just thinking about it for a second. I wonder if we should also support: ALTER TABLE DISABLE TRIGGERS I found some RDBMSes are supporting 'DISABLE TRIGGER ALL TRIGGERS' command. Does anyone know about the SQL99 spec? The spec says nothing about disabling triggers. Gavin ---(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] enable/disable trigger (Re: Fwd: [HACKERS] Open items)
Christopher Kings-Lynne [EMAIL PROTECTED] writes: The table owner can drop and create triggers - so why shouldn't they be able to enable and disable them? Not triggers belonging to RI constraints on other tables. regards, tom lane ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] pl/pgsql: END verbosity [patch]
Neil Conway [EMAIL PROTECTED] writes: BTW, I notice that some but not all the call sites of ereport(ERROR) in PL/PgSQL's gram.y set plpgsql_error_lineno. Is there a reason for this? Without looking at the code, I think it may be that you only need to set the variable if you want the error to point someplace different than the last lno nonterminal. regards, tom lane ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] enable/disable trigger (Re: Fwd: [HACKERS] Open items)
Satoshi Nagayasu wrote: Hi all, Here is a first patch to allow these commands. ALTER TABLE table ENABLE TRIGGER trigname ALTER TABLE table DISABLE TRIGGER trigname Bruce said to allow them only super-user, but currently this patch allows also the table owner. It would be convenient if all triggers could be disabled with a single command. More precise: option 1: All triggers except for RI triggers (EN/DISABLE TRIGGER ALL) option 2: really all triggers including RI triggers (superuser only) Regards, Andreas ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] Implementing SELECT FOR UPDATE [NOWAIT]
This is new version of SELECT FOR UPDATE NOWAIT patch. Karel On Sat, 2005-06-25 at 08:40 +0200, Hans-Juergen Schoenig wrote: yes, i think we can do it in time. regards, hans Bruce Momjian wrote: Are you working on a updated version of this? --- Bruce Momjian wrote: Uh, seems the code has drifted too much and now I can't apply this. Would you redo this against current CVS? Thanks. --- Hans-Juergen Schoenig wrote: Folks, We have implemented SELECT FOR UPDATE NOWAIT for PostgreSQL. -- Karel Zak [EMAIL PROTECTED] pgsql-nowait-07012005.patch.gz Description: GNU Zip compressed data ---(end of broadcast)--- TIP 8: explain analyze is your friend
[PATCHES] plperl return array support revisited
Following up a previous thought I had, yesterday I realised how to
return arays nicely without having to make the plperl programmer aware
of anything. The attached patch allows plperl to return an arrayref
where the function returns an array type. It silently calls a perl
function to stringify the array before passing it to the pg array
parser. Non-array returns are handled as before (i.e. passed through
this process) so it is backwards compatible. I will presently submit
regression tests and docs.
example:
andrew=# create or replace function blah() returns text[][] language
plperl as $$ return [['ab','c,d'],['e\\f','g']]; $$;
CREATE FUNCTION
andrew=# select blah();
blah
-
{{a\b,c,d},{e\\f,g}}
This would complete half of the TODO item:
. Pass arrays natively instead of as text between plperl and postgres
(The other half is translating pg array arguments to perl arrays - that
will have to wait for 8.1).
Some of this patch is adapted from a previously submitted patch from
Sergej Sergeev. Both he and Abhijit Menon-Sen have looked it over
briefly and tentatively said it looks ok.
cheers
andrew
Index: plperl.c
===
RCS file: /projects/cvsroot/pgsql/src/pl/plperl/plperl.c,v
retrieving revision 1.78
diff -c -r1.78 plperl.c
*** plperl.c 22 Jun 2005 16:45:51 - 1.78
--- plperl.c 1 Jul 2005 16:13:06 -
***
*** 81,86
--- 81,87
bool lanpltrusted;
bool fn_retistuple; /* true, if function returns tuple */
bool fn_retisset; /* true, if function returns set */
+ boolfn_retisarray; /* true if function returns array */
Oid result_oid; /* Oid of result type */
FmgrInfo result_in_func; /* I/O function and arg for result type */
Oid result_typioparam;
***
*** 191,198
--- 192,220
/* all one string follows (no commas please) */
SPI::bootstrap(); use vars qw(%_SHARED);
sub ::mkunsafefunc {return eval(qq[ sub { $_[0] $_[1] } ]); }
+ sub ::_plperl_to_pg_array
+ {
+ my $arg = shift; ref $arg eq 'ARRAY' || return $arg;
+ my $res = ''; my $first = 1;
+ foreach my $elem (@$arg)
+ {
+ $res .= ', ' unless $first; $first = undef;
+ if (ref $elem)
+ {
+ $res .= _plperl_to_pg_array($elem);
+ }
+ else
+ {
+ my $str = qq($elem);
+ $str =~ s/([\])/$1/g;
+ $res .= qq(\$str\);
+ }
+ }
+ return qq({$res});
+ }
};
+
static char *strict_embedding[3] = {
, -e,
/* all one string follows (no commas please) */
***
*** 225,230
--- 247,253
$PLContainer-permit_only(':default');
$PLContainer-permit(qw[:base_math !:base_io sort time]);
$PLContainer-share(qw[elog spi_exec_query return_next
+ _plperl_to_pg_array
DEBUG LOG INFO NOTICE WARNING ERROR %_SHARED ]);
;
***
*** 325,330
--- 348,381
return tup;
}
+ /*
+ * convert perl array to postgres string representation
+ */
+ static SV*
+ plperl_convert_to_pg_array(SV *src)
+ {
+ SV* rv;
+ int count;
+ dSP ;
+
+ PUSHMARK(SP) ;
+ XPUSHs(src);
+ PUTBACK ;
+
+ count = call_pv(_plperl_to_pg_array, G_SCALAR);
+
+ SPAGAIN ;
+
+ if (count != 1)
+ croak(Big trouble\n) ;
+
+ rv = POPs;
+
+ PUTBACK ;
+
+ return rv;
+ }
+
/* Set up the arguments for a trigger call. */
***
*** 863,869
rsi = (ReturnSetInfo *)fcinfo-resultinfo;
! if (prodesc-fn_retisset) {
if (!rsi || !IsA(rsi, ReturnSetInfo) ||
(rsi-allowedModes SFRM_Materialize) == 0 ||
rsi-expectedDesc == NULL)
--- 914,921
rsi = (ReturnSetInfo *)fcinfo-resultinfo;
! if (prodesc-fn_retisset)
! {
if (!rsi || !IsA(rsi, ReturnSetInfo) ||
(rsi-allowedModes SFRM_Materialize) == 0 ||
rsi-expectedDesc == NULL)
***
*** 884,890
int i = 0;
SV **svp = 0;
AV *rav = (AV *)SvRV(perlret);
! while ((svp = av_fetch(rav, i, FALSE)) != NULL) {
plperl_return_next(*svp);
i++;
}
--- 936,943
int i = 0;
SV **svp = 0;
AV *rav = (AV *)SvRV(perlret);
! while ((svp = av_fetch(rav, i, FALSE)) != NULL)
! {
plperl_return_next(*svp);
i++;
}
***
*** 898,904
}
rsi-returnMode = SFRM_Materialize;
! if (prodesc-tuple_store) {
rsi-setResult = prodesc-tuple_store;
rsi-setDesc = prodesc-tuple_desc;
}
--- 951,958
}
rsi-returnMode = SFRM_Materialize;
! if (prodesc-tuple_store)
! {
rsi-setResult = prodesc-tuple_store;
rsi-setDesc = prodesc-tuple_desc;
}
***
*** 943,950
}
else
{
! /* Return a perl string converted to a Datum */
! char *val = SvPV(perlret, PL_na);
retval = FunctionCall3(prodesc-result_in_func,
[PATCHES] psql tab-completion for COMMIT/ROLLBACK PREPARED
On Fri, 1 Jul 2005, Bruce Momjian wrote:
Heikki Linnakangas wrote:
On Fri, 1 Jul 2005, Oliver Jowett wrote:
PS: noticed in passing: psql's help doesn't seem to know about the 2PC
command syntax yet.
True.
Should we add support for it? 2PC is not something you normally do
interactively...
Yes, we should add psql support for it.
Ok, here's a patch.
There's no tab-completion for PREPARE TRANSACTION 'xxx' since that would
be ambigous with PREPARE xxx AS xx.
- HeikkiIndex: src/bin/psql/tab-complete.c
===
RCS file: /projects/cvsroot/pgsql/src/bin/psql/tab-complete.c,v
retrieving revision 1.133
diff -c -r1.133 tab-complete.c
*** src/bin/psql/tab-complete.c 22 Jun 2005 21:14:30 - 1.133
--- src/bin/psql/tab-complete.c 1 Jul 2005 18:34:20 -
***
*** 926,941
pg_strcasecmp(prev_wd, USER) == 0)
COMPLETE_WITH_QUERY(Query_for_list_of_users);
! /* BEGIN, END, COMMIT, ABORT */
else if (pg_strcasecmp(prev_wd, BEGIN) == 0 ||
pg_strcasecmp(prev_wd, END) == 0 ||
-pg_strcasecmp(prev_wd, COMMIT) == 0 ||
pg_strcasecmp(prev_wd, ABORT) == 0)
{
static const char *const list_TRANS[] =
{WORK, TRANSACTION, NULL};
COMPLETE_WITH_LIST(list_TRANS);
}
/* RELEASE SAVEPOINT */
else if (pg_strcasecmp(prev_wd, RELEASE) == 0)
--- 926,948
pg_strcasecmp(prev_wd, USER) == 0)
COMPLETE_WITH_QUERY(Query_for_list_of_users);
! /* BEGIN, END, ABORT */
else if (pg_strcasecmp(prev_wd, BEGIN) == 0 ||
pg_strcasecmp(prev_wd, END) == 0 ||
pg_strcasecmp(prev_wd, ABORT) == 0)
{
static const char *const list_TRANS[] =
{WORK, TRANSACTION, NULL};
COMPLETE_WITH_LIST(list_TRANS);
+ }
+ /* COMMIT */
+ else if(pg_strcasecmp(prev_wd, COMMIT) == 0)
+ {
+ static const char *const list_COMMIT[] =
+ {WORK, TRANSACTION, PREPARED, NULL};
+
+ COMPLETE_WITH_LIST(list_COMMIT);
}
/* RELEASE SAVEPOINT */
else if (pg_strcasecmp(prev_wd, RELEASE) == 0)
***
*** 944,950
else if (pg_strcasecmp(prev_wd, ROLLBACK) == 0)
{
static const char *const list_TRANS[] =
! {WORK, TRANSACTION, TO SAVEPOINT, NULL};
COMPLETE_WITH_LIST(list_TRANS);
}
--- 951,957
else if (pg_strcasecmp(prev_wd, ROLLBACK) == 0)
{
static const char *const list_TRANS[] =
! {WORK, TRANSACTION, TO SAVEPOINT, PREPARED, NULL};
COMPLETE_WITH_LIST(list_TRANS);
}
---(end of broadcast)---
TIP 3: 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] psql tab-completion for COMMIT/ROLLBACK PREPARED
Oh, just realized after Tom's reply that you were talking about help, not tab-completion... On Fri, 1 Jul 2005, Heikki Linnakangas wrote: On Fri, 1 Jul 2005, Bruce Momjian wrote: Heikki Linnakangas wrote: On Fri, 1 Jul 2005, Oliver Jowett wrote: PS: noticed in passing: psql's help doesn't seem to know about the 2PC command syntax yet. True. Should we add support for it? 2PC is not something you normally do interactively... Yes, we should add psql support for it. Ok, here's a patch. There's no tab-completion for PREPARE TRANSACTION 'xxx' since that would be ambigous with PREPARE xxx AS xx. - Heikki - Heikki ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
[PATCHES] Use of E'' in pg_dump
The following attached, applied patch uses E'' for strings containing
backslashes in pg_dump. It does not modify COPY data output.
--
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: src/bin/pg_dump/dumputils.c
===
RCS file: /cvsroot/pgsql/src/bin/pg_dump/dumputils.c,v
retrieving revision 1.17
diff -c -c -r1.17 dumputils.c
*** src/bin/pg_dump/dumputils.c 30 Apr 2005 08:08:51 - 1.17
--- src/bin/pg_dump/dumputils.c 1 Jul 2005 20:57:27 -
***
*** 111,116
--- 111,137
void
appendStringLiteral(PQExpBuffer buf, const char *str, bool escapeAll)
{
+ bool has_escapes = false;
+ const char *str2 = str;
+
+ while (*str2)
+ {
+ charch = *str2++;
+
+ if (ch == '\\' ||
+ ((unsigned char) ch (unsigned char) ' '
+(escapeAll ||
+ (ch != '\t' ch != '\n' ch != '\v'
+ ch != '\f' ch != '\r'
+ {
+ has_escapes = true;
+ break;
+ }
+ }
+
+ if (has_escapes)
+ appendPQExpBufferChar(buf, 'E');
+
appendPQExpBufferChar(buf, '\'');
while (*str)
{
***
*** 122,130
appendPQExpBufferChar(buf, ch);
}
else if ((unsigned char) ch (unsigned char) ' '
!(escapeAll
! || (ch != '\t' ch != '\n' ch != '\v'
ch != '\f' ch != '\r')
! ))
{
/*
* generate octal escape for control chars other than
--- 143,151
appendPQExpBufferChar(buf, ch);
}
else if ((unsigned char) ch (unsigned char) ' '
!(escapeAll ||
! (ch != '\t' ch != '\n' ch != '\v'
! ch != '\f' ch != '\r')))
{
/*
* generate octal escape for control chars other than
Index: src/bin/pg_dump/pg_backup_db.c
===
RCS file: /cvsroot/pgsql/src/bin/pg_dump/pg_backup_db.c,v
retrieving revision 1.62
diff -c -c -r1.62 pg_backup_db.c
*** src/bin/pg_dump/pg_backup_db.c 21 Jun 2005 20:45:44 - 1.62
--- src/bin/pg_dump/pg_backup_db.c 1 Jul 2005 20:57:28 -
***
*** 597,603
}
else
{
-
if (qry[pos] == '\\')
{
if
(AH-sqlparse.lastChar == '\\')
--- 597,602
Index: src/bin/pg_dump/pg_dump.c
===
RCS file: /cvsroot/pgsql/src/bin/pg_dump/pg_dump.c,v
retrieving revision 1.411
diff -c -c -r1.411 pg_dump.c
*** src/bin/pg_dump/pg_dump.c 30 Jun 2005 03:02:56 - 1.411
--- src/bin/pg_dump/pg_dump.c 1 Jul 2005 20:57:35 -
***
*** 7767,7774
p = tginfo-tgargs;
for (findx = 0; findx tginfo-tgnargs; findx++)
{
! const char *s = p;
for (;;)
{
p = strchr(p, '\\');
--- 7767,7775
p = tginfo-tgargs;
for (findx = 0; findx tginfo-tgnargs; findx++)
{
! const char *s = p, *s2 = p;
+ /* Set 'p' to end of arg string. marked by '\000' */
for (;;)
{
p = strchr(p, '\\');
***
*** 7781,7800
exit_nicely();
}
p++;
! if (*p == '\\')
{
p++;
continue;
}
! if (p[0] == '0' p[1] == '0' p[2] == '0')
break;
}
p--;
appendPQExpBufferChar(query, '\'');
while (s p)
{
if (*s == '\'')
! appendPQExpBufferChar(query, '\\');
appendPQExpBufferChar(query, *s++);
}
appendPQExpBufferChar(query, '\'');
--- 7782,7810
[PATCHES] Roles - SET ROLE
* Tom Lane ([EMAIL PROTECTED]) wrote:
Stephen Frost [EMAIL PROTECTED] writes:
Tom, if you're watching, are you working on this? I can probably spend
some time today on it, if that'd be helpful.
I am not; I was hoping you'd deal with SET ROLE. Is it really much
different from SET SESSION AUTHORIZATION?
Here's what I've got done so far on SET ROLE. I wasn't able to get as
much done as I'd hoped to. This is mostly just to get something posted
before the end of today in case some might think it's more of a feature
than a bug needing to be fixed (which is what I'd consider it,
personally). I'll try and work on it some this weekend, but in the US
it's a holiday weekend and I'm pretty busy. :/
Thanks,
Stephen
? src/backend/parser/.gram.y.swp
Index: src/backend/commands/variable.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/commands/variable.c,v
retrieving revision 1.109
diff -c -r1.109 variable.c
*** src/backend/commands/variable.c 28 Jun 2005 05:08:55 - 1.109
--- src/backend/commands/variable.c 1 Jul 2005 21:34:16 -
***
*** 564,569
--- 564,680
/*
+ * SET ROLE
+ *
+ * When resetting session auth after an error, we can't expect to do catalog
+ * lookups. Hence, the stored form of the value must provide a numeric oid
+ * that can be re-used directly. We store the string in the form of
+ * NAMEDATALEN 'x's, followed by T or F to indicate superuserness, followed
+ * by the numeric oid, followed by a comma, followed by the role name.
+ * This cannot be confused with a plain role name because of the NAMEDATALEN
+ * limit on names, so we can tell whether we're being passed an initial
+ * role name or a saved/restored value.
+ */
+ extern char *role_string; /* in guc.c */
+
+ const char *
+ assign_role(const char *value, bool doit, GucSource source)
+ {
+ Oid roleid = InvalidOid;
+ boolis_superuser = false;
+ const char *actual_rolename = NULL;
+ char *result;
+
+ if (strspn(value, x) == NAMEDATALEN
+ (value[NAMEDATALEN] == 'T' || value[NAMEDATALEN] == 'F'))
+ {
+ /* might be a saved userid string */
+ Oid savedoid;
+ char *endptr;
+
+ savedoid = (Oid) strtoul(value + NAMEDATALEN + 1, endptr, 10);
+
+ if (endptr != value + NAMEDATALEN + 1 *endptr == ',')
+ {
+ /* syntactically valid, so break out the data */
+ roleid = savedoid;
+ is_superuser = (value[NAMEDATALEN] == 'T');
+ actual_rolename = endptr + 1;
+ }
+ }
+
+ if (roleid == InvalidOid)
+ {
+ /* not a saved ID, so look it up */
+ HeapTuple roleTup;
+
+ if (!IsTransactionState())
+ {
+ /*
+* Can't do catalog lookups, so fail. The upshot of
this is
+* that session_authorization cannot be set in
+* postgresql.conf, which seems like a good thing
anyway.
+*/
+ return NULL;
+ }
+
+ roleTup = SearchSysCache(AUTHNAME,
+
PointerGetDatum(value),
+0, 0, 0);
+ if (!HeapTupleIsValid(roleTup))
+ {
+ if (source = PGC_S_INTERACTIVE)
+ ereport(ERROR,
+
(errcode(ERRCODE_UNDEFINED_OBJECT),
+errmsg(role \%s\ does not
exist, value)));
+ return NULL;
+ }
+
+ roleid = HeapTupleGetOid(roleTup);
+ is_superuser = ((Form_pg_authid) GETSTRUCT(roleTup))-rolsuper;
+ actual_rolename = value;
+
+ ReleaseSysCache(roleTup);
+ }
+
+ if (doit)
+ SetRole(roleid);
+
+ result = (char *) malloc(NAMEDATALEN + 32 + strlen(actual_rolename));
+ if (!result)
+ return NULL;
+
+ memset(result, 'x', NAMEDATALEN);
+
+ sprintf(result + NAMEDATALEN, %c%u,%s,
+ is_superuser ? 'T' : 'F',
+ roleid,
+ actual_rolename);
+
+ return result;
+ }
+
+ const char *
+ show_role(void)
+ {
+ /*
+* Extract the role name from the stored string; see
+* assign_role
+*/
+ const char *value = role_string;
+ Oid savedoid;
+ char *endptr;
+
+ Assert(strspn(value, x) == NAMEDATALEN
+
