Re: [PATCHES] [HACKERS] patches for items from TODO list
Tom Lane wrote: Bruce Momjian pgman@candle.pha.pa.us writes: Here is an updated version of the COPY \x patch. It is the first patch attached. Also, I realized that if we support \x in COPY, we should also support \x in strings to the backend. This is the second patch. Do we really want to do any of these things? We've been getting beaten up recently about the fact that we have non-SQL-spec string escapes (ie, all the backslash stuff) so I'm a bit dubious about adding more, especially when there's little if any demand for it. I don't object too much to the COPY addition, since that's outside any spec anyway, but I do think we ought to think twice about adding this to SQL literal handling. +1 from me on this for Tom -- please don't break spec compliance! Best Regards, Michael Paesold ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] skip FK trigger on UPDATE
Neil Conway [EMAIL PROTECTED] writes: I basically just moved the logic for the are the keys equal? test from the FK trigger itself into the code that enqueues the trigger. I then removed the keys-are-equal check from the FK trigger. I also had to change (somewhat awkwardly) RI_FKey_keyequal_upd() to work for updates on either the PK table or the FK table. I also removed the bogus TriggerData argument from RI_FKey_keyequal_upd(), since AFAICS it is no needed and merely adds confusion. It would probably be cleaner to have two keys-are-equal check routines than to contort RI_FKey_keyequal_upd to work this way. You seem to have also done a fair amount of unrelated hacking around. What's the point of removing the distinction between check_ins and check_upd functions? I think that may confuse existing client code that looks at the triggers, without really buying much. A possibly stronger argument is that if we find down the road that we need separate functions again, we'll be in a bit of a sticky place; at least if we need it to fix a bug without forcing initdb. This patch does cause one change to the regression test output: That's discomforting --- you had better find out exactly why that changed. regards, tom lane ---(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] skip FK trigger on UPDATE
Tom Lane wrote: You seem to have also done a fair amount of unrelated hacking around. What's the point of removing the distinction between check_ins and check_upd functions? I talked about this in an earlier message to -hackers: check_upd was actually unused (check_ins was used for both inserts and updates). I think that may confuse existing client code that looks at the triggers, without really buying much. A possibly stronger argument is that if we find down the road that we need separate functions again, we'll be in a bit of a sticky place; at least if we need it to fix a bug without forcing initdb. Hmm, I suppose -- if you prefer I can have check_ins called by the INSERT trigger and check_upd called by the UPDATE trigger, which probably makes more sense. -Neil ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] skip FK trigger on UPDATE
Neil Conway [EMAIL PROTECTED] writes: Tom Lane wrote: What's the point of removing the distinction between check_ins and check_upd functions? I talked about this in an earlier message to -hackers: check_upd was actually unused (check_ins was used for both inserts and updates). Hm, guess I missed (or forgot) that message. Hmm, I suppose -- if you prefer I can have check_ins called by the INSERT trigger and check_upd called by the UPDATE trigger, which probably makes more sense. Yeah ... I thought it was doing that already. regards, tom lane ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] O_DIRECT for WAL writes
Neil Conway [EMAIL PROTECTED] wrote: The patch adds a new choice open_direct to wal_sync_method. Have you looked at what the performance difference of this option is? Yes, I've tested pgbench and dbt2 and their performances have improved. The two results are as follows: 1. pgbench -s 100 on one Pentium4, 1GB mem, 2 ATA disks, Linux 2.6.8 (attached image) tps | wal_sync_method ---+--- 147.0 | open_direct + write multipage (previous patch) 147.2 | open_direct (this patch) 109.9 | open_sync 2. dbt2 100WH on two opterons, 8GB mem, 12 SATA-RAID disks, Linux 2.4.20 tpm | wal_sync_method +-- 1183.9 | open_direct (this patch) 911.3 | fsync http://www.mail-archive.com/pgsql-patches@postgresql.org/msg07186.html Is this data still applicable to the revised patch? Direct-IO might be good on some machines, and bad on others. This data is another reason that I revised the patch; If you don't use open_direct, WAL writer behaves quite similarly to former. However, the performances did not go down at least on my benchmarks. I have no idea why the above data was bad... --- ITAGAKI Takahiro NTT Cyber Space Laboratories attachment: pgbench-result.png ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] skip FK trigger on UPDATE
On Sun, 2005-05-29 at 21:06 -0400, Tom Lane wrote:
Neil Conway [EMAIL PROTECTED] writes:
Hmm, I suppose -- if you prefer I can have check_ins called by the
INSERT trigger and check_upd called by the UPDATE trigger, which
probably makes more sense.
Yeah ... I thought it was doing that already.
Attached are two patches: one that changes ADD FOREIGN KEY to create
separate ON INSERT and ON UPDATE triggers that invoke different trigger
functions, and a revised version of the FK UPDATE enqueuing patch.
BTW, the regression test failure was just stupidity on my part: I had
updated the expected results using the regression test output from
some intermediate version of the patch without checking it carefully
enough. The attached patch doesn't FK enqueuing patch doesn't cause any
unexpected regression test changes.
Barring any objections I'll apply both of these to HEAD today or
tomorrow.
-Neil
Index: src/backend/commands/tablecmds.c
===
RCS file: /var/lib/cvs/pgsql/src/backend/commands/tablecmds.c,v
retrieving revision 1.157
diff -c -r1.157 tablecmds.c
*** src/backend/commands/tablecmds.c 10 May 2005 13:16:26 - 1.157
--- src/backend/commands/tablecmds.c 30 May 2005 02:33:56 -
***
*** 4380,4431
pfree(trig.tgargs);
}
- /*
- * Create the triggers that implement an FK constraint.
- */
static void
! createForeignKeyTriggers(Relation rel, FkConstraint *fkconstraint,
! Oid constrOid)
{
- RangeVar *myRel;
CreateTrigStmt *fk_trigger;
ListCell *fk_attr;
ListCell *pk_attr;
- ObjectAddress trigobj,
- constrobj;
-
- /*
- * Reconstruct a RangeVar for my relation (not passed in,
- * unfortunately).
- */
- myRel = makeRangeVar(get_namespace_name(RelationGetNamespace(rel)),
- pstrdup(RelationGetRelationName(rel)));
- /*
- * Preset objectAddress fields
- */
- constrobj.classId = ConstraintRelationId;
- constrobj.objectId = constrOid;
- constrobj.objectSubId = 0;
- trigobj.classId = TriggerRelationId;
- trigobj.objectSubId = 0;
-
- /* Make changes-so-far visible */
- CommandCounterIncrement();
-
- /*
- * Build and execute a CREATE CONSTRAINT TRIGGER statement for the
- * CHECK action.
- */
fk_trigger = makeNode(CreateTrigStmt);
fk_trigger-trigname = fkconstraint-constr_name;
fk_trigger-relation = myRel;
- fk_trigger-funcname = SystemFuncName(RI_FKey_check_ins);
fk_trigger-before = false;
fk_trigger-row = true;
! fk_trigger-actions[0] = 'i';
! fk_trigger-actions[1] = 'u';
! fk_trigger-actions[2] = '\0';
fk_trigger-isconstraint = true;
fk_trigger-deferrable = fkconstraint-deferrable;
--- 4380,4412
pfree(trig.tgargs);
}
static void
! CreateFKCheckTrigger(RangeVar *myRel, FkConstraint *fkconstraint,
! ObjectAddress *constrobj, ObjectAddress *trigobj,
! bool on_insert)
{
CreateTrigStmt *fk_trigger;
ListCell *fk_attr;
ListCell *pk_attr;
fk_trigger = makeNode(CreateTrigStmt);
fk_trigger-trigname = fkconstraint-constr_name;
fk_trigger-relation = myRel;
fk_trigger-before = false;
fk_trigger-row = true;
!
! /* Either ON INSERT or ON UPDATE */
! if (on_insert)
! {
! fk_trigger-funcname = SystemFuncName(RI_FKey_check_ins);
! fk_trigger-actions[0] = 'i';
! }
! else
! {
! fk_trigger-funcname = SystemFuncName(RI_FKey_check_upd);
! fk_trigger-actions[0] = 'u';
! }
! fk_trigger-actions[1] = '\0';
fk_trigger-isconstraint = true;
fk_trigger-deferrable = fkconstraint-deferrable;
***
*** 4453,4465
fk_trigger-args = lappend(fk_trigger-args, lfirst(pk_attr));
}
! trigobj.objectId = CreateTrigger(fk_trigger, true);
/* Register dependency from trigger to constraint */
! recordDependencyOn(trigobj, constrobj, DEPENDENCY_INTERNAL);
/* Make changes-so-far visible */
CommandCounterIncrement();
/*
* Build and execute a CREATE CONSTRAINT TRIGGER statement for the ON
--- 4434,4487
fk_trigger-args = lappend(fk_trigger-args, lfirst(pk_attr));
}
! trigobj-objectId = CreateTrigger(fk_trigger, true);
/* Register dependency from trigger to constraint */
! recordDependencyOn(trigobj, constrobj, DEPENDENCY_INTERNAL);
/* Make changes-so-far visible */
CommandCounterIncrement();
+ }
+
+ /*
+ * Create the triggers that implement an FK constraint.
+ */
+ static void
+ createForeignKeyTriggers(Relation rel, FkConstraint *fkconstraint,
+ Oid constrOid)
+ {
+ RangeVar *myRel;
+ CreateTrigStmt *fk_trigger;
+ ListCell *fk_attr;
+ ListCell *pk_attr;
+ ObjectAddress trigobj,
+ constrobj;
+
+ /*
+ * Reconstruct a RangeVar for my relation (not passed in,
+ * unfortunately).
+ */
+ myRel = makeRangeVar(get_namespace_name(RelationGetNamespace(rel)),
+ pstrdup(RelationGetRelationName(rel)));
+
+ /*
+ * Preset objectAddress fields
+ */
+ constrobj.classId =
