Re: [PATCHES] PITR Phase 1 - Full set of patches
On Thu, Apr 29, 2004 at 11:21:53PM +0100, Simon Riggs wrote: ...my patch building experience is less than some might expect so there are various possible annoyances here. I am on hand to help and to learn by my mistakes. I see you attached one patch for every file ... this works, but is error prone. May I suggest another approach: 1. get CVSup if you can. I have Linux, so I got the ezm3 package and the CVSup package. Compile/install ezm3 (it's a Modula-3 compiler) first and then CVSup. Google for them, I don't have URLs. 2. get the whole Postgres repository using CVSup with this config file: *default host=cvsup.postgresql.org *default compress *default release=cvs *default delete use-rel-suffix # *default tag=. # base directory where CVSup will store its 'bookmarks' file(s) *default base=/home/alvherre/cvs # prefix directory where CVSup will store the actual distribution(s) *default prefix=/home/alvherre/cvs # complete distribution repository This will create a cvsroot. Adjust the path, obviously. Note that case matters because I have a cvs directory with the cvsroot, and a CVS directory for the checkouts. This may be a stupid idea but it is how I did it some time ago :-) 3. Checkout a couple of trees cvs -d /home/alvherre/cvs co pgsql I have several pgsql trees: /home/alvherre/CVS/pgsql/source/00orig this is the pristine CVS tip. My first modification in /home/alvherre/CVS/pgsql/source/01xact second in /home/alvherre/CVS/pgsql/source/02blahblah etc, you get the idea. (I have a 74_rel and 73_rel too, just in case.) Then I created a build tree, in /home/alvherre/CVS/pgsql/build. So each time I want to compile, I create a /home/alvherre/CVS/pgsql/build/00orig, cd to it, and then do ../../source/00orig/configure --enable-debug [etc] --prefix=/home/alvherre/CVS/pgsql/install/00orig 4. To generate a patch, just do cd /home/alvherre/CVS/pgsql/01xact cvs -q diff Note that sometimes I do things incrementally, so I do incremental diffs by commands like cd /home/alvherre/CVS/pgsql/source diff -cr --exclude-from=ignore-diff 01xact 02blahblah The ignore-diff file is there because some files are built on the source tree and generate a lot of irrelevant differences. It contains pl_gram.c pl.tab.h pl_scan.c psqlscan.c sql_help.h preproc.c preproc.h pgc.c guc-file.c fmgrtab.c fmgroids.h parse.h gram.c scan.c bootparse.c bootstrap_tokens.h bootscanner.c *~ tags CVS .*sw[poq] So, by this time you are wondering why do I use a number as the start of the tree name. Well, this is because I have a script which allows me to - build (configure, make) - install (make install) - initdb - run a server - run a psql client - run a make check against a running server And I may want to do any combination of the above with any combination of the pgsql trees I have, simultaneously. So I picked a random number as base, and the scripts takes the sum of this number and the number at the start of tree name, and uses it as the port (--with-pgport). So I can run all servers and clients, with no conflict. If there is any interest I can post the script too. It's somewhat ugly but it has worked for me. -- Alvaro Herrera (alvherre[a]dcc.uchile.cl) La virtud es el justo medio entre dos defectos (Aristóteles) ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] PITR Phase 1 - Full set of patches
On Thu, Apr 29, 2004 at 09:58:49PM -0400, Alvaro Herrera wrote: Note that sometimes I do things incrementally, so I do incremental diffs by commands like cd /home/alvherre/CVS/pgsql/source diff -cr --exclude-from=ignore-diff 01xact 02blahblah ^^^ Note that this should be -Ncr -- I don't create a lot of new files ... -- Alvaro Herrera (alvherre[a]dcc.uchile.cl) Cuando no hay humildad las personas se degradan (A. Christie) ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] FW: Timezone library
I have added this to CVS under src/timezone and have integrated this into configure. However, I am getting a compile error, probably because my OS has a timezone function defined in time.h: gcc -O2 -fno-strict-aliasing -Wall -Wmissing-prototypes -Wmissing-declarations -O1 -Wall -Wmissing-prototypes -Wmissing-declarations -Wpointer-arith -Wcast-align -I../../src/include -I/usr/local/include/readline -I/usr/contrib/include -c -o localtime.o localtime.c In file included from private.h:91, from localtime.c:25: /usr/include/time.h:104: `pg_timezone' redeclared as different kind of symbol localtime.c:5: previous declaration of `pg_timezone' localtime.c:203: `pg_timezone' redeclared as different kind of symbol /usr/include/time.h:104: previous declaration of `pg_timezone' localtime.c:933: warning: static declaration for `tzsetwall' follows non-static gmake: *** [localtime.o] Error 1 Fortunately only Win32 compiles this code right now. --- Magnus Hagander wrote: This mail apparantly didn't make it through because it was too large. Resending it without the largest file, tzlib.tgz. I've put this file (+ the patches) on http://www.hagander.net/pgsql/. //Magnus -Original Message- From: Magnus Hagander Sent: Sunday, April 18, 2004 9:05 PM To: [EMAIL PROTECTED] Subject: Timezone library Hi! Attached is a slightly modified version of Arthur David Olson's timezone library for inclusion in postgresql, pending review of course. This is required to give win32 working timezone functions on dates before 1970. It has also been indicated that it might be interesting to have such a library on other platforms as well. Note that with this enabled, the timezone data files are installed in pgdir/share/timezone. Attached are the following files: * tzlib.tgz - the timezone implementation, including the datafiles required to build the timezone data files. * timezone.patch - the modifications made to existing files to make it work. This includes setting the FRONTEND flat for ecpg. * tzcode.diff - lists the modifications that I have made to the files from the original timezone files, for reference. Bruce has offered to do the autoconf and Makefile integration parts - I don't know autoconf enough to do that part... The #defines necessary are activeted by setting USE_PGTZ. Of course, the appropriate files also have to be linked in. With this patch applied, and the timezone data installed, win32 now passes all time-related regression tests (3 now fails, 2 known and one being locale-inflicted, still investigating that one) //Magnus tzcode_changes.diff timezone.patch Content-Description: tzcode_changes.diff [ Attachment, skipping... ] Content-Description: timezone.patch [ Attachment, skipping... ] ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (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 8: explain analyze is your friend
Re: [PATCHES] width_bucket() per SQL2003 (WIP)
Neil Conway [EMAIL PROTECTED] writes: The attached patch implements the width_bucket() function (as specified by Section 6.27 of the SQL2003 standard, in particular page 250 in my copy). Other than it's in SQL2003, what's the point of implementing this? It seems like a mighty marginal feature ... I believe the function comes from Oracle 9i, Trying to implement everything in Oracle is a goal up with which we must not try to keep. regards, tom lane ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] width_bucket() per SQL2003 (WIP)
Other than it's in SQL2003, what's the point of implementing this? It seems like a mighty marginal feature ... I don't see a problem with implementing every function that SQL2003 has...isn't that the point of implementing the standard? Chris ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] SECURITY DEFINER not being propagated...
Without any rationale as to why CREATE TEMP TABLEs checks the session
user in the archives, could we open this up for discussion again?
Well, let me put it this way: if you want to change the behavior you're
going to have to work much harder than just reverting the prior patch.
IIRC the fundamental reason the code works that way is that
InitTempTableNamespace is done only once per session. If it checks
against current_user rather than session_user then (a) the results will
be inconsistent, and (b) you create a different sort of security hole,
which is that if a setuid function is the first to try to create a temp
table in a particular session, then not-so-privileged functions will
still be able to create temp tables later in the session.
Hrm, I didn't realize that: points taken. Since temp schemas are
always owned by the superuser, why aren't the ACL checks done when the
temp relation is created as opposed to when the schema is created? I
see what you're saying about things currently needing to use
GetSessionuserId() instead of GetUserId(), but if a check for istemp is
pushed down into DefineRelation(), then (from what I can tell)
GetUserId() can be used in InitTempTableNamespace(). Object owners can
only delete their objects, the temp schema can't be deleted as is
because its owner is the superuser.
I think the attached patch addresses both of your concerns. Things are
consistent in that SECURITY DEFINER/TEMP TABLEs will now work as
expected. The security hole isn't an issue because security checks are
applied both in InitTempTableNamespace() and DefineRelation().
At the moment, this behavior cripples the usefulness
of having a TEMP table be used as a trusted cache for data.
What exactly do you think makes a temp table suitable as a trusted
cache? Or more suitable than non-temp tables?
Revoke all privs on temp tables except from the DBA, then setup
functions to use the temp table as a way of maintaining state
information across transactions (within the same session). It's a hack
to get around the lack of server side variables. In many ways,
actually, it works out better because I can wrap functions or
PostgreSQL's permissions around the temp relations and get exactly the
access that I need... far more fine grained than anything I could do
with a GUC or some other server side MIB/variable implementation.
I don't really believe in the notion of restricting temp table creation
to setuid functions. AFAICS the only reason for forbidding temp table
creation is to prevent a session from using any on-disk resources, and
that hardly works if it can still do so via calling setuid functions.
It can't populate or read data out of the temp relation though, which
is ideal for my situation.
-scIndex: catalog/namespace.c
===
RCS file: /projects/cvsroot/pgsql-server/src/backend/catalog/namespace.c,v
retrieving revision 1.63
diff -u -r1.63 namespace.c
--- catalog/namespace.c 13 Feb 2004 01:08:20 - 1.63
+++ catalog/namespace.c 29 Apr 2004 07:33:49 -
@@ -1639,12 +1639,8 @@
* First, do permission check to see if we are authorized to make temp
* tables. We use a nonstandard error message here since
* databasename: permission denied might be a tad cryptic.
-*
-* Note we apply the check to the session user, not the currently active
-* userid, since we are not going to change our minds about temp table
-* availability during the session.
*/
- if (pg_database_aclcheck(MyDatabaseId, GetSessionUserId(),
+ if (pg_database_aclcheck(MyDatabaseId, GetUserId(),
ACL_CREATE_TEMP) !=
ACLCHECK_OK)
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
Index: commands/tablecmds.c
===
RCS file: /projects/cvsroot/pgsql-server/src/backend/commands/tablecmds.c,v
retrieving revision 1.102
diff -u -r1.102 tablecmds.c
--- commands/tablecmds.c1 Apr 2004 21:28:44 - 1.102
+++ commands/tablecmds.c29 Apr 2004 07:33:49 -
@@ -165,8 +165,12 @@
{
AclResult aclresult;
- aclresult = pg_namespace_aclcheck(namespaceId, GetUserId(),
-
ACL_CREATE);
+ if (stmt-relation-istemp)
+ aclresult = pg_namespace_aclcheck(MyDatabaseId, GetUserId(),
+ ACL_CREATE_TEMP);
+ else
+ aclresult = pg_namespace_aclcheck(namespaceId, GetUserId(),
+ ACL_CREATE);
if (aclresult != ACLCHECK_OK)
aclcheck_error(aclresult, ACL_KIND_NAMESPACE,
Re: [PATCHES] Basic subtransaction facility
On Wed, 28 Apr 2004 12:02:44 -0400, Alvaro Herrera [EMAIL PROTECTED] wrote: In fact, I think we should mark ERROR as aborting the whole transaction tree, and create a new level which would abort the innermost subtransaction. We would then change whatever is appropiate to the new elevel. Doing otherwise would leave us open to unexpected conditions causing only subtrans abort, which could lead to unreliable behavior. Why? Subtransaction commit propagates an error state to the parent transaction. And if a subtransaction is rolled back the parent can continue cleanly no matter what was the reason for the subtrans abort. Servus Manfred ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [PATCHES] Basic subtransaction facility
Manfred Koizar wrote: On Wed, 28 Apr 2004 12:02:44 -0400, Alvaro Herrera [EMAIL PROTECTED] wrote: In fact, I think we should mark ERROR as aborting the whole transaction tree, and create a new level which would abort the innermost subtransaction. We would then change whatever is appropiate to the new elevel. Doing otherwise would leave us open to unexpected conditions causing only subtrans abort, which could lead to unreliable behavior. Why? Subtransaction commit propagates an error state to the parent transaction. And if a subtransaction is rolled back the parent can continue cleanly no matter what was the reason for the subtrans abort. I think his point was that there are some errors that should abort the outer transaction too. I think Alvaro mentioned out of memory, but that is a FATAL error. Alvaro, what error were you thinking of that should abort the outer transaction? -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (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
Re: [PATCHES] Basic subtransaction facility
Bruce Momjian [EMAIL PROTECTED] writes: Manfred Koizar wrote: Why? Subtransaction commit propagates an error state to the parent transaction. And if a subtransaction is rolled back the parent can continue cleanly no matter what was the reason for the subtrans abort. I think his point was that there are some errors that should abort the outer transaction too. I think Alvaro mentioned out of memory, but that is a FATAL error. Nonsense. In the first place, out-of-memory hasn't been FATAL for years. In the second, there is no reason to think that we can't continue the outer transaction(s), as aborting the innermost one is likely to free quite a lot of memory. (And if it doesn't, well, the outer one will get its own out-of-memory ERROR soon enough.) In general I tend to agree with Manfred's point: if you have reason to suspect global corruption of a backend's state then you should do FATAL (or possibly PANIC). If you do not suspect this then you ought to just ERROR. I do not see the use-case for abort-all-levels-of-xact-but- don't-exit. regards, tom lane ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] Basic subtransaction facility
On Thu, Apr 29, 2004 at 12:26:01AM -0400, Bruce Momjian wrote: I don't understand your elog(ERROR) vs. ereport(ERROR) distinction. Was that a typo? Nope. When Tom upgraded the error handling, he changed almost everything to ereport(), but in the places where there's a violation of expected conditions, he retained elog(). We don't provide special error code, nor there is space for errhints etc. Those unexpected conditions I thought we could just abort the transaction tree; but maybe we have to close the backend as Manfred and Tom say. I don't think there's space for PANIC though (unless we suspect shared state corruption ... is that checked for anywhere? I haven't looked.) -- Alvaro Herrera (alvherre[a]dcc.uchile.cl) No single strategy is always right (Unless the boss says so) (Larry Wall) ---(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] Basic subtransaction facility
On Thu, Apr 29, 2004 at 02:42:23PM -0400, Tom Lane wrote: In general I tend to agree with Manfred's point: if you have reason to suspect global corruption of a backend's state then you should do FATAL (or possibly PANIC). If you do not suspect this then you ought to just ERROR. I do not see the use-case for abort-all-levels-of-xact-but- don't-exit. Ok, I'm not wedded to the idea of a new elevel. So you think elog(ERROR) should rather be elog(FATAL) ? -- Alvaro Herrera (alvherre[a]dcc.uchile.cl) Y una voz del caos me habló y me dijo Sonríe y sé feliz, podría ser peor. Y sonreí. Y fui feliz. Y fue peor. ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] subtransactions -- storage manager
On Sun, 2004-04-25 at 19:06, Alvaro Herrera wrote: Hackers, This patch adds subtransaction support into the storage manager. Files created or dropped inside a subtransaction are correctly dealt with at subtransaction commit or abort. - pg_clog/pg_subtrans. Need a solution. PS: somehow I managed to get tired of the phrase nested transactions and I'm using the term subtransactions instead. In my head they are the same thing ... Impressive. As you're aware, our current work overlaps. pg_clog doesn't seem like the place to record subtransactions, though maybe it is... could we not give subtransactions a txnid just as with flat transactions? That way we can record everything in pg_clog AND recovery will work without further modification - as long as the failure of a top level transaction causes failure of every subtransaction EVEN if the subtrans originally committed. If you add pg_subtrans, you will need to make recovery work all over again...really, you don't want to be doing that, do you? I also have other questions Forgive my lack of attention: I want SAVEPOINTs, not subtransactions... how do we do those? My last focus on this was to do with SQL handling of transactional rollback characteristics on error. PostgreSQL performs rollback on complete txn when error occurs, rather than allowing statement level abort and then retry...this was characterised as requiring nested transactions...are your aware of this...is it on your roadmap. Best Regards, Simon Riggs ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] subtransactions -- storage manager
Simon Riggs wrote: On Sun, 2004-04-25 at 19:06, Alvaro Herrera wrote: Hackers, This patch adds subtransaction support into the storage manager. Files created or dropped inside a subtransaction are correctly dealt with at subtransaction commit or abort. - pg_clog/pg_subtrans. Need a solution. PS: somehow I managed to get tired of the phrase nested transactions and I'm using the term subtransactions instead. In my head they are the same thing ... Impressive. As you're aware, our current work overlaps. pg_clog doesn't seem like the place to record subtransactions, though maybe it is... could we not give subtransactions a txnid just as with flat transactions? That way we can record everything in pg_clog AND recovery will work without further modification - as long as the failure of a top level transaction causes failure of every subtransaction EVEN if the subtrans originally committed. The problem is we have to atomically mark multiple transactions as committed/aborted in pg_clog. Each subtransaction does get its own xid, it is just that pg_subtrans maps each xid to is parent xid for use in atomically marking the xids as committed/aborted. Recovery via xlog should be fine. If you add pg_subtrans, you will need to make recovery work all over again...really, you don't want to be doing that, do you? I also have other questions Forgive my lack of attention: I want SAVEPOINTs, not subtransactions... how do we do those? Savepoints are basically just a BEGIN at the save point, and a ROLLBACK to get you back to the saved spot. It is just window-dressing on top of nested transactions. My last focus on this was to do with SQL handling of transactional rollback characteristics on error. PostgreSQL performs rollback on complete txn when error occurs, rather than allowing statement level abort and then retry...this was characterised as requiring nested transactions...are your aware of this...is it on your roadmap. Yes, that is the whole point --- to allow individual queries to fail without rolling back the entire transaction. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (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 6: Have you searched our list archives? http://archives.postgresql.org
