Re: [PATCHES] [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions?
Sean Chittenden <[EMAIL PROTECTED]> writes: >> I would NOT call it a "security" provision, as it is fairly easily >> defeated using SET TRANSACTION. > Um, why not make it an actual full blown security feature by applying > the following patch? It's not intended to be a security measure, and I would strongly resist any attempt to make it so along the lines you propose. I do not want to try to base real security on GUC settings. The GUC mechanism is not designed to be unsubvertible, it's designed to allow convenient administration of a bunch of settings. In any case, we already have mechanisms for preventing specific users from altering data: that's what GRANT/REVOKE are for. I don't think anyone would have bothered with START TRANSACTION READ ONLY if it weren't required by the SQL spec. regards, tom lane ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions?
Sean, > Um, why not make it an actual full blown security feature by applying > the following patch? This gives PostgreSQL real read only > transactions that users can't escape from. Notes about the patch: Way nifty. I vote in favor of this patch (suitably documented & debugged) for 7.5. -- Josh Berkus Aglio Database Solutions San Francisco ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] array expression NULL fix [was: [HACKERS] odd behavior/possible bug]
Joe Conway <[EMAIL PROTECTED]> writes: > Here's a patch that replaces the ERROR with a NULL return value when an > element in an array expression is NULL. Applied. I also added a comment to remind us to change contain_nonstrict_functions() when ARRAY's behavior is changed again. regards, tom lane ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions?
> >> I would NOT call it a "security" provision, as it is fairly > >> easily defeated using SET TRANSACTION. > > > Um, why not make it an actual full blown security feature by > > applying the following patch? > > It's not intended to be a security measure, and I would strongly > resist any attempt to make it so along the lines you propose. Intended or not, it does work. > I do not want to try to base real security on GUC settings. The GUC > mechanism is not designed to be unsubvertible, it's designed to > allow convenient administration of a bunch of settings. I agree that permissions of objects or anything specific should remain outside of the GUC system, however for global GUC like things, such as the default mode of a transaction (READ ONLY/READ WRITE), this is perfect (I think of PostgreSQL's GUC system the same way I do FreeBSD's sysctl MIB system or Linux's /proc file system: useful for global things, in appropriate for anything fine grained). I was thinking about that this morning, a better name would be "jail_read_only_transactions" as the GUC contains a user to a read only transaction. It could be confusing in that it doesn't force a transaction to be read only. > In any case, we already have mechanisms for preventing specific > users from altering data: that's what GRANT/REVOKE are for. I don't > think anyone would have bothered with START TRANSACTION READ ONLY if > it weren't required by the SQL spec. Ah, but this falls on its face when you want a user who does have write access to tables to go through a fixed procedure before opening up the DB for write access (logging of SELECTs will always require some goo). To prevent a user who does have write access (INSERT/UPDATE/DELETE) to tables from modifying tables before they've started a transaction inside of the database system (different than BEGIN, custom function start_txn() that sets up the database for logging), in every function, I used to have to test to see if the txn_id was in the temp table. With the posted patch, I can ensure a fully auditable and exceedingly secure database (except for malicious DBAs) that prevents any kind of unlogged abuse. Here's what I'm planning on doing in my tree: -- Username joe is any non-dba ALTER USER joe SET transaction_force_read_only TO TRUE; ALTER USER joe SET default_transaction_read_only TO TRUE; CREATE FUNCTION public.start_txn() RETURNS BIGINT EXTERNAL SECURITY DEFINER AS ' -- Pulls a txn_id from a sequence and stuff value into a temp -- table that a user doesn't have write access to. Once -- transaction ID is stored, change the transaction from READ -- ONLY to READ WRITE. Return the txn_id to the user. ' LANGUAGE 'plpgsql'; Before, I had to have every function that modified data test to see if a txn_id existed in the session temp table. Now, by relying on the transaction's mode, I only have to test for that on the tables that I log when there is SELECT activity, which will cut the number of lines of pl/PgSQL code by about 1200-1500 lines[1]! At the very least, it's an easier way of guaranteeing a READ ONLY database. Securing a database with GRANT/REVOKE can be tedious and error prone. In the case of a PHP Web shop/hacker that hasn't a clue about quoting data before sending queries to the backend, this is a nice safety blanket that takes a few seconds to setup (create user www, alter user, alter user && *poof*, www user is secure). Right now, to secure a user, you have to REVOKE INSERT, UPDATE, DELETE on all tables, schemas and functions running as SECURITY DEFINER that modify data, whereas jail_read_only_transactions is a simple and effective blanket. IMHO, this is a huge 2nd safety belt that's easy to apply, even though you're right, people _should_ rely on GRANT/REVOKE though GRANT/REVOKE doesn't work in some situations as mentioned above. -sc [1] Pl/PgSQL code + surrounding white space (* >300): PERFORM TRUE FROM [temp_tbl]...; IF NOT FOUND THEN RAISE EXCEPTION ''my error message''; END IF; -- Sean Chittenden ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions?
> > Um, why not make it an actual full blown security feature by > > applying the following patch? This gives PostgreSQL real read > > only transactions that users can't escape from. Notes about the > > patch: > > Way nifty. > > I vote in favor of this patch (suitably documented & debugged) for 7.5. Heh, there ain't much to debug: it's pretty straight forward. I ran all the use cases/syntaxes I could think of and they worked as expected. It's a pretty chump little ditty that I originally wrote for the sake of the 7.4 PR, but it's proving to be quite useful here in my tree... though I like the name "jail_read_only_transactions" more... patch updated for new name. -sc -- Sean Chittenden ---(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] [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions?
> > > Um, why not make it an actual full blown security feature by
> > > applying the following patch? This gives PostgreSQL real read
> > > only transactions that users can't escape from. Notes about the
> > > patch:
> >
> > Way nifty.
> >
> > I vote in favor of this patch (suitably documented & debugged) for 7.5.
>
> Heh, there ain't much to debug: it's pretty straight forward. I ran
> all the use cases/syntaxes I could think of and they worked as
> expected. It's a pretty chump little ditty that I originally wrote
> for the sake of the 7.4 PR, but it's proving to be quite useful here
> in my tree... though I like the name "jail_read_only_transactions"
> more... patch updated for new name.
Err.. and attached. -sc
--
Sean Chittenden
Index: src/include/access/xact.h
===
RCS file: /home/ncvs/pgsql/pgsql-server/src/include/access/xact.h,v
retrieving revision 1.52
diff -u -r1.52 xact.h
--- src/include/access/xact.h 14 May 2003 03:26:03 - 1.52
+++ src/include/access/xact.h 30 Jul 2003 21:27:04 -
@@ -33,6 +33,7 @@
/* Xact read-only state */
extern boolDefaultXactReadOnly;
extern boolXactReadOnly;
+extern boolJailReadOnlyXacts;
/*
* transaction states - transaction state from server perspective
Index: src/backend/utils/misc/guc.c
===
RCS file: /home/ncvs/pgsql/pgsql-server/src/backend/utils/misc/guc.c,v
retrieving revision 1.144
diff -u -r1.144 guc.c
--- src/backend/utils/misc/guc.c29 Jul 2003 00:03:18 - 1.144
+++ src/backend/utils/misc/guc.c30 Jul 2003 21:30:50 -
@@ -94,6 +94,7 @@
static const char *assign_log_error_verbosity(const char *newval, bool doit,
bool interactive);
static bool assign_phony_autocommit(bool newval, bool doit, bool interactive);
+static bool assign_transaction_read_only(bool newval, bool doit, bool interactive);
/*
@@ -814,6 +815,15 @@
GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
},
&XactReadOnly,
+ false, assign_transaction_read_only, NULL
+ },
+ {
+ {"jail_read_only_transactions", PGC_SUSET, CLIENT_CONN_STATEMENT,
+ gettext_noop("Jails transactions that are READ ONLY so that
users can't change the transaction from being in a READ ONLY mode"),
+ NULL,
+ GUC_NO_SHOW_ALL | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE
+ },
+ &JailReadOnlyXacts,
false, NULL, NULL
},
{
@@ -4375,6 +4385,39 @@
return false;
}
return true;
+}
+
+
+static bool
+assign_transaction_read_only(bool newval, bool doit, bool interactive)
+{
+ if (JailReadOnlyXacts == false)
+ {
+ if (doit == true)
+ XactReadOnly = newval;
+ return true;
+ } else {
+ if (superuser() == false)
+ {
+ if (newval == true)
+ {
+ if (doit)
+ XactReadOnly = true;
+
+ return true;
+ } else {
+ if (doit && interactive)
+ ereport(ERROR,
+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+errmsg("Insufficient privileges to
SET transaction_read_only TO FALSE")));
+ return false;
+ }
+ } else {
+ if (doit)
+ XactReadOnly = newval;
+ return true;
+ }
+ }
}
Index: src/backend/access/transam/xact.c
===
RCS file: /home/ncvs/pgsql/pgsql-server/src/backend/access/transam/xact.c,v
retrieving revision 1.149
diff -u -r1.149 xact.c
--- src/backend/access/transam/xact.c 21 Jul 2003 20:29:39 - 1.149
+++ src/backend/access/transam/xact.c 30 Jul 2003 21:31:17 -
@@ -211,6 +211,7 @@
bool DefaultXactReadOnly = false;
bool XactReadOnly;
+bool JailReadOnlyXacts = false;
intCommitDelay = 0;/* precommit delay in microseconds */
intCommitSiblings = 5; /* number of concurrent xacts needed to
---(end of broadcast)---
TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions?
Sean Chittenden <[EMAIL PROTECTED]> writes: >> It's not intended to be a security measure, and I would strongly >> resist any attempt to make it so along the lines you propose. > Intended or not, it does work. No, you just haven't thought of a way to get around it yet. When you do think of one, you'll be wanting us to contort the GUC system to plug the loophole. We've already got a horrid mess in there for the LOG_XXX variables, and I don't want to add more. I'm not objecting to the idea of being able to make users read-only. I'm objecting to using GUC for it. Send in a patch that, say, adds a bool column to pg_shadow, and I'll be happy. regards, tom lane ---(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] ruleutils with pretty-print option
Andreas Pflug <[EMAIL PROTECTED]> writes: > I recoded the stuff as Tom recommended, leaving the non-pretty version > function names as they used to be, inventing new pg_get__ext > functions for the extended stuff, and pushing the code down into > pg_get__worker functions when needed. We now need the additional > prototype include patch from builtins.h. Applied with some editorializing. In particular, I don't believe the original did the right thing with (a - (b - c)). regards, tom lane ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions?
> >> It's not intended to be a security measure, and I would strongly > >> resist any attempt to make it so along the lines you propose. > > > Intended or not, it does work. > > No, you just haven't thought of a way to get around it yet. When > you do think of one, you'll be wanting us to contort the GUC system > to plug the loophole. We've already got a horrid mess in there for > the LOG_XXX variables, and I don't want to add more. > > I'm not objecting to the idea of being able to make users read-only. > I'm objecting to using GUC for it. Send in a patch that, say, adds > a bool column to pg_shadow, and I'll be happy. How is that any different than ALTER USER [username] SET jail_read_only_transactions TO true? It sets something in pg_shadow.useconfig column, which is permanent. Ultimately, the XactReadOnly variable is going to be used and the assign_transaction_read_only() function in guc.c will be necessary too. Would a different GUC that required only one ALTER USER statement make you happier? If so, then how about: ALTER USER [username] SET readonly TO TRUE; ALTER USER [username] SET read_only TO TRUE; ALTER USER [username] SET readonly_user TO TRUE; ALTER USER [username] SET read_only_user TO TRUE; When "read_only_user" is set to true, it changes transaction_read_only, default_transaction_read_only, and jail_read_only_transactions all to TRUE. The read_only_user GUC does nothing if set to FALSE and can only be set by the superuser. If I were to add a specific syntax for this, what would you like? ALTER USER [username] [READONLY|NOTREADONLY]; ?? Use of the GUC infrastructure makes much more sense and is loads cleaner, IMHO. Use of GUC is also going to be much more lightweight than adding a new syntax for making accounts read only, esp since the read only transaction infrastructure is already GUC backed. Is your objection that GUC doesn't scale well? If so, it shouldn't too hard for me to change GUC to use a hash lookup instead of a linear scan (that'd be something I'd do for 7.5). If it's that GUC is a flat namespace, I'm very inclined agree that it's limited in that regard and a full MIB tree would be preferred. Ex: ALTER USER [username] SET user.readonly = TRUE; ALTER USER [username] SET user.dba = TRUE; ALTER USER [username] SET user.create_database = TRUE; ALTER USER [username] SET user.create_user = TRUE; ALTER USER [username] SET user.security.ssl.require = TRUE; ALTER USER [username] SET user.security.ssl.rsa_cert = "text cert authenticating this user"; ALTER USER [username] SET user.security.ssl.sslv2_enable = FALSE; ALTER USER [username] SET user.security.ssl.sslv3_require = TRUE; ALTER USER [username] SET user.schema = [schema1, schema2, schema3, public]; ALTER USER [username] SET user.security.see_other_schemas = FALSE; ALTER USER [username] SET database.name = "some non-existent dbname"; New databases, once created, would also show up in the MIB hierarchy, allowing things like: ALTER USER [username] SET database.[dbname].readonly TO TRUE; That last option, for example, would let users connect to a centrally hosted database, but would spoof the dbname that the user would see via CURRENT_DATABASE. I could imagine it being useful for hosted DB environments wherein you want to give the user the illusion of a private database. Same with user.security.see_other_schemas. Where the textual OIDs would be converted to their numeric counterparts and then stored with their value in useconfig. Now that PostgreSQL has slick array support (thanks Joe!), the various user options could be stored as array elements in the pg_catalog.pg_shadow.useconfig column. Imagine adding a syntax for every feature that a user could have vs. setting user features via a GUC/MIB system. I'd take the MIB system any day of the week and twice on Friday given the resulting reduction of bloat to gram.y. -sc -- Sean Chittenden ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions?
Sean Chittenden <[EMAIL PROTECTED]> writes: >> I'm not objecting to the idea of being able to make users read-only. >> I'm objecting to using GUC for it. Send in a patch that, say, adds >> a bool column to pg_shadow, and I'll be happy. > How is that any different than ALTER USER [username] SET > jail_read_only_transactions TO true? It sets something in > pg_shadow.useconfig column, which is permanent. But it has to go through a mechanism that is designed and built to allow that value to be overridden from other places. I think using GUC for this is just asking for trouble. Even if there is no security hole today, it's very easy to imagine future changes in GUC that would unintentionally create one. regards, tom lane ---(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] [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions?
> >> I'm not objecting to the idea of being able to make users > >> read-only. I'm objecting to using GUC for it. Send in a patch > >> that, say, adds a bool column to pg_shadow, and I'll be happy. > > > How is that any different than ALTER USER [username] SET > > jail_read_only_transactions TO true? It sets something in > > pg_shadow.useconfig column, which is permanent. > > But it has to go through a mechanism that is designed and built to > allow that value to be overridden from other places. I think using > GUC for this is just asking for trouble. Even if there is no > security hole today, it's very easy to imagine future changes in GUC > that would unintentionally create one. *nods* Anything that goes outside of SetConfigOption(), however, is incorrectly setting a GUC value and can't really be prevented. At the C level, nothing is safe and there's no way to make things 100% secure (except for possibly by moving PostgreSQL over into protected mode). If PostgreSQL can't trust itself, who can it trust? If you're worried about someone setting JailReadOnlyXacts or XactsReadOnly in a C extension, then let me hide those two variables away in their own file, declare them static, and provide accessor methods to the variables. It doesn't prevent someone from changing their values if they know the address, but it'll at least prevent someone from #include'ing a header and mucking with things. Would moving things into their own files and declaring them static be a sufficient compromise? I'll declare the accessor functions inline too, that way there should be zero loss of performance given XactReadOnly is frequently used. -sc -- Sean Chittenden ---(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] [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions?
If we change default_transaction_read_only to PGC_USERLIMIT, the administrator can turn it on and off, but an ordinary user can only turn it on, but not off. Would that help? --- Sean Chittenden wrote: -- Start of PGP signed section. > > - Read only transactions, bringing a greater level of > > security to web and enterprise applications by protecting > > data from modification. > > > > >> This should be removed. Even though I added it to the press > > >> release, I've just realised it's not really a security measure > > >> against SQL injection since injected code can just specify 'SET > > >> TRANSACTION READ WRITE'. We should still mention it, but not as a > > >> security measure. > > > > > Aside from spec compliance, whats the bonus for having it then? Or > > > put a better way, why/when would I want to use this? > > > > If I am writing a "report program" that isn't supposed to do any > > updates to anything, then I would be quite happy to set things to > > READ-ONLY as it means that I won't _accidentally_ do updates. > > > > It's like adding a pair of suspenders to your wardrobe. You can > > _always_, if you really try, get your pants to fall down, but this > > provides some protection. > > > > I would NOT call it a "security" provision, as it is fairly easily > > defeated using SET TRANSACTION. > > Um, why not make it an actual full blown security feature by applying > the following patch? This gives PostgreSQL real read only > transactions that users can't escape from. Notes about the patch: > > *) If the GUC transaction_force_read_only is set to FALSE, nothing >changes in PostgreSQL's behavior. The default is FALSE, letting >users change from READ ONLY to READ WRITE at will. > > *) If transaction_force_read_only is TRUE, this sandboxes the >connection for the remainder of the connection if the session is >set to read only. The following bits apply: > >a) if you're a super user, you can change transaction_read_only. > >b) if you're not a super user, you can change transaction_read_only > to true. > >c) if you're not a super user, you can always change > transaction_read_only from false to true. If > transaction_force_read_only is true, you can't change > transaction_read_only from true to false. > >d) If transaction_force_read_only is TRUE, but > transaction_read_only is FALSE, the transaction is still READ > WRITE. > >e) Only super users can change transaction_force_read_only. > > > Basically, if you want to permanently prevent a user from ever being > able to get in a non-read only transaction, do: > > \c [dbname] [db_superuser] > BEGIN; > ALTER USER test SET default_transaction_read_only TO TRUE; > ALTER USER test SET transaction_force_read_only TO TRUE; > COMMIT; > > -- To test: > regression=# \c regression test > regression=> SHOW transaction_read_only; > transaction_read_only > --- > on > (1 row) > > regression=> SHOW transaction_force_read_only; > transaction_force_read_only > - > on > (1 row) > > regression=> SET transaction_read_only TO FALSE; > ERROR: Insufficient privileges to SET transaction_read_only TO FALSE > > > It's also possible to set transaction_force_read_only in > postgresql.conf making it possible to create read only databases to > non-superusers by starting postgresql with > default_transaction_read_only and transaction_force_read_only set to > TRUE. If this patch is well received, I'll quickly bang out some > documentation for this new GUC. From a security stand point, this is > a nifty feature. -sc > > -- > Sean Chittenden [ Attachment, skipping... ] -- End of PGP section, PGP failed! -- 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 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions?
Tom, have you considered using PGC_USERLIMIT for the existing default_transaction_read_only variable? You could allow admins to turn it on and off, but non-admins could only turn it on. --- Tom Lane wrote: > Sean Chittenden <[EMAIL PROTECTED]> writes: > >> I'm not objecting to the idea of being able to make users read-only. > >> I'm objecting to using GUC for it. Send in a patch that, say, adds > >> a bool column to pg_shadow, and I'll be happy. > > > How is that any different than ALTER USER [username] SET > > jail_read_only_transactions TO true? It sets something in > > pg_shadow.useconfig column, which is permanent. > > But it has to go through a mechanism that is designed and built to allow > that value to be overridden from other places. I think using GUC for > this is just asking for trouble. Even if there is no security hole > today, it's very easy to imagine future changes in GUC that would > unintentionally create one. > > regards, tom lane > > ---(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 > -- 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 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
[PATCHES] Any unapplied patches out there?
Does anyone have any outstanding patches they expected would be applied for 7.4? I think everything in my inbox has been dealt with, but I might have missed something. Bruce is not done generating release notes, so we probably will not have a formal beta package ready until Friday. However, CVS tip or a nightly snapshot tarball should be pretty-durn-close-to-the-beta. regards, tom lane ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] updateable cursors
Can I get some comments on this? I know the work was completed pre-feature freeze, but submitted only recently. --- Gavin Sherry wrote: > Attached is a patch implementing updatable cursors in HEAD. Regression > test and documentation are included. > > Updateable cursors are used as follows: > > begin; > declare foo cursor for select * from bar for update; > fetch foo; > update bar set abc='def' where current of foo; > fetch foo; > delete from bar where current of foo; > commit; > > > Two points: > > i) The patch doesn't implement updateable cursors for cursors marked WITH > HOLD. I have working code for this and will send it in soon. > > ii) I've implemented a new snapshot type since, AFAICT, updateable cursors > have a type of tuple visibility. Namely, if the base table of a cursor is > updated based on that cursor, the new and old tuples are removed from the > cursor's set of visible tuples. Like wise, deleted tuples are also > removed. > > Thanks, > > Gavin Content-Description: [ Attachment, skipping... ] > > ---(end of broadcast)--- > TIP 8: explain analyze is your friend -- 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
Re: [PATCHES] Spelling fix
Patch applied. Thanks. --- Rod Taylor wrote: -- Start of PGP signed section. > contraints -> constraints [ Attachment, skipping... ] -- End of PGP section, PGP failed! -- 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 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] ruleutils with pretty-print option
I didn't like the functions ending in _ext. I renamed them to _pp for
pretty print. Patch attached and applied.
---
Tom Lane wrote:
> Andreas Pflug <[EMAIL PROTECTED]> writes:
> > I recoded the stuff as Tom recommended, leaving the non-pretty version
> > function names as they used to be, inventing new pg_get__ext
> > functions for the extended stuff, and pushing the code down into
> > pg_get__worker functions when needed. We now need the additional
> > prototype include patch from builtins.h.
>
> Applied with some editorializing. In particular, I don't believe the
> original did the right thing with (a - (b - c)).
>
> regards, tom lane
>
> ---(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
Index: src/backend/utils/adt/ruleutils.c
===
RCS file: /cvsroot/pgsql-server/src/backend/utils/adt/ruleutils.c,v
retrieving revision 1.147
diff -c -c -r1.147 ruleutils.c
*** src/backend/utils/adt/ruleutils.c 30 Jul 2003 22:56:23 - 1.147
--- src/backend/utils/adt/ruleutils.c 31 Jul 2003 04:48:35 -
***
*** 228,234
Datum
! pg_get_ruledef_ext(PG_FUNCTION_ARGS)
{
Oid ruleoid = PG_GETARG_OID(0);
boolpretty = PG_GETARG_BOOL(1);
--- 228,234
Datum
! pg_get_ruledef_pp(PG_FUNCTION_ARGS)
{
Oid ruleoid = PG_GETARG_OID(0);
boolpretty = PG_GETARG_BOOL(1);
***
*** 337,343
Datum
! pg_get_viewdef_ext(PG_FUNCTION_ARGS)
{
/* By OID */
Oid viewoid = PG_GETARG_OID(0);
--- 337,343
Datum
! pg_get_viewdef_pp(PG_FUNCTION_ARGS)
{
/* By OID */
Oid viewoid = PG_GETARG_OID(0);
***
*** 369,375
Datum
! pg_get_viewdef_name_ext(PG_FUNCTION_ARGS)
{
/* By qualified name */
text *viewname = PG_GETARG_TEXT_P(0);
--- 369,375
Datum
! pg_get_viewdef_name_pp(PG_FUNCTION_ARGS)
{
/* By qualified name */
text *viewname = PG_GETARG_TEXT_P(0);
***
*** 630,636
}
Datum
! pg_get_indexdef_ext(PG_FUNCTION_ARGS)
{
Oid indexrelid = PG_GETARG_OID(0);
int32 colno = PG_GETARG_INT32(1);
--- 630,636
}
Datum
! pg_get_indexdef_pp(PG_FUNCTION_ARGS)
{
Oid indexrelid = PG_GETARG_OID(0);
int32 colno = PG_GETARG_INT32(1);
***
*** 856,862
}
Datum
! pg_get_constraintdef_ext(PG_FUNCTION_ARGS)
{
Oid constraintId = PG_GETARG_OID(0);
boolpretty = PG_GETARG_BOOL(1);
--- 856,862
}
Datum
! pg_get_constraintdef_pp(PG_FUNCTION_ARGS)
{
Oid constraintId = PG_GETARG_OID(0);
boolpretty = PG_GETARG_BOOL(1);
***
*** 1175,1181
}
Datum
! pg_get_expr_ext(PG_FUNCTION_ARGS)
{
text*expr = PG_GETARG_TEXT_P(0);
Oid relid = PG_GETARG_OID(1);
--- 1175,1181
}
Datum
! pg_get_expr_pp(PG_FUNCTION_ARGS)
{
text*expr = PG_GETARG_TEXT_P(0);
Oid relid = PG_GETARG_OID(1);
Index: src/include/catalog/pg_proc.h
===
RCS file: /cvsroot/pgsql-server/src/include/catalog/pg_proc.h,v
retrieving revision 1.310
diff -c -c -r1.310 pg_proc.h
*** src/include/catalog/pg_proc.h 30 Jul 2003 22:56:24 - 1.310
--- src/include/catalog/pg_proc.h 31 Jul 2003 04:48:40 -
***
*** 3406,3422
DESCR("I/O");
/* System-view support functions with pretty-print option */
! DATA(insert OID = 2504 ( pg_get_ruledef PGNSP PGUID 12 f f t f s 2 25 "26
16" pg_get_ruledef_ext - _null_ ));
DESCR("source text of a rule with pretty-print option");
! DATA(insert OID = 2505 ( pg_get_viewdef PGNSP PGUID 12 f f t f s 2 25 "25
16" pg_get_viewdef_name_ext - _null_ ));
DESCR("select statement of a view with pretty-print option");
! DATA(insert OID = 2506 ( pg_get_viewdef PGNSP PGUID 12 f f t f s 2 25 "26
16" pg_get_viewdef_ext - _null_ ));
DESCR("select statement of a view with pretty-print option");
! DATA(insert OID = 2507 ( pg_get_indexdefPGNSP PGUID 12 f f t f s 3 25 "26
23 16" pg_get_indexdef_ext - _null_ ));
DESCR("index description (full create statement or single expression) with
pretty-print option");
! DATA(insert OID
Re: [PATCHES] ruleutils with pretty-print option
Patch applied, modified by Tom and myself.
---
Andreas Pflug wrote:
> Bruce Momjian wrote:
>
> >Andreas, looks good, but I need a diff -c, context diff.
> >
> >
> >
> Hi Bruce,
> I intentionally only attached only non-context diffs because the patch
> is about 50 % size of the original file. Now, here's the same as context
> diff.
>
> Regards,
> Andreas
> Index: pg_proc.h
> ===
> RCS file: /projects/cvsroot/pgsql-server/src/include/catalog/pg_proc.h,v
> retrieving revision 1.309
> diff -c -r1.309 pg_proc.h
> *** pg_proc.h 1 Jul 2003 00:04:38 - 1.309
> --- pg_proc.h 22 Jul 2003 12:52:07 -
> ***
> *** 3405,3410
> --- 3405,3424
> DATA(insert OID = 2503 ( anyarray_sendPGNSP PGUID 12 f f t f s 1
> 17 "2277" anyarray_send - _null_ ));
> DESCR("I/O");
>
> + /* System-view support functions with pretty-print option */
> + DATA(insert OID = 2504 ( pg_get_ruledef PGNSP PGUID 12 f f t f s 2 25 "26
> 16" pg_get_ruledef - _null_ ));
> + DESCR("source text of a rule with pretty-print option");
> + DATA(insert OID = 2505 ( pg_get_viewdef PGNSP PGUID 12 f f t f s 2 25 "25
> 16" pg_get_viewdef_name - _null_ ));
> + DESCR("select statement of a view with pretty-print option");
> + DATA(insert OID = 2506 ( pg_get_viewdef PGNSP PGUID 12 f f t f s 2 25 "26
> 16" pg_get_viewdef - _null_ ));
> + DESCR("select statement of a view with pretty-print option");
> + DATA(insert OID = 2507 ( pg_get_indexdef PGNSP PGUID 12 f f t f s 3 25 "26
> 23 16" pg_get_indexdef - _null_ ));
> + DESCR("index description (full create statement or single expression) with
> pretty-print option");
> + DATA(insert OID = 2508 ( pg_get_constraintdef PGNSP PGUID 12 f f t f s 2 25 "26
> 16" pg_get_constraintdef - _null_ ));
> + DESCR("constraint description with pretty-print option");
> + DATA(insert OID = 2509 ( pg_get_expr PGNSP PGUID 12 f f t f s 3
> 25 "25 26 16" pg_get_expr - _null_ ));
> + DESCR("deparse an encoded expression with pretty-print option");
> +
>
> /*
>* Symbolic values for provolatile column: these indicate whether the result
> Index: ruleutils.c
> ===
> RCS file: /projects/cvsroot/pgsql-server/src/backend/utils/adt/ruleutils.c,v
> retrieving revision 1.145
> diff -c -r1.145 ruleutils.c
> *** ruleutils.c 4 Jul 2003 02:51:34 - 1.145
> --- ruleutils.c 22 Jul 2003 12:54:10 -
> ***
> *** 71,76
> --- 71,95
> #include "utils/lsyscache.h"
>
>
> + /**
> + * Pretty formatting constants
> + **/
> +
> + /* Indent counts */
> + #define PRETTYINDENT_STD8
> + #define PRETTYINDENT_JOIN 13
> + #define PRETTYINDENT_JOIN_ON(PRETTYINDENT_JOIN-PRETTYINDENT_STD)
> + #define PRETTYINDENT_VAR4
> +
> + /* Pretty flags */
> + #define PRETTYFLAG_PAREN1
> + #define PRETTYFLAG_INDENT 2
> +
> + /* macro to test if pretty action needed */
> + #define PRETTY_PAREN(context) (context->prettyFlags & PRETTYFLAG_PAREN)
> + #define PRETTY_INDENT(context) (context->prettyFlags & PRETTYFLAG_INDENT)
> +
> +
> /* --
>* Local data types
>* --
> ***
> *** 81,86
> --- 100,107
> {
> StringInfo buf;/* output buffer to append to */
> List *namespaces; /* List of deparse_namespace nodes */
> + int prettyFlags;/* enabling/disabling of
> pretty-print functions */
> + int indentLevel;/* for prettyPrint, current space
> indents are counted */
> boolvarprefix; /* TRUE to print prefixes on Vars */
> } deparse_context;
>
> ***
> *** 123,135
>* as a parameter, and append their text output to its contents.
>* --
>*/
> ! static text *pg_do_getviewdef(Oid viewoid);
> static void decompile_column_index_array(Datum column_index_array, Oid relId,
>StringInfo buf);
> ! static void make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc);
> ! static void make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc);
> static void get_query_def(Query *query, StringInfo buf, List *parentnamespace,
> ! TupleDesc resultDesc);
> static void get_select_query_def(Query *query, deparse_context *context,
>TupleDesc resultDesc);
> static void get_insert_query_def(Query *query, deparse_context *context);
> --- 144,162
>* as a parameter, and append their text output to its contents.
>* --
>*/
> ! static char *get_simpl
Re: [PATCHES] ruleutils with pretty-print option
Didn't Tom already apply that???
Chris
- Original Message -
From: "Bruce Momjian" <[EMAIL PROTECTED]>
To: "Tom Lane" <[EMAIL PROTECTED]>
Cc: "Andreas Pflug" <[EMAIL PROTECTED]>;
<[EMAIL PROTECTED]>
Sent: Thursday, July 31, 2003 12:52 PM
Subject: Re: [PATCHES] ruleutils with pretty-print option
>
> I didn't like the functions ending in _ext. I renamed them to _pp for
> pretty print. Patch attached and applied.
>
> --
-
>
> Tom Lane wrote:
> > Andreas Pflug <[EMAIL PROTECTED]> writes:
> > > I recoded the stuff as Tom recommended, leaving the non-pretty version
> > > function names as they used to be, inventing new pg_get__ext
> > > functions for the extended stuff, and pushing the code down into
> > > pg_get__worker functions when needed. We now need the additional
> > > prototype include patch from builtins.h.
> >
> > Applied with some editorializing. In particular, I don't believe the
> > original did the right thing with (a - (b - c)).
> >
> > regards, tom lane
> >
> > ---(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
>
> Index: src/backend/utils/adt/ruleutils.c
> ===
> RCS file: /cvsroot/pgsql-server/src/backend/utils/adt/ruleutils.c,v
> retrieving revision 1.147
> diff -c -c -r1.147 ruleutils.c
> *** src/backend/utils/adt/ruleutils.c 30 Jul 2003 22:56:23 - 1.147
> --- src/backend/utils/adt/ruleutils.c 31 Jul 2003 04:48:35 -
> ***
> *** 228,234
>
>
> Datum
> ! pg_get_ruledef_ext(PG_FUNCTION_ARGS)
> {
> Oid ruleoid = PG_GETARG_OID(0);
> bool pretty = PG_GETARG_BOOL(1);
> --- 228,234
>
>
> Datum
> ! pg_get_ruledef_pp(PG_FUNCTION_ARGS)
> {
> Oid ruleoid = PG_GETARG_OID(0);
> bool pretty = PG_GETARG_BOOL(1);
> ***
> *** 337,343
>
>
> Datum
> ! pg_get_viewdef_ext(PG_FUNCTION_ARGS)
> {
> /* By OID */
> Oid viewoid = PG_GETARG_OID(0);
> --- 337,343
>
>
> Datum
> ! pg_get_viewdef_pp(PG_FUNCTION_ARGS)
> {
> /* By OID */
> Oid viewoid = PG_GETARG_OID(0);
> ***
> *** 369,375
>
>
> Datum
> ! pg_get_viewdef_name_ext(PG_FUNCTION_ARGS)
> {
> /* By qualified name */
> text*viewname = PG_GETARG_TEXT_P(0);
> --- 369,375
>
>
> Datum
> ! pg_get_viewdef_name_pp(PG_FUNCTION_ARGS)
> {
> /* By qualified name */
> text*viewname = PG_GETARG_TEXT_P(0);
> ***
> *** 630,636
> }
>
> Datum
> ! pg_get_indexdef_ext(PG_FUNCTION_ARGS)
> {
> Oid indexrelid = PG_GETARG_OID(0);
> int32 colno = PG_GETARG_INT32(1);
> --- 630,636
> }
>
> Datum
> ! pg_get_indexdef_pp(PG_FUNCTION_ARGS)
> {
> Oid indexrelid = PG_GETARG_OID(0);
> int32 colno = PG_GETARG_INT32(1);
> ***
> *** 856,862
> }
>
> Datum
> ! pg_get_constraintdef_ext(PG_FUNCTION_ARGS)
> {
> Oid constraintId = PG_GETARG_OID(0);
> bool pretty = PG_GETARG_BOOL(1);
> --- 856,862
> }
>
> Datum
> ! pg_get_constraintdef_pp(PG_FUNCTION_ARGS)
> {
> Oid constraintId = PG_GETARG_OID(0);
> bool pretty = PG_GETARG_BOOL(1);
> ***
> *** 1175,1181
> }
>
> Datum
> ! pg_get_expr_ext(PG_FUNCTION_ARGS)
> {
> text *expr = PG_GETARG_TEXT_P(0);
> Oid relid = PG_GETARG_OID(1);
> --- 1175,1181
> }
>
> Datum
> ! pg_get_expr_pp(PG_FUNCTION_ARGS)
> {
> text *expr = PG_GETARG_TEXT_P(0);
> Oid relid = PG_GETARG_OID(1);
> Index: src/include/catalog/pg_proc.h
> ===
> RCS file: /cvsroot/pgsql-server/src/include/catalog/pg_proc.h,v
> retrieving revision 1.310
> diff -c -c -r1.310 pg_proc.h
> *** src/include/catalog/pg_proc.h 30 Jul 2003 22:56:24 - 1.310
> --- src/include/catalog/pg_proc.h 31 Jul 2003 04:48:40 -
> ***
> *** 3406,3422
> DESCR("I/O");
>
> /* System-view support functions with pretty-print option */
> ! DATA(insert OID = 2504 ( pg_get_ruledefPGNSP PGUID 12 f f t f s 2
25 "26 16" pg_get_ruledef_ext - _null_ ));
> DESCR("source text of a rule with pretty-print option");
> ! DATA(insert OID = 2505 ( pg_get_viewdefPGNSP PGUID 12 f f t f s 2
25 "25 16" pg_get_viewdef_name_ext - _null_ ));
> DESCR("select statement of a view with pretty-print option");
> ! DATA(insert OID = 2506 ( pg_get_viewdefPGNSP PGUID 12 f f t f s 2
25 "26 16" pg_get_viewdef_ext - _null_ ));
> DESCR("select statement of a view with pretty-print option");
> ! DATA(insert OID = 2507 ( pg_get_inde
Re: [PATCHES] updateable cursors
Bruce Momjian <[EMAIL PROTECTED]> writes: > Can I get some comments on this? It's far from ready for prime time :-( The main problem with it is it's doing the wrong things in the wrong places. The key part of the patch is a hack in parse_expr.c to transform "WHERE CURRENT OF cursor" into "WHERE ctid = ", where the TID constant has been obtained by reaching into the cursor and grabbing the current position. Parse analysis is entirely the wrong time to be doing that --- in any context where the statement will not be executed immediately, there is no good reason to expect that the cursor even exists, let alone is positioned at the row it will be positioned at when you finally execute the query. This approach cannot support WHERE CURRENT OF in plpgsql functions, rules, or prepared statements. A lesser but still fatal objection is that the rule reverse-lister would show the query as "WHERE ctid = " rather than "WHERE CURRENT OF". AFAICS, WHERE CURRENT OF has to be a construct that propagates in just that form all the way into the executor (with a hack in the planner to ensure that a TidScan plan is chosen) and then the reaching into the cursor has to happen at plan startup in nodeTidscan.c. The business with a substitute snapshot seems pretty dubious too. It looks like a cursor that started with a normal snapshot --- some transactions visible, some not --- will suddenly flip into SnapshotNow except for the specific row(s) updated by the current transaction. That's not gonna do. In the first place, only rows updated *through that cursor* ought to change visibility, according to my reading of the spec. In the second place, SnapshotNow will allow visibility of external transactions that should not have been visible according to the original snapshot. I don't understand what the diffs in postgres.c are supposed to accomplish, but I can just about promise that they are misplaced too, since they won't affect queries executed via functions or SPI. 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] ruleutils with pretty-print option
Bruce Momjian <[EMAIL PROTECTED]> writes: > I didn't like the functions ending in _ext. I renamed them to _pp for > pretty print. Patch attached and applied. Seems to be shy a catversion bump; since you have just made an incompatible change in the internal-function-names array, one is needed. But I disagree with your opinion anyway --- _ext for "extended" made perfect sense to me, especially for the get_indexdef one which adds more than just prettyprint functionality. regards, tom lane ---(end of broadcast)--- TIP 8: explain analyze is your friend
[PATCHES] Minor verbosity increase for analyze
Attached is a (very small) patch to make analyze display some interesting info in verbose mode about the analyzed relation (pages, rows per block and rows ). This is all available in the statistics catalog but I have found it useful to see it appear as you run... mods to : src/backend/commands/analyze.c plain diff against current CVS (31/07/2003 NZST) Best wishes Mark 696a697,702 > /* >* emit some interesting relation info >*/ > elog(elevel, " pages = %d rows/page = %d rows = %d", > onerel->rd_nblocks, (int)tuplesperpage, (int)*totalrows); > ---(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
