[HACKERS] missing entry in GucSource_Names
It appears that the patch here: http://git.postgresql.org/gitweb?p=postgresql.git;a=commit;h=a30fa4ca33d055c46bebc0e5c701d5b4fd27814d missed adding PGC_S_DATABASE_USER to a few locations, most notably GucSource_Names, where the PGC_S_SESSION now points off the end of the array. Patch attached. Regards, Jeff Davis diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 1f63e06..776efe3 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -460,6 +460,7 @@ const char *const GucSource_Names[] = /* PGC_S_ARGV */ "command line", /* PGC_S_DATABASE */ "database", /* PGC_S_USER */ "user", + /* PGC_S_DATABASE_USER */ "database user", /* PGC_S_CLIENT */ "client", /* PGC_S_OVERRIDE */ "override", /* PGC_S_INTERACTIVE */ "interactive", @@ -4556,7 +4557,8 @@ set_config_option(const char *name, const char *value, */ elevel = IsUnderPostmaster ? DEBUG3 : LOG; } - else if (source == PGC_S_DATABASE || source == PGC_S_USER) + else if (source == PGC_S_DATABASE || source == PGC_S_USER || + source == PGC_S_DATABASE_USER) elevel = WARNING; else elevel = ERROR; @@ -5762,6 +5764,7 @@ define_custom_variable(struct config_generic * variable) break; case PGC_S_DATABASE: case PGC_S_USER: + case PGC_S_DATABASE_USER: case PGC_S_CLIENT: case PGC_S_SESSION: default: -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] transaction_isolation vs. default_transaction_isolation
Itagaki-san, > BEGIN; > SET transaction_isolation = 'serializable'; > SET default_transaction_isolation = 'read committed'; > SHOW transaction_isolation; > => serializable > SHOW default_transaction_isolation; > => read committed > COMMIT; > -- next transaction uses default_transaction_isolation > SHOW transaction_isolation; > => read committed Thank you; that was very informative. --Josh -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] transaction_isolation vs. default_transaction_isolation
On Mon, 2009-10-12 at 22:13 -0700, Josh Berkus wrote: > However, for *two* settings, and two settings only, we distinguish that > by naming an identical setting "default_*" in postgresql.conf. This is > confusing and inconsistent with the rest of the GUCS. Namely: > > default_transaction_isolation > default_transaction_read_only I think they are named "default_" because whatever you specify at the beginning of a transaction overrides the GUC. For example, in: BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED; SET default_transaction_isolation=serializable; ... the "default_" makes it more clear which setting overrides the other. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] transaction_isolation vs. default_transaction_isolation
Josh Berkus wrote: > default_transaction_isolation > default_transaction_read_only They are settings of transaction_isolation and transaction_read_only for *next* transactions, no? > transaction_isolation > transaction_read_only Non-default versions are almost read-only variables because we can set them at the beginning of transactions. BEGIN; SET transaction_isolation = 'serializable'; SET default_transaction_isolation = 'read committed'; SHOW transaction_isolation; => serializable SHOW default_transaction_isolation; => read committed COMMIT; -- next transaction uses default_transaction_isolation SHOW transaction_isolation; => read committed Regards, --- ITAGAKI Takahiro NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] transaction_isolation vs. default_transaction_isolation
Hackers, A slew of settings in postgresql.conf, including work_mem, search_path, DateStyle, and about 80 others are effectively just defaults for new connections, since they can be changed by any user. However, for *two* settings, and two settings only, we distinguish that by naming an identical setting "default_*" in postgresql.conf. This is confusing and inconsistent with the rest of the GUCS. Namely: default_transaction_isolation default_transaction_read_only transaction_isolation transaction_read_only For 8.5, I would like to consolidate these into only 2 settings and drop the default_* settings. --Josh Berkus -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Skip WAL in ALTER TABLE
We can skip writing WAL during COPY and CLUSTER if archive_mode is off, but we don't use the skipping during tables rewrites in ALTER TABLE. Also we don't use BulkInsertState there. Is it possible to use WAL-skipping and BulkInsertState in ATRewriteTable() ? If ok, I'll submit a patch for the next commitfest. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CTE bug?
David Fetter writes: > WITH RECURSIVE t1(n) AS ( > VALUES(2) > UNION ALL > SELECT n+1 FROM t1 WHERE n < 1000 > ), > t2 (n, i) AS ( > SELECT 2*n+2, 2 > FROM t1 WHERE 2*n+2 <= 1000 > UNION ALL > WITH t3(k) AS ( > SELECT max(i) FROM t2 > ) > SELECT k*n+k, k > FROM > t1 > CROSS JOIN > t3 > WHERE k*n+k <= 1000 > ) > SELECT * FROM t1 EXCEPT SELECT n FROM t2 ORDER BY 1; > ERROR: syntax error at or near "WITH t3" > LINE 10: WITH t3(k) AS ( > ^ I think you need parentheses to have WITH attached to a member of a UNION. In WITH (...) SELECT x UNION SELECT y I'm pretty sure the WITH attaches to the whole union, not the first member. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] CTE bug?
Folks, While working to write the Sieve of Eratosthenes using CTEs, I ran across a strange error, namely that it appears I'm not allowed to nest WITH. Is this a bug? Cheers, David. WITH RECURSIVE t1(n) AS ( VALUES(2) UNION ALL SELECT n+1 FROM t1 WHERE n < 1000 ), t2 (n, i) AS ( SELECT 2*n+2, 2 FROM t1 WHERE 2*n+2 <= 1000 UNION ALL WITH t3(k) AS ( SELECT max(i) FROM t2 ) SELECT k*n+k, k FROM t1 CROSS JOIN t3 WHERE k*n+k <= 1000 ) SELECT * FROM t1 EXCEPT SELECT n FROM t2 ORDER BY 1; ERROR: syntax error at or near "WITH t3" LINE 10: WITH t3(k) AS ( ^ -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GRANT ON ALL IN schema
2009/10/13 Tom Lane : > I started looking at this, and the first thing I noticed was that it > adds TABLES, FUNCTIONS, and SEQUENCES as unreserved keywords. Now > I'm not a fan of bloating the parser that way, but I have to admit > that "GRANT ON ALL TABLE IN SCHEMA" wouldn't read well. What I am > wondering is whether we should not go back and adjust the syntax > for the default-ACLs patch to use the same keywords, ie not > > ALTER DEFAULT PRIVILEGES ... GRANT ... ON TABLE TO ... > > but > > ALTER DEFAULT PRIVILEGES ... GRANT ... ON TABLES TO ... > > Comments? My personal feeling is that the syntax of ALTER DEFAULT PRIVILEGES works fine as it stands. When you specify a default priv of "GRANT SELECT ON TABLE TO dave" on a schema, it means that whenever you create a table it implicitly does a "GRANT SELECT ON TO dave". I think the symmetry between the default priv and the related GRANT outweighs the consideration of whether the command parses more like a valid English sentence. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Reworks for Access Control facilities (r2350)
Stephen, Thanks for your reviewing comments, although you have busy days. Stephen Frost wrote: > KaiGai, > > * KaiGai Kohei (kai...@ak.jp.nec.com) wrote: >> Please review the new revision, Thanks, > > In general, I'm pretty happy with this revision. You still have a > number of places where you have comments about code which does not exist > any more. For example, the comments about the check being removed from > LookupCreationNamespace. I would recommend pulling out those comments > and instead having a comment at the top of the function that says > "namespace creation permission checks are handled in the individual > object ac_*_create() routines". > > I don't like having comments that are about code which was removed. > Some of these could be moved to the README if they aren't there already > and they really need to be kept. OK, I'll check and revise these commenting issues soon. Please wait for a couple of days at most. > There are some other grammatical and spelling issues in the comments, > but I don't believe any of this should hold this patch up from being > ready for committer. At a minimum, I think this really needs to have a > committer comment on it to ensure we're going in the right direction. > I'd be happy to continue working with KaiGai to review his changes going > forward, either with the next set of SE-PG patches or reworking this one > if necessary. > > Thanks, > > Stephen -- OSS Platform Development Division, NEC KaiGai Kohei -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Largeobject access controls
The attached patch is the revised one for largeobejct access controls, because it conflicted to the "GRANT/REVOKE ON ALL xxx IN SCHEMA". No other changes are here. Please apply this one, instead of the older revision (r2354). Thanks, $ diffstat /home/kaigai/RPMS/SOURCES/sepgsql-02-blob-8.5devel-r2362.patch.gz doc/src/sgml/config.sgml | 28 + doc/src/sgml/ref/allfiles.sgml|1 doc/src/sgml/ref/alter_large_object.sgml | 75 doc/src/sgml/ref/grant.sgml |8 doc/src/sgml/ref/revoke.sgml |6 doc/src/sgml/reference.sgml |1 src/backend/catalog/Makefile |6 src/backend/catalog/aclchk.c | 294 ++ src/backend/catalog/dependency.c | 14 src/backend/catalog/pg_largeobject.c | 406 + src/backend/catalog/pg_shdepend.c |5 src/backend/commands/alter.c |5 src/backend/commands/comment.c|7 src/backend/commands/tablecmds.c |1 src/backend/libpq/be-fsstubs.c| 49 +- src/backend/parser/gram.y | 21 + src/backend/storage/large_object/inv_api.c| 108 +---! src/backend/tcop/utility.c|3 src/backend/utils/adt/acl.c |5 src/backend/utils/misc/guc.c | 10 src/backend/utils/misc/postgresql.conf.sample |1 src/bin/psql/large_obj.c | 10 src/include/catalog/dependency.h |1 src/include/catalog/indexing.h|3 src/include/catalog/pg_largeobject.h |4 src/include/catalog/pg_largeobject_metadata.h | 68 src/include/nodes/parsenodes.h|1 src/include/utils/acl.h |6 src/test/regress/expected/privileges.out | 206 + src/test/regress/expected/sanity_check.out|3 src/test/regress/sql/privileges.sql | 84 + 31 files changed, 1167 insertions(+), 80 deletions(-), 193 modifications(!) KaiGai Kohei wrote: > Tom Lane wrote: >> KaiGai Kohei writes: >>> I rebased the largeobject access controls patch to the CVS HEAD >>> because of the patch confliction to the default ACL patch. >> Quick comment on this --- I think that using a syscache for large >> objects is probably not a good idea. There is no provision in the >> catcache code for limiting the cache size anymore, and that means that >> anybody who touches a large number of large objects is going to blow out >> memory. We removed the old cache limit code because that seemed most >> sensible for the use of the caches for regular catalog objects, but >> I don't think LOs will have the same characteristics with respect to >> either number of objects or locality of access. > > The attached patch is the revised largeobject access controls. > > It replaced any usage of system cache for pg_largeobject_metadata. > In this patch, we basically follow the manner in the pg_tablespace > which also does not have its own system cache. > For example, it needs to open the pg_largeobject_metadata relation > with AccessShareLock when pg_largeobject_aclcheck() is called, as > pg_tablespace_aclcheck() doing. > > > $ diffstat sepgsql-02-blob-8.5devel-r2354.patch.gz > doc/src/sgml/config.sgml | 28 + > doc/src/sgml/ref/allfiles.sgml|1 > doc/src/sgml/ref/alter_large_object.sgml | 75 > doc/src/sgml/ref/grant.sgml |8 > doc/src/sgml/ref/revoke.sgml |6 > doc/src/sgml/reference.sgml |1 > src/backend/catalog/Makefile |6 > src/backend/catalog/aclchk.c | 294 ++ > src/backend/catalog/dependency.c | 14 > src/backend/catalog/pg_largeobject.c | 406 > + > src/backend/catalog/pg_shdepend.c |5 > src/backend/commands/alter.c |5 > src/backend/commands/comment.c|7 > src/backend/commands/tablecmds.c |1 > src/backend/libpq/be-fsstubs.c| 49 +- > src/backend/parser/gram.y | 20 + > src/backend/storage/large_object/inv_api.c| 108 +---! > src/backend/tcop/utility.c|3 > src/backend/utils/adt/acl.c |5 > src/backend/utils/misc/guc.c | 10 > src/backend/utils/misc/postgresql.conf.sample |1 > src/bin/psql/large_obj.c | 10 > src/include/catalog/dependency.h |1 > src/include/catalog/indexing.h|3 > src/include/catalog/pg_largeobject.h |4 > src/include/catalog/pg_largeobject_metadata.h | 68 > src/include/nodes/parsenodes.h|1 > src/include/utils/ac
Re: [HACKERS] Encoding issues in console and eventlog on win32
Magnus Hagander wrote: > One other question - you note that WriteConsoleW() "could fail if > stderr is redirected". Are you saying that it will always fail when > stderr is redirected, or only sometimes? If ony sometimes, do you know > under which conditions it happens? It will always fail if redirected. We can test the conditions using: pg_ctl start > result.log So, the comment should be: /* WriteConsoleW always fails if stderr is redirected. */ I cleaned up the patch per comments. I hope this will be the final one ;-). * Use in_error_recursion_trouble() instead of own implementation. * Use def_enc2name() macro to avoid adding the codepage field on non-win32 platforms. * Fix a bug of calculation of result length. * Fix a memory leak on error handling path in pgwin32_toUTF16(). > If it's always, I assume this just means that the logfile will be in > the database encoding and not in UTF16? Is this what we want, or would > we like the logfile to also be in UTF16? If we can convert it to > UTF16, that would fix the case when you have different databases in > different encodings, wouldn't it? (Even if your editor, unlike the > console subsystem, can view the individual encoding you need, I bet it > can't deal with multiple encodings in the same file) Sure, the logfile will be filled with mixed encoding strings, that could happen in logfile and syslog on non-win32 platforms. I think UTF8 is better than UTF16 for logfile encoding because there are some text editors that do not support wide characters. At any rate, the logfile encoding feature will come from another patch, that might add "log_encoding" variable and work on any platforms. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center eventlog_20091013.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Feature Request: "generate_series(DATE, DATE)" and "generate_series(DATE, DATE, INT)"
Hi,
as discussed on -general, I'd like to propose that Post-
greSQL provides "generate_series(DATE, DATE)" and
"generate_series(DATE, DATE, INT)" functions by default.
They are merely syntactic sugar for
"generate_series($1::TIMESTAMP, $2::TIMESTAMP[,
'$3 days'::INTERVAL])::DATE" and can be used whenever a
range of dates is used, e. g. to ("LEFT JOIN"ly) group rows
by calendar date.
Non-C implementations would be (credits to Sam Mason):
| CREATE FUNCTION generate_series(DATE, DATE)
|RETURNS SETOF DATE
|IMMUTABLE LANGUAGE SQL AS
|$$SELECT generate_series($1::TIMESTAMP, $2::TIMESTAMP, '1
day'::INTERVAL)::DATE;$$;
| CREATE FUNCTION generate_series(DATE, DATE, INT)
|RETURNS SETOF DATE
|IMMUTABLE LANGUAGE SQL AS
|$$SELECT generate_series($1::TIMESTAMP, $2::TIMESTAMP, ($3 || '
days')::INTERVAL)::DATE;$$;
TIA,
Tim
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GRANT ON ALL IN schema
Jaime Casanova writes: > On Mon, Oct 12, 2009 at 1:42 PM, Tom Lane wrote: >> ALTER DEFAULT PRIVILEGES ... GRANT ... ON TABLES TO ... > this makes sense to me, because you want the default to affect all new > tables not only a new single table. > so, as someone once told, +1 from me ;) Done. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GRANT ON ALL IN schema
Petr Jelinek writes: > [ GRANT ON ALL ] Applied with minor editorialization (mainly changing some choices of identifiers) regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GRANT ON ALL IN schema
On Mon, Oct 12, 2009 at 1:42 PM, Tom Lane wrote: > > ALTER DEFAULT PRIVILEGES ... GRANT ... ON TABLES TO ... > this makes sense to me, because you want the default to affect all new tables not only a new single table. so, as someone once told, +1 from me ;) -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CREATE LIKE INCLUDING COMMENTS and STORAGES
Itagaki Takahiro wrote: Andrew Dunstan wrote: I'm wondering why we are not copying comments on cloned indexes. I realize that might involve a bit more code, but I think I'd feel happier if we cloned all the comments we reasonably could from the outset. Is it really that hard to do? I found it is not so difficult as I expected; patch attached. Now it copies comments on indexes and columns of the indexes on INCLUDING COMMENTS. Regression test and documentation are also adjusted. Please review around chooseIndexName() and uses of it. The codes becomes a bit complex and might be ugly because we will have some duplicated codes; "pg_expression_%d" hacks and uses of ChooseRelationName() are spread into index.c, indexcmds.c and parse_utilcmd.c. I don't think that's a terrible tragedy - you haven't copied huge swags of code. Committed with slight adjustments for bitrot etc. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] EvalPlanQual seems a tad broken
Simon Riggs writes: > On Mon, 2009-10-12 at 11:42 -0400, Tom Lane wrote: >> The problem is that execMain.c is set up to pull as many rows as it can >> from execution of an EvalPlanQual query. Then, once that's exhausted, >> it steps to the next result from the original query. So if a row that >> requires locking joins to more than one row in some other table, you >> get duplicated output rows. > Surely the SQL Standard recognises such queries as failing test 23b) on > 7.12 (p379, SQL2008 Foundation). So the query is > not potentially updateable and should give a runtime error using a FOR > UPDATE. You're confusing our implementation of FOR UPDATE with the spec. As I said earlier, they are only very loosely related. In particular, our reading of FOR UPDATE is to lock the referenced rows, not to enforce that they are referenced only once. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] EvalPlanQual seems a tad broken
On Mon, 2009-10-12 at 11:42 -0400, Tom Lane wrote: > The problem is that execMain.c is set up to pull as many rows as it can > from execution of an EvalPlanQual query. Then, once that's exhausted, > it steps to the next result from the original query. So if a row that > requires locking joins to more than one row in some other table, you > get duplicated output rows. Surely the SQL Standard recognises such queries as failing test 23b) on 7.12 (p379, SQL2008 Foundation). So the query is not potentially updateable and should give a runtime error using a FOR UPDATE. Can we add something to check for uniqueness of the join and throw an error when we detect this situation? Seems like a better general solution. > I do not see any very good way around #2. I'm tempted to propose > that we just forbid SRFs in the targetlist of a FOR UPDATE query. > This could be justified on the same grounds that we forbid aggregate > functions there, ie, they destroy the one-to-one correspondence between > table rows and SELECT output rows. If you really had to have it you > could do something like Until we have a way to specify that the return set from an SRF is unique, that seems the only way. That would be desirable because it would allow FunctionScans to be join removed as well. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GRANT ON ALL IN schema
Petr Jelinek writes: > [ latest GRANT ALL patch ] I started looking at this, and the first thing I noticed was that it adds TABLES, FUNCTIONS, and SEQUENCES as unreserved keywords. Now I'm not a fan of bloating the parser that way, but I have to admit that "GRANT ON ALL TABLE IN SCHEMA" wouldn't read well. What I am wondering is whether we should not go back and adjust the syntax for the default-ACLs patch to use the same keywords, ie not ALTER DEFAULT PRIVILEGES ... GRANT ... ON TABLE TO ... but ALTER DEFAULT PRIVILEGES ... GRANT ... ON TABLES TO ... Comments? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Is FOR UPDATE an optimization fence?
On Mon, Oct 12, 2009 at 1:59 PM, Tom Lane wrote: > Robert Haas writes: >> On Sun, Oct 11, 2009 at 12:35 PM, Tom Lane wrote: >>> Of course the downside of changing it is that queries that worked fine >>> before might work differently (and much slower) now; first because not >>> flattening the sub-select might lead to a worse plan, and second because >>> locking more rows takes more time. >>> >>> The alternative would be to let it continue to flatten such sub-selects >>> when possible, and to tell anyone who doesn't want that to stick in >>> OFFSET 0 as an optimization fence. >>> >>> It's an entirely trivial code change either way. I'm inclined to think >>> that we should prevent flattening, on the grounds of least astonishment. > >> The other comment I have is that I *expect* subqueries to be pulled >> up. So my own personal POLA would not be violated by locking only the >> rows with a join partner; in fact it would be more likely to be >> violated by the reverse behavior. I might not be typical, though. My >> experience is that not pulling up subqueries tends to have disastrous >> effects on performance, so I'm somewhat biased against creating more >> situations where that will happen. > > On further reflection I've decided to stick with the old behavior on > this point, at least for the time being. I'm concerned about subtly > altering the behavior of existing queries, and I've also realized that > changing it isn't as much of a one-liner as I thought. The current > behavior of the parser and rewriter really depends on the assumption > that there's not much of a semantic difference between FOR UPDATE > markings at different syntactic levels, because they will happily push > down a FOR UPDATE *into* a sub-select. That is, For the record, I wasn't sure if I agreed with your original point that: select * from a join (select * from b for update) ss on a.x = ss.y; should necessarily be expected to lock all rows from b (does the standard insist on it?). The select inside the join clause describes 'how you get' the records, not that they should be all gotten. Along the same vein, does: create view foo_update as select * from foo for update; necessarily lock all the rows from foo for any query against the view? (It doesn't and IMO shouldn't). ISTM that the particular rows being locked in your first example are not really defined very well. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Is FOR UPDATE an optimization fence?
Robert Haas writes: > On Sun, Oct 11, 2009 at 12:35 PM, Tom Lane wrote: >> Of course the downside of changing it is that queries that worked fine >> before might work differently (and much slower) now; first because not >> flattening the sub-select might lead to a worse plan, and second because >> locking more rows takes more time. >> >> The alternative would be to let it continue to flatten such sub-selects >> when possible, and to tell anyone who doesn't want that to stick in >> OFFSET 0 as an optimization fence. >> >> It's an entirely trivial code change either way. I'm inclined to think >> that we should prevent flattening, on the grounds of least astonishment. > The other comment I have is that I *expect* subqueries to be pulled > up. So my own personal POLA would not be violated by locking only the > rows with a join partner; in fact it would be more likely to be > violated by the reverse behavior. I might not be typical, though. My > experience is that not pulling up subqueries tends to have disastrous > effects on performance, so I'm somewhat biased against creating more > situations where that will happen. On further reflection I've decided to stick with the old behavior on this point, at least for the time being. I'm concerned about subtly altering the behavior of existing queries, and I've also realized that changing it isn't as much of a one-liner as I thought. The current behavior of the parser and rewriter really depends on the assumption that there's not much of a semantic difference between FOR UPDATE markings at different syntactic levels, because they will happily push down a FOR UPDATE *into* a sub-select. That is, select * from a join (select * from b) ss on a.x = ss.y for update; gets transformed into select * from a join (select * from b for update of b) ss on a.x = ss.y for update of a; There isn't any simple way to avoid that with the current RowMarkClause representation, because it only applies to the current query level. Maybe we should think about changing that sometime, but it seems like material for a different patch. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [GENERAL] contrib/plantuner - enable PostgreSQL planner hints
On Mon, Oct 12, 2009 at 11:31:24AM -0400, Robert Haas wrote: > 2009/10/12 Teodor Sigaev : > >> Are you planning to submit this as a /contrib module? > > > > I haven't objections to do that, we don't planned that because we > > know sceptical relation of community to hints :) > > I think it would be pretty useful to have some additional knobs to > poke at the planner. A contrib module would certainly help test that idea, at least as far as any knobs it provides. > I sometimes want to know what the planner thinks the cost of some > plan other than the one actually selected would be. For simple > queries, it's often possible to accomplish this by using the > enable_* parameters, but those are a pretty coarse instrument and > what you can do with them is fairly limited. So I think it would be > nice to have some more options, and I wouldn't object to including > this as one of them, provided that the code isn't too much of a > kludge. > > That having been said, my tables don't tend to be heavily indexed > and the planner basically never picks the wrong one. Most of my > query planning problems (and many of the ones on -performance) are > the result of bad selectivity estimates. So what I'd really like to > see is a way to override the selectivity of a given expression. > Making the planner smarter about estimating selectivity in the first > place would be *great*, too, but I don't have much hope that it's > ever going to be perfect. Nathan Boley (cc'd) has proposed smartening it up by figuring out what class of distributions the table looks like it belongs to and acting on that. Unsure how far this got as far as code, but I suspect Nathan can address this :) Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] EvalPlanQual seems a tad broken
While fooling around with moving FOR UPDATE into a plan node (WIP version attached), I came across this interesting behavior: 1. Create test tables: create table t1(f1 int, f2 int); insert into t1 values (1,1); insert into t1 values (2,2); create table t2(f3 int, f4 int); insert into t2 values (1,111); insert into t2 values (1,112); insert into t2 values (2,113); insert into t2 values (2,114); 2. Execute test query: select * from t1 join t2 on f1=f3 for update; f1 | f2 | f3 | f4 +++- 1 | 1 | 1 | 111 1 | 1 | 1 | 112 2 | 2 | 2 | 113 2 | 2 | 2 | 114 (4 rows) 3. In another session, execute: begin; update t1 set f2=42 where f1=2; 4. In first session, repeat test query: select * from t1 join t2 on f1=f3 for update; As expected, it blocks waiting for the second session to commit. 5. Now commit in the second session. First session resumes and prints f1 | f2 | f3 | f4 +++- 1 | 1 | 1 | 111 1 | 1 | 1 | 112 2 | 42 | 2 | 113 2 | 42 | 2 | 114 2 | 42 | 2 | 113 2 | 42 | 2 | 114 (6 rows) Of course the expected answer is f1 | f2 | f3 | f4 +++- 1 | 1 | 1 | 111 1 | 1 | 1 | 112 2 | 42 | 2 | 113 2 | 42 | 2 | 114 (4 rows) which is what you'll get if you simply repeat the test query. This isn't a new bug ... the same behavior can be demonstrated as far back as 7.0. The problem is that execMain.c is set up to pull as many rows as it can from execution of an EvalPlanQual query. Then, once that's exhausted, it steps to the next result from the original query. So if a row that requires locking joins to more than one row in some other table, you get duplicated output rows. The attached patch alleviates some cases of this by having the new LockRows plan node lock *all* the target rows, not just one, before firing the EvalPlanQual query. It doesn't fix the example above because only one of the rows being joined is seen to need EPQ treatment. We could improve that by feeding successfully locked rows into the EPQ machinery as well as ones that were found to be outdated. But that would still leave us with two failure cases: 1. if some of the tables being joined are not selected FOR UPDATE. 2. if the select involves any set-returning functions in the targetlist. I think we could get around #1 by having *all* tables in the query marked FOR UPDATE at least in a dummy form, ie give them entries in the rowMarks list and create junk tlist entries to report their current ctid. Then we'd feed all the relevant rows into the EPQ machinery. We'd just not lock the ones we weren't asked to lock. I do not see any very good way around #2. I'm tempted to propose that we just forbid SRFs in the targetlist of a FOR UPDATE query. This could be justified on the same grounds that we forbid aggregate functions there, ie, they destroy the one-to-one correspondence between table rows and SELECT output rows. If you really had to have it you could do something like select srf(...) from (select ... for update) ss; Anyone have a better idea? regards, tom lane bintwJlTXVdr0.bin Description: lockrows-1.patch.gz -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [GENERAL] contrib/plantuner - enable PostgreSQL planner hints
2009/10/12 Teodor Sigaev : >> Are you planning to submit this as a /contrib module? > > I haven't objections to do that, we don't planned that because we know > sceptical > relation of community to hints :) I think it would be pretty useful to have some additional knobs to poke at the planner. I sometimes want to know what the planner thinks the cost of some plan other than the one actually selected would be. For simple queries, it's often possible to accomplish this by using the enable_* parameters, but those are a pretty coarse instrument and what you can do with them is fairly limited. So I think it would be nice to have some more options, and I wouldn't object to including this as one of them, provided that the code isn't too much of a kludge. That having been said, my tables don't tend to be heavily indexed and the planner basically never picks the wrong one. Most of my query planning problems (and many of the ones on -performance) are the result of bad selectivity estimates. So what I'd really like to see is a way to override the selectivity of a given expression. Making the planner smarter about estimating selectivity in the first place would be *great*, too, but I don't have much hope that it's ever going to be perfect. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] contrib/plantuner - enable PostgreSQL planner hints
hi there ... for this work i will include you in my evening prayers for at least one week. i know there has been a lot of discussion about this but what you just posted it excellent and more important: USEFUL to many people. i had something else in mind recently as well: virtual indexes. it would help people to decide whether and index would make sense if it would actually exist. in some cases this would make sense as well as many datasets are just to big to try out if an index help.s if there was a vote whether this should be in contrib or in core: +999 from me ... many thanks, hans Oleg Bartunov wrote: Hi there, this is an announcement of our new contribution module for PostgreSQL - Plantuner - enable planner hints (http://www.sai.msu.su/~megera/wiki/plantuner). Example: =# LOAD 'plantuner'; =# create table test(id int); =# create index id_idx on test(id); =# create index id_idx2 on test(id); =# \d test Table "public.test" Column | Type | Modifiers +-+--- id | integer | Indexes: "id_idx" btree (id) "id_idx2" btree (id) =# explain select id from test where id=1; QUERY PLAN --- Bitmap Heap Scan on test (cost=4.34..15.03 rows=12 width=4) Recheck Cond: (id = 1) -> Bitmap Index Scan on id_idx2 (cost=0.00..4.34 rows=12 width=0) Index Cond: (id = 1) (4 rows) =# set enable_seqscan=off; =# set plantuner.forbid_index='id_idx2'; =# explain select id from test where id=1; QUERY PLAN -- Bitmap Heap Scan on test (cost=4.34..15.03 rows=12 width=4) Recheck Cond: (id = 1) -> Bitmap Index Scan on id_idx (cost=0.00..4.34 rows=12 width=0) Index Cond: (id = 1) (4 rows) =# set plantuner.forbid_index='id_idx2,id_idx'; =# explain select id from test where id=1; QUERY PLAN - Seq Scan on test (cost=100.00..140.00 rows=12 width=4) Filter: (id = 1) (2 rows) Regards, Oleg _ Oleg Bartunov, Research Scientist, Head of AstroNet (www.astronet.ru), Sternberg Astronomical Institute, Moscow University, Russia Internet: o...@sai.msu.su, http://www.sai.msu.su/~megera/ phone: +007(495)939-16-83, +007(495)939-23-83 -- Cybertec Schoenig & Schoenig GmbH Reyergasse 9 / 2 A-2700 Wiener Neustadt Web: www.postgresql-support.de -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [GENERAL] contrib/plantuner - enable PostgreSQL planner hints
Teodor Sigaev wrote: > > > > Are you planning to submit this as a /contrib module? > > I haven't objections to do that, we don't planned that because we know > sceptical > relation of community to hints :) Well, the nice thing about this patch is that the hints are mostly external to the backend, and are not installed by default. I think it would make a great /contrib module. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [GENERAL] contrib/plantuner - enable PostgreSQL planner hints
Are you planning to submit this as a /contrib module? I haven't objections to do that, we don't planned that because we know sceptical relation of community to hints :) -- Teodor Sigaev E-mail: teo...@sigaev.ru WWW: http://www.sigaev.ru/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] COPY enhancements
On Thu, 2009-10-08 at 11:01 -0400, Tom Lane wrote: > So as far as I can see, the only form of COPY error handling that > wouldn't be a cruel joke is to run a separate subtransaction for each > row, and roll back the subtransaction on error. Of course the > problems > with that are (a) speed, (b) the 2^32 limit on command counter IDs > would mean a max of 2^32 rows per COPY, which is uncomfortably small > these days. Previous discussions of the problem have mentioned trying > to batch multiple rows per subtransaction to alleviate both issues. > Not easy of course, but that's why it's not been done yet. With a > patch like this you'd also have (c) how to avoid rolling back the > insertions into the logging table. (d) using too many xids will force the system to begin immediate wraparound-avoidance vacuuming to freeze rows. Dimitri's pgloader is looking even more attractive, not least because it exists and it works. (And is the reason I personally stopped considering the COPY-error-logging feature as important). -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Hot Standby and 64+ subxids (was COPY enhancements)
On Thu, 2009-10-08 at 11:50 -0400, Tom Lane wrote: > Robert Haas writes: > > Subcommitting every single row is going to be really painful, > > especially after Hot Standby goes in and we have to issue a WAL record > > after every 64 subtransactions (AIUI). > > Yikes ... I had not been following that discussion, but that sure sounds > like a deal-breaker. For HS, not this. Probably worth expanding this thought... HS writes a WAL record for subtransactions at the point that the subxid cache overflows for any single transaction. Current cache size = 64. Top-level transaction then writes one additional WAL record every additional 64 subxids after that. These are known as xid assignment records. If we execute transactions that completely fit in subxid cache we don't write any WAL records at all. There is no cumulative effect. So in most applications, we never write xid assignment records at all. Does that cover your objection, or do you see other issues? -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
