[HACKERS] FK NOT VALID can't be deferrable?
Hi, Testing the CHECK NOT VALID patch i found $subject... is this intended? -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación -- 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] time-delayed standbys
On Wed, Jun 15, 2011 at 12:58 AM, Fujii Masao wrote: > > http://forge.mysql.com/worklog/task.php?id=344 > According to the above page, one purpose of time-delayed replication is to > protect against user mistakes on master. But, when an user notices his wrong > operation on master, what should he do next? The WAL records of his wrong > operation might have already arrived at the standby, so neither "promote" nor > "restart" doesn't cancel that wrong operation. Instead, probably he should > shutdown the standby, investigate the timestamp of XID of the operation > he'd like to cancel, set recovery_target_time and restart the standby. > Something like this procedures should be documented? Or, we should > implement new "promote" mode which finishes a recovery as soon as > "promote" is requested (i.e., not replay all the available WAL records)? > i would prefer something like "pg_ctl promote -m immediate" that terminates the recovery -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación -- 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] creating CHECK constraints as NOT VALID
On Tue, Jun 14, 2011 at 4:14 PM, Alvaro Herrera wrote: > Excerpts from Alvaro Herrera's message of lun jun 13 18:08:12 -0400 2011: >> Excerpts from Dean Rasheed's message of sáb jun 11 09:32:15 -0400 2011: > >> > I think that you also need to update the constraint exclusion code >> > (get_relation_constraints() or nearby), otherwise the planner might >> > exclude a relation on the basis of a CHECK constraint that is not >> > currently VALID. >> >> Ouch, yeah, thanks for pointing that out. Fortunately the patch to fix >> this is quite simple. I don't have it handy right now but I'll post it >> soon. > > Here's the complete patch. > psql \h says (among other things) for ALTER TABLE """ ADD table_constraint ADD table_constraint_using_index ADD table_constraint [ NOT VALID ] """ ADD table_constraint appears twice and isn't true that all table_constraint accept the NOT VALID syntax... maybe we can accpet the syntax and send an unimplemented feature message for the other table_constraints? attached, is a script with the examples i have tried: EXAMPLE 1: constraint_exclusion when using NOT VALID check constraints... and it works well, except when the constraint has been validated, it keeps ignoring it (which means i won't benefit from constraint_exclusion) until i execute ANALYZE on the table or close connection EXAMPLE 2: if i have a DOMAIN with a NOT VALID check constraint, and i use it as the new type of a column it checks the constraint -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación /* example 1 */ DROP TABLE IF EXISTS padre CASCADE; create table padre(i serial primary key, d date); create table hija_2010 () inherits (padre); create table hija_2011 () inherits (padre); insert into hija_2010(d) values ('2011-08-15'::date); insert into hija_2011(d) values ('2011-09-15'::date); alter table hija_2010 add check (d between '2010-01-01'::date and '2010-12-31'::date) not valid; alter table hija_2011 add check (d between '2011-01-01'::date and '2011-12-31'::date) not valid; explain analyze select * from padre where d between '2011-08-01'::date and '2011-08-31'::date; create table hija_2009 (check (d between '2009-01-01'::date and '2009-12-31'::date)) inherits (padre); insert into hija_2009(d) values ('2009-06-13'); explain analyze select * from padre where d between '2011-08-01'::date and '2011-08-31'::date; explain analyze select * from padre where d between '2009-08-01'::date and '2009-08-31'::date; alter table hija_2011 VALIDATE CONSTRAINT hija_2011_d_check; explain analyze select * from padre where d between '2009-08-01'::date and '2009-08-31'::date; /* example 2 */ create domain mes as int; create table t_mes (m mes); insert into t_mes values(13); alter domain mes add check (value between 1 and 12) not valid; create table t_mes2 (m int); insert into t_mes2 values(13); alter table t_mes2 ALTER m type mes; ERROR: value for domain mes violates check constraint "mes_check" -- 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] time-delayed standbys
On Thu, Apr 21, 2011 at 12:18 PM, Robert Haas wrote: > On Wed, Apr 20, 2011 at 11:15 AM, Tom Lane wrote: >> Robert Haas writes: >>> I am a bit concerned about the reliability of this approach. If there >>> is some network lag, or some lag in processing from the master, we >>> could easily get the idea that there is time skew between the machines >>> when there really isn't. And our perception of the time skew could >>> easily bounce around from message to message, as the lag varies. I >>> think it would be tremendously ironic of the two machines were >>> actually synchronized to the microsecond, but by trying to be clever >>> about it we managed to make the lag-time accurate only to within >>> several seconds. >> >> Well, if walreceiver concludes that there is no more than a few seconds' >> difference between the clocks, it'd probably be OK to take the master >> timestamps at face value. The problem comes when the skew gets large >> (compared to the configured time delay, I guess). > > I suppose. Any bound on how much lag there can be before we start > applying to skew correction is going to be fairly arbitrary. When the replication connection is terminated, the standby tries to read WAL files from the archive. In this case, there is no walreceiver process, so how does the standby calculate the clock difference? > errmsg("parameter \"%s\" requires a temporal value", "recovery_time_delay"), We should s/"a temporal"/"an Integer"? After we run "pg_ctl promote", time-delayed replication should be disabled? Otherwise, failover might take very long time when we set recovery_time_delay to high value. http://forge.mysql.com/worklog/task.php?id=344 According to the above page, one purpose of time-delayed replication is to protect against user mistakes on master. But, when an user notices his wrong operation on master, what should he do next? The WAL records of his wrong operation might have already arrived at the standby, so neither "promote" nor "restart" doesn't cancel that wrong operation. Instead, probably he should shutdown the standby, investigate the timestamp of XID of the operation he'd like to cancel, set recovery_target_time and restart the standby. Something like this procedures should be documented? Or, we should implement new "promote" mode which finishes a recovery as soon as "promote" is requested (i.e., not replay all the available WAL records)? Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION 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] pg_upgrade using appname to lock out other users
On 2011-06-15 05:01, Bruce Momjian wrote: You might remember we added a postmaster/postgres -b switch to indicate binary upgrade mode. The attached patch prevents any client without an application_name of 'binary-upgrade' from connecting to the cluster while it is binary upgrade mode. This helps prevent unauthorized users from connecting during the upgrade. This will not help for clusters that do not have the -b flag, e.g. pre-9.1. Does this seem useful? Something for 9.1 or 9.2? This idea came from Andrew Dunstan via IRC during a pg_upgrade run by Stephen Frost when some clients accidentally connected. (Stephen reran pg_upgrade successfully.) Couldn't the -b flag also imply a very strict hba.conf configuration, that essentially only lets pg_upgrade in..? -- Jesper -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Avoid index rebuilds for no-rewrite ALTER TABLE ALTER TYPE
Previous version (at7-alt-index-opfamily.patch): http://archives.postgresql.org/message-id/20110113230124.ga18...@tornado.gateway.2wire.net Design: http://archives.postgresql.org/message-id/20110524104029.gb18...@tornado.gateway.2wire.net Patches committed in 2011CF1 allow ALTER TABLE ... ALTER ... SET DATA TYPE to avoid rewriting the table in certain cases. We still rebuild all indexes and recheck all foreign key constraints dependent on the changing column. This patch avoids the rebuild for some indexes, using the operator family facility to identify when that is safe. Changes since last version: * Instead of noting in comments that the operator family contracts do not make the guarantees I desire and opining that the code is fine in practice anyway, I amend those contracts in the documentation. See the aforementioned design explanation, but note that its third and fifth sentences are incorrect. I settled on requiring identical behavior among operators in the family after implicit casts (of any castmethod) or binary coercion casts (of any castcontext) on operands. I really only need the requirement for binary coercion casts, but implicit casts seem like a obvious extension. * Update for per-column collations. Index collations need to match. * We don't assign meaning to GIN or GiST operator family groupings. For access methods other than btree and hash, require an index rebuild unless the operator classes match exactly. Even if we could do otherwise, it would currently be academic; GIN "array_ops" is the only such family with more than one constituent operator class. The only practical case I've observed to be helped here is a conversion like varchar(2)[] -> varchar(4)[] with a GIN index on the column; operator class comparison covers that fine. * Remove support for avoiding foreign key constraint revalidation. This is orthogonal to the index case, though they share many principles. If we get this committed, I will submit a small patch for foreign keys at a later date. * Original patch was submitted in two forms for comment on the relative merits. The first form had tablecmds.c updating catalog entries pertaining to an old index until it matched how the index would be recreated with the new column data type. The second form actually dropped and recreated the index using the usual facilities, then reattached the old RelFileNode to the "new" index. Nobody commented, but I've come around to thinking the second approach is clearly better. Therefore, I'm submitting only that. * Change the division of labor between tablecmds.c and indexcmds.c. * Test cases removed per 2011CF1 discussion. * Sync with variable names changed since last submission. This patch is fully independent of the "Eliding no-op varchar length coercions" patch I've posted recently, though they do have synergy. Thanks, nm diff --git a/doc/src/sgml/xindex.sgml b/doc/src/sgml/xindex.sgml index a90b4e2..b6fe908 100644 *** a/doc/src/sgml/xindex.sgml --- b/doc/src/sgml/xindex.sgml *** *** 834,846 ALTER OPERATOR FAMILY integer_ops USING btree ADD In a B-tree operator family, all the operators in the family must sort compatibly, meaning that the transitive laws hold across all the data types !supported by the family: if A = B and B = C, then A = !C, and if A < B and B < C, then A < C. For each !operator in the family there must be a support function having the same !two input data types as the operator. It is recommended that a family be !complete, i.e., for each combination of data types, all operators are !included. Each operator class should include just the non-cross-type !operators and support function for its data type. --- 834,848 In a B-tree operator family, all the operators in the family must sort compatibly, meaning that the transitive laws hold across all the data types !supported by the family: if A = B and B = C, then A = C, !and if A < B and B < C, then A < C. Subjecting operands !to any number of implicit casts or binary coercion casts involving only !types represented in the family must not change the result other than to !require a similar cast thereon. For each operator in the family there must !be a support function having the same two input data types as the operator. !It is recommended that a family be complete, i.e., for each combination of !data types, all operators are included. Each operator class should include !just the non-cross-type operators and support function for its data type. *** *** 851,861 ALTER OPERATOR FAMILY integer_ops USING btree ADD by the family's equality operators, even when the values are of different types. This is usually difficult to accomplish when the types have different physical representations, but it can be done in some cases. !Notice that there is only one support function per data type,
Re: [HACKERS] procpid?
On Tue, Jun 14, 2011 at 11:04 PM, Greg Sabino Mullane wrote: >> For me, the litmus test is whether the change provides enough >> improvement that it outweighs the disruption when the user runs into >> it. > > For the procpid that started all of this, the clear answer is no. I'm > surprised people seriously considered making this change. It's a > historical accident: document and move on. I agree with you on this one... >> This is why I suggested a specific, useful, and commonly requested >> (to me at least) change to pg_stat_activity go along with this. > > +1. The procpid change is silly, but fixing the current_query field > would be very useful. You don't know how many times my fingers > have typed "WHERE current_query <> ''" ...but I'm not even excited about this. *Maybe* it's worth adding another column, but the problem with the existing system is *entirely* cosmetic. The string chosen here is unconfusable with an actual query, so we are talking here, as with the procpid -> pid proposal, ONLY about saving a few keystrokes when writing queries. That is a pretty thin justification for a compatibility break IMV. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] [WIP] cache estimates, cache access cost
On Tue, Jun 14, 2011 at 6:17 PM, Greg Smith wrote: > Who said anything about this being a commit candidate? The "WIP" in the > subject says it's not intended to be. The community asks people to submit > design ideas early so that ideas around them can be explored publicly. One > of the things that needs to be explored, and that could use some community > feedback, is exactly how this should be benchmarked in the first place. > This topic--planning based on cached percentage--keeps coming up, but > hasn't gone very far as an abstract discussion. Having a patch to test lets > it turn to a concrete one. > > Note that I already listed myself as the reviewer here, so it's not even > like this is asking explicitly for a community volunteer to help. Would you > like us to research this privately and then dump a giant patch that is > commit candidate quality on everyone six months from now, without anyone > else getting input to the process, or would you like the work to happen > here? I recommended Cédric not ever bother soliciting ideas early, because > I didn't want to get into this sort of debate. I avoid sending anything > here unless I already have a strong idea about the solution, because it's > hard to keep criticism at bay even with that. He was more optimistic about > working within the community contribution guidelines and decided to send > this over early instead. If you feel this is too rough to even discuss, > I'll mark it returned with feedback and we'll go develop this ourselves. My usual trope on this subject is that WIP patches tend to elicit helpful feedback if and only if the patch author is clear about what sort of feedback they are seeking. I'm interested in this topic, so, I'm willing to put some effort into it; but, as I've said before, I think this patch is coming from the wrong end, so in the absence of any specific guidance on what sort of input would be useful, that's the feedback you're getting. Feel free to clarify what would be more helpful. :-) Incidentally, I have done a bit of math around how to rejigger the costing formulas to take cached_page_cost and caching_percentage into account, which I think is the most interesting end place to start this work. If it's helpful, I can write it up in a more organized way and post that; it likely wouldn't be that much work to incorporate it into what Cedric has here already. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] pg_upgrade using appname to lock out other users
Tom Lane wrote: > Bruce Momjian writes: > > You might remember we added a postmaster/postgres -b switch to indicate > > binary upgrade mode. The attached patch prevents any client without an > > application_name of 'binary-upgrade' from connecting to the cluster > > while it is binary upgrade mode. This helps prevent unauthorized users > > from connecting during the upgrade. This will not help for clusters > > that do not have the -b flag, e.g. pre-9.1. > > > Does this seem useful? > > No ... that seems like a kluge. It's ugly and it's leaky. > > What we really ought to be doing here is fixing things so that > pg_upgrade does not need to have a running postmaster in either > installation, but works with some variant of standalone mode. > That would actually be *safe* against concurrent connections, > rather than only sorta kinda maybe safe. I keep replying to that suggestion by reminding people that pg_upgrade relies heavily on psql features, as does pg_dumpall, and recoding that in the backend will be error-prone. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] pg_upgrade using appname to lock out other users
Bruce Momjian writes: > You might remember we added a postmaster/postgres -b switch to indicate > binary upgrade mode. The attached patch prevents any client without an > application_name of 'binary-upgrade' from connecting to the cluster > while it is binary upgrade mode. This helps prevent unauthorized users > from connecting during the upgrade. This will not help for clusters > that do not have the -b flag, e.g. pre-9.1. > Does this seem useful? No ... that seems like a kluge. It's ugly and it's leaky. What we really ought to be doing here is fixing things so that pg_upgrade does not need to have a running postmaster in either installation, but works with some variant of standalone mode. That would actually be *safe* against concurrent connections, rather than only sorta kinda maybe safe. 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] planinstr, showing planner time on EXPLAIN
Yesterday on PGXN I just released the first version of planinstr, a plugin module to append planner time to EXPLAIN. I post this here since it is mostly for developers. http://www.pgxn.org/dist/planinstr/ db1=# load '$libdir/planinstr'; LOAD db1=# explain select * from pg_class; QUERY PLAN --- Seq Scan on pg_class (cost=0.00..141.87 rows=3287 width=194) Plan time: 0.119 ms db1=# explain analyze select count(*) from size_m; QUERY PLAN Aggregate (cost=26272.00..26272.01 rows=1 width=0) (actual time=51.938..51.938 rows=1 loops=1) -> Append (cost=0.00..23147.00 rows=125 width=0) (actual time=0.037..45.809 rows=2 loops=1) -> Seq Scan on size_m (cost=0.00..847.00 rows=2 width=0) (actual time=0.037..41.863 rows=2 loops=1) <.. snip ..> -> Seq Scan on myt1000 size_m (cost=0.00..22.30 rows=1230 width=0) (actual time=0.001..0.001 rows=0 loops=1) Total runtime: 75.217 ms Plan time: 61.353 ms (1005 rows) This may help to make the planner performance regression visible on some internal logic refactoring, etc. Hope this helps someone. Feel free to tell me if similar mechanism already exists, or you want more rich interfaces. Github is here: https://github.com/umitanuki/planinstr Also free to fork and send pull request! Regards, -- Hitoshi Harada -- 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] procpid?
-BEGIN PGP SIGNED MESSAGE- Hash: RIPEMD160 > For me, the litmus test is whether the change provides enough > improvement that it outweighs the disruption when the user runs into > it. For the procpid that started all of this, the clear answer is no. I'm surprised people seriously considered making this change. It's a historical accident: document and move on. And if we are going to talk about changing misnamed things, I've got a whole bunch of others I could throw at you (such as abbreviation rules: blks_read on the one extreme, and autovacuum_analyze_scale_factor on the other) :) > This is why I suggested a specific, useful, and commonly requested > (to me at least) change to pg_stat_activity go along with this. +1. The procpid change is silly, but fixing the current_query field would be very useful. You don't know how many times my fingers have typed "WHERE current_query <> ''" - -- Greg Sabino Mullane g...@turnstep.com End Point Corporation http://www.endpoint.com/ PGP Key: 0x14964AC8 201106142300 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -BEGIN PGP SIGNATURE- iEYEAREDAAYFAk34IRoACgkQvJuQZxSWSsi0dgCgi37mrLYbD6G3dS99GPbSFhHW EjYAniZNpRUXxYmhBHfb1k1LsMSoOHE7 =61nA -END PGP SIGNATURE- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] pg_upgrade using appname to lock out other users
You might remember we added a postmaster/postgres -b switch to indicate binary upgrade mode. The attached patch prevents any client without an application_name of 'binary-upgrade' from connecting to the cluster while it is binary upgrade mode. This helps prevent unauthorized users from connecting during the upgrade. This will not help for clusters that do not have the -b flag, e.g. pre-9.1. Does this seem useful? Something for 9.1 or 9.2? This idea came from Andrew Dunstan via IRC during a pg_upgrade run by Stephen Frost when some clients accidentally connected. (Stephen reran pg_upgrade successfully.) -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + diff --git a/contrib/pg_upgrade/pg_upgrade.c b/contrib/pg_upgrade/pg_upgrade.c new file mode 100644 index e329dc3..0b6fb61 *** a/contrib/pg_upgrade/pg_upgrade.c --- b/contrib/pg_upgrade/pg_upgrade.c *** setup(char *argv0, bool live_check) *** 171,176 --- 171,178 *last_dir_separator(exec_path) = '\0'; canonicalize_path(exec_path); os_info.exec_path = pg_strdup(exec_path); + + pg_putenv("PGAPPNAME", "binary-upgrade"); } diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c new file mode 100644 index 8347f52..f359af2 *** a/src/backend/utils/init/postinit.c --- b/src/backend/utils/init/postinit.c *** InitPostgres(const char *in_dbname, Oid *** 833,838 --- 833,848 if (MyProcPort != NULL) process_startup_options(MyProcPort, am_superuser); + /* + * Binary upgrades only allow the proper application name + */ + if (IsBinaryUpgrade && strcmp(application_name, "binary-upgrade") != 0) + { + ereport(FATAL, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("the application name must be \"binary-upgrade\" to connect in binary upgrade mode"))); + } + /* Process pg_db_role_setting options */ process_settings(MyDatabaseId, GetSessionUserId()); -- 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] possible connection leak in dblink?
On Wed, Jun 15, 2011 at 11:41, Fujii Masao wrote: > ISTM that the root problem is that dblink_send_query calls DBLINK_GET_CONN > though it doesn't accept the connection string as an argument. +1 for the fix. I have the same conclusion at the first glance. > Since the first > argument in dblink_send_query must be the connection name, dblink_send_query > should call DBLINK_GET_NAMED_CONN instead. The variable 'freeconn' is used > only when DBLINK_GET_CONN is called. So, if dblink_send_query uses > DBLINK_GET_NAMED_CONN instead, the variable 'freeconn' is no longer necessary. > > The similar problem exists in dblink_get_result and dblink_record_internal. > Attached patch fixes those problems. -- Itagaki Takahiro -- 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] Why polecat and colugos are failing to build back branches
On 06/14/2011 06:09 PM, Tom Lane wrote: Andrew Dunstan writes: On 06/14/2011 05:45 PM, Tom Lane wrote: I've committed patches that fix these issues on my own OS X machine, Well, OSX is just using our usual *nix paraphernalia, so if it's broken won't all such platforms probably be broken too? Yes, certainly. The reason I specified OS X in particular is that I only tested the darwin branch of Makefile.shlib. The -install_name switch that was the problem there is specific to OS X, but I wouldn't be surprised if some of the other branches have their own platform- specific issues. We have some more work to do. Vpath builds don't like spaces. :-( 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] possible connection leak in dblink?
On Wed, Jun 15, 2011 at 5:34 AM, Peter Eisentraut wrote: > With gcc 4.6, I get this warning: > > dblink.c: In function ‘dblink_send_query’: > dblink.c:620:7: warning: variable ‘freeconn’ set but not used > [-Wunused-but-set-variable] > > I don't know much about the internals of dblink, but judging from the > surrounding code, I guess that this fix is necessary: > > diff --git i/contrib/dblink/dblink.c w/contrib/dblink/dblink.c > index 19b98fb..e014c1a 100644 > --- i/contrib/dblink/dblink.c > +++ w/contrib/dblink/dblink.c > @@ -634,6 +634,10 @@ dblink_send_query(PG_FUNCTION_ARGS) > if (retval != 1) > elog(NOTICE, "%s", PQerrorMessage(conn)); > > + /* if needed, close the connection to the database and cleanup */ > + if (freeconn) > + PQfinish(conn); > + > PG_RETURN_INT32(retval); > } > > Otherwise the connection might not get freed. Could someone verify > that? ISTM that the root problem is that dblink_send_query calls DBLINK_GET_CONN though it doesn't accept the connection string as an argument. Since the first argument in dblink_send_query must be the connection name, dblink_send_query should call DBLINK_GET_NAMED_CONN instead. The variable 'freeconn' is used only when DBLINK_GET_CONN is called. So, if dblink_send_query uses DBLINK_GET_NAMED_CONN instead, the variable 'freeconn' is no longer necessary. The similar problem exists in dblink_get_result and dblink_record_internal. Attached patch fixes those problems. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center *** a/contrib/dblink/dblink.c --- b/contrib/dblink/dblink.c *** *** 613,628 Datum dblink_send_query(PG_FUNCTION_ARGS) { PGconn *conn = NULL; - char *connstr = NULL; char *sql = NULL; remoteConn *rconn = NULL; - char *msg; - bool freeconn = false; int retval; if (PG_NARGS() == 2) { ! DBLINK_GET_CONN; sql = text_to_cstring(PG_GETARG_TEXT_PP(1)); } else --- 613,625 dblink_send_query(PG_FUNCTION_ARGS) { PGconn *conn = NULL; char *sql = NULL; remoteConn *rconn = NULL; int retval; if (PG_NARGS() == 2) { ! DBLINK_GET_NAMED_CONN; sql = text_to_cstring(PG_GETARG_TEXT_PP(1)); } else *** *** 711,723 dblink_record_internal(FunctionCallInfo fcinfo, bool is_async) if (PG_NARGS() == 2) { /* text,bool */ ! DBLINK_GET_CONN; fail = PG_GETARG_BOOL(1); } else if (PG_NARGS() == 1) { /* text */ ! DBLINK_GET_CONN; } else /* shouldn't happen */ --- 708,720 if (PG_NARGS() == 2) { /* text,bool */ ! DBLINK_GET_NAMED_CONN; fail = PG_GETARG_BOOL(1); } else if (PG_NARGS() == 1) { /* text */ ! DBLINK_GET_NAMED_CONN; } else /* shouldn't happen */ -- 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] procpid?
On Wed, Jun 15, 2011 at 2:50 AM, Bruce Momjian wrote: > Agreed on moving '' and ' in transaction' into separate > fields. If I had thought of it I would have done it that way years ago. > (At least I think it was me.) Using angle brackets to put magic values > in that field was clearly wrong. I think of these as just placeholders in the SQL text field for cases where there's no SQL text available. But they do clearly indicate a need for columns with this information. For what it's worth Oracle provides a whole list of states the transaction can be in, it can be waiting for client traffic, waiting on i/o, waiting on a lock, etc. Separately whether the session is in a transaction might need to become slightly richer than a boolean now that we have snapshot management. You can be in a transaction but not have any snapshots or be in the traditional state where you have at least one snapshot. And If we do autonomous transactions the field might have be much much richer again. -- greg -- 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] psql describe.c cleanup
On Tue, Jun 14, 2011 at 3:25 PM, Alvaro Herrera wrote: > pgindent moves strings back to the left when it thinks they fit within > 80 columns. Yes, that seems pretty screwy. I am losing track of the ways in which pgindent has managed to mangle our source code :-/ 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] psql describe.c cleanup
On Tue, Jun 14, 2011 at 12:15 PM, Merlin Moncure wrote: > I did a quick review and test of your patch. It didn't quite apply > cleanly due to recent non-related describe.c changes -- updated patch > attached. Thanks for looking at this. Your updated patch looks good to me. > First, I'll give you a thumbs up on the original inspiration for the > patch. The output should be standardized, and I see no reason not to > append a semicolon on usability basis. Beyond that, the changes are > mostly cosmetic and I can't see how it will break things outside of > terminating a query early by accident (I didn't see any). Yeah, I really didn't want to break any queries, so I did my best to test every query I changed. > What I do wonder though is if the ; appending should really be > happening in printQuery() instead of in each query -- the idea being > that formatting for external consumption should be happening in one > place. Maybe that's over-thinking it though. That's a fair point, and hacking up printQuery() would indeed solve my original gripe about copy-pasting queries out of psql -E. But more generally, I think that standardizing the style of the code is a Good Thing, particularly where style issues impact usability (like here). I would actually like to see a movement towards having all these queries use whitespace/formatting consistently. For instance, when I do a \d sometable I see some queries with lines bleeding out maybe 200 columns, some wrapped at 80 columns, etc. This sort of style issue is not something that a simple kludge in printQuery() could solve (and even if we put in a sophisticated formatting tool inside printQuery(), we'd still be left with an ugly describe.c). For the record, I find that having SQL queries consistently formatted makes them much more readable, allowing a human to roughly "parse" a query on sight once you're used to the formatting. So I wouldn't be opposed to putting the kludge in printQuery(), but I would like to see us standardize the queries regardless. (And in that case, I wouldn't mind if we dropped all the semicolons in describe.c, just that we're consistent.) 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] procpid?
Greg Smith wrote: > On 06/14/2011 06:00 PM, Tom Lane wrote: > > As far as Greg's proposal is concerned, I don't see how a proposed > > addition of two columns would justify renaming an existing column. > > Additions should not break any sanely-implemented application, but > > renamings certainly will. > > > > It's not so much justification as something that makes the inevitable > complaints easier to stomach, in terms of not leaving a really bad taste > in the user's mouth. My thinking is that if we're going to mess with > pg_stat_activity in a way that breaks something, I'd like to see it > completely refactored for better usability in the process. If code > breaks and the resulting investigation by the admin highlights something > new, that offsets some of the bad user experience resulting from the > breakage. > > Also, I haven't fully worked whether it makes sense to really change > what current_query means if the idle/transaction component of it gets > moved to another column. Would it be better to set current_query to > null if you are idle, rather than the way it's currently overloaded with > text in that case? I don't like the way this view works at all, but I'm > not sure the best way to change it. Just changing procpid wouldn't be > the only thing on the list though. Agreed on moving '' and ' in transaction' into separate fields. If I had thought of it I would have done it that way years ago. (At least I think it was me.) Using angle brackets to put magic values in that field was clearly wrong. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] Why polecat and colugos are failing to build back branches
On Jun 14, 2011, at 3:45 PM, Tom Lane wrote: > I've committed patches that fix these issues on my own OS X machine, > though it remains to be seen whether polecat and colugos will like > them. It turns out that whatever setup Robert has got with > '/Volumes/High Usage/' is really *not* fully exercising the system > as far as space-containing paths go, because I found bugs in all '/Volumes/High Usage' is a "new" external drive as I put in a SSD as the main drive. Rather than re-do all my configs, I created a symbolic link from /usr/local/src/pg_buildfarm... to the external disk. > active branches when I tried to do builds and installs underneath > '/Users/tgl/foo bar/'. Is it worth setting up a buildfarm critter > to exercise the case on a long-term basis? If we don't, I think > we can expect that it'll break regularly. I'd be happy to do that on my iMac, which isn't a build farm member yet, although I had plans to add it. I can easily do a single tester with spaces in the path, or multiple testers with and without spaces. I'll work on the build farm config setup at least for the space scenario. Cheers, Rob -- 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] creating CHECK constraints as NOT VALID
On Tue, Jun 14, 2011 at 4:41 PM, Jaime Casanova wrote: > On Tue, Jun 14, 2011 at 4:14 PM, Alvaro Herrera > wrote: >> Excerpts from Alvaro Herrera's message of lun jun 13 18:08:12 -0400 2011: >>> Excerpts from Dean Rasheed's message of sáb jun 11 09:32:15 -0400 2011: >> >>> > I think that you also need to update the constraint exclusion code >>> > (get_relation_constraints() or nearby), otherwise the planner might >>> > exclude a relation on the basis of a CHECK constraint that is not >>> > currently VALID. >>> >>> Ouch, yeah, thanks for pointing that out. Fortunately the patch to fix >>> this is quite simple. I don't have it handy right now but I'll post it >>> soon. >> >> Here's the complete patch. >> > > this doesn't apply > oops! sorry for the noise... this was gmail problem... the patch was expanded as text, instead of an attachments and when i copy/paste it it should have moved something... copy'ing from archives instead was good -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación -- 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: patch review : Add ability to constrain backend temporary file space
On 15/06/11 02:52, Cédric Villemain wrote: 2011/6/3 Mark Kirkwood: Corrected v4 patch with the test files, for completeness. Note that discussion has moved on and there will be a v5 :-) Mark, can you submit your updated patch ? Thanks for the reminder! Here is v5. The changes are: - guc is called temp_file_limit, which seems like the best choice to date :-) - removed code to do with truncating files, as after testing I agree with you that temp work files don't seem to get truncated. I have not done anything about the business on units - so we are in KB still - there is no MB unit avaiable in the code as yet - I'm not sure we need one at this point, as most folk who use this feature will find 4096GB a big enough *per backend* limit. Obviously it might be a good investment to plan to have MB, and GB as possible guc units too. Maybe this could be a separate piece of work since any *other* resource limiters we add might find it convenient to have these available? Cheers Mark temp-files-v5.patch.gz Description: GNU Zip compressed data -- 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] procpid?
On 06/14/2011 06:00 PM, Tom Lane wrote: As far as Greg's proposal is concerned, I don't see how a proposed addition of two columns would justify renaming an existing column. Additions should not break any sanely-implemented application, but renamings certainly will. It's not so much justification as something that makes the inevitable complaints easier to stomach, in terms of not leaving a really bad taste in the user's mouth. My thinking is that if we're going to mess with pg_stat_activity in a way that breaks something, I'd like to see it completely refactored for better usability in the process. If code breaks and the resulting investigation by the admin highlights something new, that offsets some of the bad user experience resulting from the breakage. Also, I haven't fully worked whether it makes sense to really change what current_query means if the idle/transaction component of it gets moved to another column. Would it be better to set current_query to null if you are idle, rather than the way it's currently overloaded with text in that case? I don't like the way this view works at all, but I'm not sure the best way to change it. Just changing procpid wouldn't be the only thing on the list though. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us -- 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] [WIP] cache estimates, cache access cost
On 06/14/2011 07:08 PM, Tom Lane wrote: I concur with Robert's desire to not push experimental code into the main repository, but we need to have *some* way of working with it. Maybe a separate repo where experimental branches could hang out would be helpful? Well, this one is sitting around in branches at both git.postgresql.org and github so far, both being updated periodically. Maybe there's some value around an official experimental repository too, but I thought that was the idea of individual people having their own directories on git.postgres.org. Do we need something fancier than that? It would be nice, but seems little return on investment to improve that, relative to what you can do easily enough now. The idea David Fetter has been advocating of having a "bit rot" farm to help detect when the experimental branches drift too far out of date tries to make that concept really formal. I like that idea, too, but find it hard to marshal enough resources to do something about it. The current status quo isn't that terrible; noticing bit rot when it's relevant isn't that hard to do. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us -- 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] [WIP] cache estimates, cache access cost
Greg Smith writes: > On 06/14/2011 01:16 PM, Robert Haas wrote: >> But there's no reason that code (which may or may not eventually prove >> useful) has to be incorporated into the main tree. We don't commit >> code so people can go benchmark it; we ask for the benchmarking to be >> done first, and then if the results are favorable, we commit the code. > Who said anything about this being a commit candidate? The "WIP" in the > subject says it's not intended to be. The community asks people to > submit design ideas early so that ideas around them can be explored > publicly. One of the things that needs to be explored, and that could > use some community feedback, is exactly how this should be benchmarked > in the first place. This topic--planning based on cached > percentage--keeps coming up, but hasn't gone very far as an abstract > discussion. Having a patch to test lets it turn to a concrete one. Yeah, it *can't* go very far as an abstract discussion ... we need some realistic testing to decide whether this is a good idea, and you can't get that without code. I think the real underlying issue here is that we have this CommitFest process that is focused on getting committable or nearly-committable code into the tree, and it just doesn't fit well for experimental code. I concur with Robert's desire to not push experimental code into the main repository, but we need to have *some* way of working with it. Maybe a separate repo where experimental branches could hang out would be helpful? (Another way of phrasing my point is that "WIP" is not conveying the true status of this patch. Maybe "Experimental" would be an appropriate label.) 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] [WIP] cache estimates, cache access cost
Greg Smith wrote: > On 06/14/2011 01:16 PM, Robert Haas wrote: > > But there's no reason that code (which may or may not eventually prove > > useful) has to be incorporated into the main tree. We don't commit > > code so people can go benchmark it; we ask for the benchmarking to be > > done first, and then if the results are favorable, we commit the code. > > > > Who said anything about this being a commit candidate? The "WIP" in the > subject says it's not intended to be. The community asks people to > submit design ideas early so that ideas around them can be explored > publicly. One of the things that needs to be explored, and that could > use some community feedback, is exactly how this should be benchmarked > in the first place. This topic--planning based on cached > percentage--keeps coming up, but hasn't gone very far as an abstract > discussion. Having a patch to test lets it turn to a concrete one. > > Note that I already listed myself as the reviewer here, so it's not > even like this is asking explicitly for a community volunteer to help. > Would you like us to research this privately and then dump a giant patch > that is commit candidate quality on everyone six months from now, > without anyone else getting input to the process, or would you like the > work to happen here? I recommended C?dric not ever bother soliciting > ideas early, because I didn't want to get into this sort of debate. I > avoid sending anything here unless I already have a strong idea about > the solution, because it's hard to keep criticism at bay even with > that. He was more optimistic about working within the community > contribution guidelines and decided to send this over early instead. If > you feel this is too rough to even discuss, I'll mark it returned with > feedback and we'll go develop this ourselves. I would like to see us continue researching in this direction. I think perhaps the background writer would be ideal for collecting this information because it scans the buffer cache already, and frequently. (Yes, I know it can't access databases.) I think random_page_cost is a dead-end --- it will never be possible for it to produce the right value for us. Its value is tied up in caching, e.g. the default 4 is not the right value for a physical drive (it should be much higher), but kernel and shared buffer caching require it to be a hybrid number that isn't really realistic. And once we have caching in that number, it is not going to be even caching for all tables, obviously. Hence, there is no way for random_page_cost to be improved and we have to start thinking about alternatives. Basically, random_page_cost is a terrible setting and we have to admit that and move forward. I realize the concerns about unstable plans, and we might need to give users the option of stable plans with a fixed random_page_cost, but at this point we don't even have enough data to know we need that. What we do know is that random_page_cost is inadequate. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] [WIP] cache estimates, cache access cost
On 06/14/2011 01:16 PM, Robert Haas wrote: But there's no reason that code (which may or may not eventually prove useful) has to be incorporated into the main tree. We don't commit code so people can go benchmark it; we ask for the benchmarking to be done first, and then if the results are favorable, we commit the code. Who said anything about this being a commit candidate? The "WIP" in the subject says it's not intended to be. The community asks people to submit design ideas early so that ideas around them can be explored publicly. One of the things that needs to be explored, and that could use some community feedback, is exactly how this should be benchmarked in the first place. This topic--planning based on cached percentage--keeps coming up, but hasn't gone very far as an abstract discussion. Having a patch to test lets it turn to a concrete one. Note that I already listed myself as the reviewer here, so it's not even like this is asking explicitly for a community volunteer to help. Would you like us to research this privately and then dump a giant patch that is commit candidate quality on everyone six months from now, without anyone else getting input to the process, or would you like the work to happen here? I recommended Cédric not ever bother soliciting ideas early, because I didn't want to get into this sort of debate. I avoid sending anything here unless I already have a strong idea about the solution, because it's hard to keep criticism at bay even with that. He was more optimistic about working within the community contribution guidelines and decided to send this over early instead. If you feel this is too rough to even discuss, I'll mark it returned with feedback and we'll go develop this ourselves. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us -- 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] Why polecat and colugos are failing to build back branches
Andrew Dunstan writes: > On 06/14/2011 05:45 PM, Tom Lane wrote: >> I've committed patches that fix these issues on my own OS X machine, > Well, OSX is just using our usual *nix paraphernalia, so if it's broken > won't all such platforms probably be broken too? Yes, certainly. The reason I specified OS X in particular is that I only tested the darwin branch of Makefile.shlib. The -install_name switch that was the problem there is specific to OS X, but I wouldn't be surprised if some of the other branches have their own platform- specific issues. > I'd actually bet a modest amount MSVC is less broken Very possibly, but unless it's being tested it's no sure bet. 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] [WIP] cache estimates, cache access cost
Alvaro Herrera writes: > Excerpts from Cédric Villemain's message of mar jun 14 10:29:36 -0400 2011: >> 0001-Add-reloscache-column-to-pg_class.patch > Hmm, do you really need this to be a new column? Would it work to have > it be a reloption? If it's to be updated in the same way as ANALYZE updates reltuples and relpages (ie, an in-place non-transactional update), I think it'll have to be a real column. 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] Why polecat and colugos are failing to build back branches
On 06/14/2011 05:45 PM, Tom Lane wrote: Andrew Dunstan writes: On 06/13/2011 08:05 PM, Tom Lane wrote: I looked into $SUBJECT. There appear to be two distinct issues: ... I think we can be a bit more liberal about build patches than things that can affect the runtime behaviour. So +1 for fixing both of these. I've committed patches that fix these issues on my own OS X machine, though it remains to be seen whether polecat and colugos will like them. It turns out that whatever setup Robert has got with '/Volumes/High Usage/' is really *not* fully exercising the system as far as space-containing paths go, because I found bugs in all active branches when I tried to do builds and installs underneath '/Users/tgl/foo bar/'. Is it worth setting up a buildfarm critter to exercise the case on a long-term basis? If we don't, I think we can expect that it'll break regularly. (I wouldn't care to bet that the MSVC case works yet, either.) Well, OSX is just using our usual *nix paraphernalia, so if it's broken won't all such platforms probably be broken too? I'd actually bet a modest amount MSVC is less broken because it uses perl modules like File::Copy to do most of its work and so will be less prone to shell parsing breakage. But yes, we should check regularly. 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] procpid?
Peter Eisentraut writes: > On tis, 2011-06-14 at 13:50 -0400, Robert Haas wrote: >> There are real problems with the idea of having one release where we >> break everything that we want to break - mostly from a process >> standpoint. We aren't always good at being organized and disciplined, >> and coming up with a multi-year plan to break everything all at once >> in 2014 for release in 2015 may be difficult, because it requires a >> consensus on release management to hold together for years, and >> sometimes we can't even manage "days". > I have had this fantasy of a break-everything release for a long time as > well, but frankly, experience from other projects such as Python 3, Perl > 6, KDE 4, Samba 4, add-yours-here, indicates that such things might not > work out so well. > OK, some of those were rewrites as well as interface changes, but the > effect visible to the end user is mostly the same. Good point. I think the case that has actually been discussed is the idea of saving up binary-compatibility breaks (on-disk format changes). That seems sensible. It doesn't create a bigger problem for users, since a dump/reload is a dump/reload no matter how many individual format changes happened underneath. But we should be wary of applying that approach to application-visible incompatibilities. As far as Greg's proposal is concerned, I don't see how a proposed addition of two columns would justify renaming an existing column. Additions should not break any sanely-implemented application, but renamings certainly will. 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] procpid?
Peter Eisentraut wrote: > On tis, 2011-06-14 at 13:50 -0400, Robert Haas wrote: > > There are real problems with the idea of having one release where we > > break everything that we want to break - mostly from a process > > standpoint. We aren't always good at being organized and disciplined, > > and coming up with a multi-year plan to break everything all at once > > in 2014 for release in 2015 may be difficult, because it requires a > > consensus on release management to hold together for years, and > > sometimes we can't even manage "days". > > I have had this fantasy of a break-everything release for a long time as > well, but frankly, experience from other projects such as Python 3, Perl > 6, KDE 4, Samba 4, add-yours-here, indicates that such things might not > work out so well. > > OK, some of those were rewrites as well as interface changes, but the > effect visible to the end user is mostly the same. Funny you mentioned Perl 6 because I just blogged about that: http://momjian.us/main/blogs/pgblog/2011.html#June_14_2011 -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] Why polecat and colugos are failing to build back branches
Andrew Dunstan writes: > On 06/13/2011 08:05 PM, Tom Lane wrote: >> I looked into $SUBJECT. There appear to be two distinct issues: >> ... > I think we can be a bit more liberal about build patches than things > that can affect the runtime behaviour. > So +1 for fixing both of these. I've committed patches that fix these issues on my own OS X machine, though it remains to be seen whether polecat and colugos will like them. It turns out that whatever setup Robert has got with '/Volumes/High Usage/' is really *not* fully exercising the system as far as space-containing paths go, because I found bugs in all active branches when I tried to do builds and installs underneath '/Users/tgl/foo bar/'. Is it worth setting up a buildfarm critter to exercise the case on a long-term basis? If we don't, I think we can expect that it'll break regularly. (I wouldn't care to bet that the MSVC case works yet, either.) 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] creating CHECK constraints as NOT VALID
On Tue, Jun 14, 2011 at 4:14 PM, Alvaro Herrera wrote: > Excerpts from Alvaro Herrera's message of lun jun 13 18:08:12 -0400 2011: >> Excerpts from Dean Rasheed's message of sáb jun 11 09:32:15 -0400 2011: > >> > I think that you also need to update the constraint exclusion code >> > (get_relation_constraints() or nearby), otherwise the planner might >> > exclude a relation on the basis of a CHECK constraint that is not >> > currently VALID. >> >> Ouch, yeah, thanks for pointing that out. Fortunately the patch to fix >> this is quite simple. I don't have it handy right now but I'll post it >> soon. > > Here's the complete patch. > this doesn't apply -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación -- 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] [WIP] cache estimates, cache access cost
Excerpts from Cédric Villemain's message of mar jun 14 17:10:20 -0400 2011: > If we can have ALTER TABLE running on heavy workload, why not. > I am bit scared by the effect of such reloption, it focus on HINT > oriented strategy when I would like to allow a dynamic strategy from > the server. This work is not done and may not work, so a reloption is > good at least as a backup (and is more in the idea suggested by Tom > and others) Hmm, sounds like yet another use case for pg_class_nt. Why do these keep popping up? -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] creating CHECK constraints as NOT VALID
Excerpts from Alvaro Herrera's message of lun jun 13 18:08:12 -0400 2011: > Excerpts from Dean Rasheed's message of sáb jun 11 09:32:15 -0400 2011: > > I think that you also need to update the constraint exclusion code > > (get_relation_constraints() or nearby), otherwise the planner might > > exclude a relation on the basis of a CHECK constraint that is not > > currently VALID. > > Ouch, yeah, thanks for pointing that out. Fortunately the patch to fix > this is quite simple. I don't have it handy right now but I'll post it > soon. Here's the complete patch. *** a/doc/src/sgml/catalogs.sgml --- b/doc/src/sgml/catalogs.sgml *** *** 1898,1904 convalidated bool ! Has the constraint been validated? Can only be false for foreign keys --- 1898,1904 convalidated bool ! Has the constraint been validated? Can only be false for foreign keys and CHECK constraints *** a/doc/src/sgml/ref/alter_domain.sgml --- b/doc/src/sgml/ref/alter_domain.sgml *** *** 28,37 ALTER DOMAIN name ALTER DOMAIN name { SET | DROP } NOT NULL ALTER DOMAIN name ! ADD domain_constraint ALTER DOMAIN name DROP CONSTRAINT constraint_name [ RESTRICT | CASCADE ] ALTER DOMAIN name OWNER TO new_owner ALTER DOMAIN name SET SCHEMA new_schema --- 28,39 ALTER DOMAIN name { SET | DROP } NOT NULL ALTER DOMAIN name ! ADD domain_constraint [ NOT VALID ] ALTER DOMAIN name DROP CONSTRAINT constraint_name [ RESTRICT | CASCADE ] ALTER DOMAIN name + VALIDATE CONSTRAINT constraint_name + ALTER DOMAIN name OWNER TO new_owner ALTER DOMAIN name SET SCHEMA new_schema *** *** 70,82 ALTER DOMAIN name ! ADD domain_constraint This form adds a new constraint to a domain using the same syntax as . ! This will only succeed if all columns using the domain satisfy the ! new constraint. --- 72,88 ! ADD domain_constraint [ NOT VALID ] This form adds a new constraint to a domain using the same syntax as . ! If NOT VALID is not specified, ! the command will only succeed if all columns using the ! domain satisfy the new constraint. ! The constraint is going to be enforced on new data inserted into columns ! using the domain in all cases, regardless of NOT VALID. ! NOT VALID is only accepted for CHECK constraints. *** *** 91,96 ALTER DOMAIN name --- 97,113 + VALIDATE CONSTRAINT + + + This form validates a constraint previously added, that is, verify that + all data in columns using the domain satisfy the specified constraint. + + + + + + OWNER *** *** 156,161 ALTER DOMAIN name --- 173,188 + NOT VALID + + + Do not verify existing column data for constraint validity. + + + + + + CASCADE *** *** 251,257 ALTER DOMAIN zipcode SET SCHEMA customers; ALTER DOMAIN conforms to the SQL standard, !except for the OWNER and SET SCHEMA variants, which are PostgreSQL extensions. --- 278,286 ALTER DOMAIN conforms to the SQL standard, !except for the OWNER, SET SCHEMA and !VALIDATE CONSTRAINT variants, !as well as the NOT VALID clause of the ADD CONSTRAINT variant, which are PostgreSQL extensions. *** a/doc/src/sgml/ref/alter_table.sgml --- b/doc/src/sgml/ref/alter_table.sgml *** *** 240,246 ALTER TABLE name This form adds a new constraint to a table using the same syntax as ! . Newly added foreign key constraints can also be defined as NOT VALID to avoid the potentially lengthy initial check that must otherwise be performed. Constraint checks are skipped at create table time, so --- 240,246 This form adds a new constraint to a table using the same syntax as ! . Newly added foreign key and CHECK constraints can also be defined as NOT VALID to avoid the potentially lengthy initial check that must otherwise be performed. Constraint checks are skipped at create table time, so *** *** 253,259 ALTER TABLE name VALIDATE CONSTRAINT ! This form validates a foreign key constraint that was previously created as NOT VALID. Constraints already marked valid do not cause an error response. --- 253,259 VALIDATE CONSTRAINT ! This form validates a foreign key or CHECK constraint that was previously created as NO
Re: [HACKERS] [WIP] cache estimates, cache access cost
2011/6/14 Alvaro Herrera : > Excerpts from Cédric Villemain's message of mar jun 14 10:29:36 -0400 2011: > >> Attached are updated patches without the plugin itself. I've also >> added the cache_page_cost GUC, this one is not per tablespace, like >> others page_cost. >> >> There are 6 patches: >> >> 0001-Add-reloscache-column-to-pg_class.patch > > Hmm, do you really need this to be a new column? Would it work to have > it be a reloption? If we can have ALTER TABLE running on heavy workload, why not. I am bit scared by the effect of such reloption, it focus on HINT oriented strategy when I would like to allow a dynamic strategy from the server. This work is not done and may not work, so a reloption is good at least as a backup (and is more in the idea suggested by Tom and others) > > -- > Álvaro Herrera > The PostgreSQL Company - Command Prompt, Inc. > PostgreSQL Replication, Consulting, Custom Development, 24x7 support > -- Cédric Villemain 2ndQuadrant http://2ndQuadrant.fr/ PostgreSQL : Expertise, Formation et Support -- 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] procpid?
On tis, 2011-06-14 at 13:50 -0400, Robert Haas wrote: > There are real problems with the idea of having one release where we > break everything that we want to break - mostly from a process > standpoint. We aren't always good at being organized and disciplined, > and coming up with a multi-year plan to break everything all at once > in 2014 for release in 2015 may be difficult, because it requires a > consensus on release management to hold together for years, and > sometimes we can't even manage "days". I have had this fantasy of a break-everything release for a long time as well, but frankly, experience from other projects such as Python 3, Perl 6, KDE 4, Samba 4, add-yours-here, indicates that such things might not work out so well. OK, some of those were rewrites as well as interface changes, but the effect visible to the end user is mostly the same. -- 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] Polecat "quit unexpectdly"
Robert Creager wrote: > You believe it was related to the flurry of errors that popped up > then. I haven't looked at all the error in the "flurry". I think your particular report is consistent with being caused by this commit: http://git.postgresql.org/gitweb?p=postgresql.git;a=commit;h=b81831acbc671445061ed41a55fb1cc21d8e2979 and solved by this one: http://git.postgresql.org/gitweb?p=postgresql.git;a=commit;h=85ea93384ae21ff59f5e5b292884a86f9c10b852 I suspect that other builds based on checkouts in this window of time would probably show similar errors. -Kevin -- 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] One-Shot Plans
Simon Riggs writes: > Currently, the planner and executor are mostly independent of each > other: the planner doesn't really know when the plan will be executed, > and the executor doesn't know how recently the plan was made. > We can work out the various paths through the traffic cop to see when > a plan will be a "one-shot" - planned and then executed immediately, > then discarded. I have already got plans for a significantly more sophisticated approach to this. > In those cases we can take advantage of better optimisations. Most > interestingly, we can evaluate stable functions at plan time, to allow > us to handle partitioning and partial indexes better. I don't believe that's correct in detail. 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] possible connection leak in dblink?
With gcc 4.6, I get this warning: dblink.c: In function ‘dblink_send_query’: dblink.c:620:7: warning: variable ‘freeconn’ set but not used [-Wunused-but-set-variable] I don't know much about the internals of dblink, but judging from the surrounding code, I guess that this fix is necessary: diff --git i/contrib/dblink/dblink.c w/contrib/dblink/dblink.c index 19b98fb..e014c1a 100644 --- i/contrib/dblink/dblink.c +++ w/contrib/dblink/dblink.c @@ -634,6 +634,10 @@ dblink_send_query(PG_FUNCTION_ARGS) if (retval != 1) elog(NOTICE, "%s", PQerrorMessage(conn)); + /* if needed, close the connection to the database and cleanup */ + if (freeconn) + PQfinish(conn); + PG_RETURN_INT32(retval); } Otherwise the connection might not get freed. Could someone verify that? -- 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] [WIP] cache estimates, cache access cost
Excerpts from Cédric Villemain's message of mar jun 14 10:29:36 -0400 2011: > Attached are updated patches without the plugin itself. I've also > added the cache_page_cost GUC, this one is not per tablespace, like > others page_cost. > > There are 6 patches: > > 0001-Add-reloscache-column-to-pg_class.patch Hmm, do you really need this to be a new column? Would it work to have it be a reloption? -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] procpid?
Excerpts from Bruce Momjian's message of mar jun 14 12:59:15 -0400 2011: > Well, someone doing SELECT *, which is probably 90% of the users, are > going to be pretty confused by duplicate columns, asking, "What is the > difference"? For those people this would make things worse than they > are now. > > I would say 90% of users are doing SELECT *, and 10% are joining to > other tables or displaying specific columns. We want to help that 10% > without making that 90% confused. I think if you had column synonyms, you would get only a single one when doing "select *". The other name would still be accepted in a query that explicitely asked for it. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] procpid?
On 06/14/2011 02:20 PM, Kevin Grittner wrote: Just on our Wiki pages we have some queries available for copy/paste which would need multiple versions while both column names were in supported versions of the software: http://wiki.postgresql.org/wiki/Lock_dependency_information http://wiki.postgresql.org/wiki/Lock_Monitoring http://wiki.postgresql.org/wiki/Backend_killer_function ...and most of these would actually be simplified if they could just JOIN on pid instead of needing this common idiom: join pg_catalog.pg_stat_activity ka on kl.pid = ka.procpid Yes, there are a lot of these floating around. I'd bet that in an hour of research I could find 95% of them though, and make sure they were all updated in advance of the release. (I already did most of this search as part of stealing every good idea I could find in this area for my book) I think that's consistent with the "save up our breaking changes to do them all at once" approach. I don't actually buy into this whole idea at all. We already have this big wall at 8.3 because changes made in that release are too big for people on the earlier side to upgrade past. I'd rather see a series of smaller changes in each release, even if they are disruptive, so that no one version turns into a frustrating hurdle seen as impossible to clear. This adjustment is a perfect candidate for putting into 9.2 to me, because I'd rather reduce max(breakage) across releases than intentionally aim at increasing it but bundling them into larger clumps. For me, the litmus test is whether the change provides enough improvement that it outweighs the disruption when the user runs into it. This is why I suggested a specific, useful, and commonly requested (to me at least) change to pg_stat_activity go along with this. If people discover their existing pg_stat_activity tools break, presumably they're going to look at the view again to see what changed. When they do that, I don't want the reaction to be "why was this random change made?" I want it to be "look, there are useful new fields in here; let me see if I can use them too here". That's how you make people tolerate disruption in upgrades. If they see a clear improvement in the same spot when forced to fix around it, the experience is much more pleasant if they get something new out of it too. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us -- 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] Polecat "quit unexpectdly"
Robert Creager wrote: > Stack trace, nothing else. > 3 postgres 0x00010005cafa > multixact_twophase_postcommit + 74 (multixact.c:1367) > 4 postgres 0x00010005deab > ProcessRecords + 91 (twophase.c:1407) > 5 postgres 0x00010005f78a > FinishPreparedTransaction + 1610 (twophase.c:1368) If this was a checkout from more than about 7 hours ago and less than about 10 hours ago, please get a fresh copy of the source and try again. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: pgbench cpu overhead (was Re: [HACKERS] lazy vxid locks, v1)
On 06/14/2011 02:27 AM, Jeff Janes wrote: > On Mon, Jun 13, 2011 at 7:03 AM, Stefan Kaltenbrunner > wrote: > ... >> >> >> so it seems that sysbench is actually significantly less overhead than >> pgbench and the lower throughput at the higher conncurency seems to be >> cause by sysbench being able to stress the backend even more than >> pgbench can. > > Hi Stefan, > > pgbench sends each query (per connection) and waits for the reply > before sending another. > > Do we know whether sysbench does that, or if it just stuffs the > kernel's IPC buffer full of queries without synchronously waiting for > individual replies? > > I can't get sysbench to "make" for me, or I'd strace in single client > mode and see what kind of messages are going back and forth. yeah sysbench compiled from a release tarball needs some autoconf/makefile hackery to get running on a modern system - but I can provide you with the data you are interested in if you tell me exactly what you are looking for... Stefan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Polecat "quit unexpectdly"
Stack trace, nothing else. Now that the horse has left the barn, I changed to keep error builds...Process: postgres [17457]Path: /Volumes/High Usage/usr/local/src/build-farm-4.4/builds/HEAD/pgsql.6849/src/test/regress/tmp_check/install/usr/local/src/build-farm-4.4/builds/HEAD/inst/bin/postgresIdentifier: postgresVersion: ??? (???)Code Type: X86-64 (Native)Parent Process: postgres [17136]Date/Time: 2011-06-14 05:00:27.466 -0600OS Version: Mac OS X 10.6.7 (10J869)Report Version: 6Interval Since Last Report: 658000 secCrashes Since Last Report: 1Per-App Crashes Since Last Report: 1Anonymous UUID: 77053C10-A2B5-4078-A796-5862E233A1ACException Type: EXC_CRASH (SIGABRT)Exception Codes: 0x, 0xCrashed Thread: 0 Dispatch queue: com.apple.main-threadApplication Specific Information:abort() calledThread 0 Crashed: Dispatch queue: com.apple.main-thread0 libSystem.B.dylib 0x7fff821815d6 __kill + 101 libSystem.B.dylib 0x7fff82221cd6 abort + 832 postgres 0x0001002fcd52 0x1 + 31327543 postgres 0x00010005cafa multixact_twophase_postcommit + 74 (multixact.c:1367)4 postgres 0x00010005deab ProcessRecords + 91 (twophase.c:1407)5 postgres 0x00010005f78a FinishPreparedTransaction + 1610 (twophase.c:1368)6 postgres 0x000100236a5a PortalRunUtility + 298 (palloc.h:88)7 postgres 0x0001002381c5 PortalRunMulti + 597 (pquery.c:1315)8 postgres 0x000100238b78 PortalRun + 984 (pquery.c:817)9 postgres 0x000100234b2d exec_simple_query + 589 (postgres.c:1025)10 postgres 0x0001002356f1 PostgresMain + 1937 (postgres.c:3926)11 postgres 0x0001001e82ec ServerLoop + 2780 (postmaster.c:3600)12 postgres 0x0001001e9251 PostmasterMain + 2753 (postmaster.c:1120)13 postgres 0x00010017c305 main + 965 (main.c:199)14 postgres 0x00010b14 start + 52Thread 0 crashed with X86 Thread State (64-bit): rax: 0x rbx: 0x0018 rcx: 0x7fff5fbfd3b8 rdx: 0x rdi: 0x4431 rsi: 0x0006 rbp: 0x7fff5fbfd3d0 rsp: 0x7fff5fbfd3b8 r8: 0x0004 r9: 0x r10: 0x7fff8217d616 r11: 0xff80002e28b0 r12: 0x006a r13: 0x000100508980 r14: 0x0aba r15: 0x0001004ffb20 rip: 0x7fff821815d6 rfl: 0x0202 cr2: 0x7fff70419230Binary Images: 0x1 - 0x1004fbfef +postgres ??? (???) /Volumes/High Usage/usr/local/src/build-farm-4.4/builds/HEAD/pgsql.6849/src/test/regress/tmp_check/install/usr/local/src/build-farm-4.4/builds/HEAD/inst/bin/postgres 0x7fff5fc0 - 0x7fff5fc3bdef dyld 132.1 (???) <486E6C61-1197-CC7C-2197-82CE505102D7> /usr/lib/dyld 0x7fff80b92000 - 0x7fff80cb3fe7 libcrypto.0.9.8.dylib 0.9.8 (compatibility 0.9.8) <48AEAFE1-21F4-B3C8-4199-35AD5E8D0613> /usr/lib/libcrypto.0.9.8.dylib 0x7fff80cb4000 - 0x7fff80cc5ff7 libz.1.dylib 1.2.3 (compatibility 1.0.0) <97019C74-161A-3488-41EC-A6CA8738418C> /usr/lib/libz.1.dylib 0x7fff80df1000 - 0x7fff80f08fef libxml2.2.dylib 10.3.0 (compatibility 10.0.0) <1B27AFDD-DF87-2009-170E-C129E1572E8B> /usr/lib/libxml2.2.dylib 0x7fff82132000 - 0x7fff822f3fff libSystem.B.dylib 125.2.10 (compatibility 1.0.0) <9BAEB2F2-B485-6349-E1AB-637FE12EE770> /usr/lib/libSystem.B.dylib 0x7fff8330 - 0x7fff834befff libicucore.A.dylib 40.0.0 (compatibility 1.0.0) <2C6ECACF-CD56-1714-6F63-CB6F5EE7A1E2> /usr/lib/libicucore.A.dylib 0x7fff835a7000 - 0x7fff835b9fe7 libsasl2.2.dylib 3.15.0 (compatibility 3.0.0) <76B83C8D-8EFE-4467-0F75-275648AFED97> /usr/lib/libsasl2.2.dylib 0x7fff83c6c000 - 0x7fff83c70ff7 libmathCommon.A.dylib 315.0.0 (compatibility 1.0.0) <95718673-FEEE-B6ED-B127-BCDBDB60D4E5> /usr/lib/system/libmathCommon.A.dylib 0x7fff843c3000 - 0x7fff843c4ff7 com.apple.TrustEvaluationAgent 1.1 (1) <5952A9FA-BC2B-16EF-91A7-43902A5C07B6> /System/Library/PrivateFrameworks/TrustEvaluationAgent.framework/Versions/A/TrustEvaluationAgent 0x7fff85c2e000 - 0x7fff85cabfef libstdc++.6.dylib 7.9.0 (compatibility 7.0.0) <35ECA411-2C08-FD7D-11B1-1B7A04921A5C> /usr/lib/libstdc++.6.dylib 0x7fff85d27000 - 0x7fff85d48fff libresolv.9.dylib 41.0.0 (compatibility 1.0.0) <9F322F47-0584-CB7D-5B73-9EBD670851CD> /usr/lib/libresolv.9.dylib 0x7fff85e5c000 - 0x7fff85e99fff com.apple.LDAPFramework 2.0 (120.1) <54A6769E-D7E2-DBE2-EA61-87B9EA355DA4> /System/Library/Frameworks/LDAP.framework/Versions/A/LDAP 0x7fff87cdd000 - 0x7fff87d17fff libssl.0.9.8.dylib 0.9.8 (compatibility 0.9.8
Re: [HACKERS] Detailed documentation for external calls (threading, shared resources etc)
Seref Arikan writes: > This is actually a request for documentation guidance. I intend to > develop an extension to postgresql. Basically I'd like to place calls > to network using ZeroMQ, and I need to have detailed information about You didn't tell us about what you want to achieve, only how. It could be that your problem could be solved with a custom PGQ consumer, that leaves well outside the database server. PGQ is the generic queue layer on which the replication system Londiste is built. http://wiki.postgresql.org/wiki/Skytools http://wiki.postgresql.org/wiki/PGQ_Tutorial http://wiki.postgresql.org/wiki/Londiste_Tutorial Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: pgbench cpu overhead (was Re: [HACKERS] lazy vxid locks, v1)
On Mon, Jun 13, 2011 at 9:09 PM, Alvaro Herrera wrote: > I noticed that pgbench's doCustom (the function highest in the profile > posted) returns doing nothing if the connection is supposed to be > "sleeping"; seems an open door for busy waiting. I didn't check the > rest of the code to see if there's something avoiding that condition. Yes, there is a "select" in threadRun that avoids that. Also, I don't think anyone would but in a "sleep" in this particular type of pgbench run. > I > also noticed that it seems to be very liberal about calling > INSTR_TIME_SET_CURRENT in the same function which perhaps could be > optimizing by calling it a single time at entry and reusing the value, > but I guess that would show up in the profile as a kernel call so it's > maybe not a problem. I think that only gets called when you specifically asked for latencies or for logging, or when making new connection (which should be rare) Cheers, Jeff -- 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] perltidy
On Tue, Jun 14, 2011 at 21:39, Bruce Momjian wrote: > Peter Eisentraut wrote: >> How much do we care about applying perltidy, as described in >> src/tools/msvc/README, everywhere? I just ran it across the entire >> tree, using >> >> perltidy -b -bl -nsfs -naws -l=100 -ole=unix **/*.pl **/*.pm >> >> and it generated 6531 lines of (unified) diff, of which 357 are in >> src/tools/msvc/ itself. So clearly it's not being applied very >> consistently. >> >> Given how easily this appears to work and how we're sneakily expanding >> the use of Perl, I think we ought to add this to the standard pgindent >> routine. > > Yes, I would support that. I think I suggested that before at some point, but can't find the reference. But that means, +1. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] perltidy
Peter Eisentraut wrote: > How much do we care about applying perltidy, as described in > src/tools/msvc/README, everywhere? I just ran it across the entire > tree, using > > perltidy -b -bl -nsfs -naws -l=100 -ole=unix **/*.pl **/*.pm > > and it generated 6531 lines of (unified) diff, of which 357 are in > src/tools/msvc/ itself. So clearly it's not being applied very > consistently. > > Given how easily this appears to work and how we're sneakily expanding > the use of Perl, I think we ought to add this to the standard pgindent > routine. Yes, I would support that. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] perltidy
How much do we care about applying perltidy, as described in src/tools/msvc/README, everywhere? I just ran it across the entire tree, using perltidy -b -bl -nsfs -naws -l=100 -ole=unix **/*.pl **/*.pm and it generated 6531 lines of (unified) diff, of which 357 are in src/tools/msvc/ itself. So clearly it's not being applied very consistently. Given how easily this appears to work and how we're sneakily expanding the use of Perl, I think we ought to add this to the standard pgindent routine. -- 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] psql describe.c cleanup
Excerpts from Josh Kupershmidt's message of sáb may 07 16:40:35 -0300 2011: > And while I'm griping about describe.c, is it just me or is the source > code indentation in that file totally screwy? I'm using emacs and I've > loaded the snippet for pgsql-c-mode from > ./src/tools/editors/emacs.samples into my ~/.emacs, but the > indentation of multiline strings still looks inconsistent. See > attached screenshot, e.g. the start of line 80 has four tabs and one > space, while line 79 has six tabs and two spaces? pgindent moves strings back to the left when it thinks they fit within 80 columns. Yes, that seems pretty screwy. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] ITYM DROP TABLE
Excerpts from Tom Lane's message of mar jun 14 13:04:30 -0400 2011: > Alvaro Herrera writes: > > Done that way (9.0 and beyond). > > Re-reading the actual commit, I notice that there's now a grammatical > problem: the following sentence says > >It also entirely avoids the VACUUM >overhead caused by a bulk DELETE. > > which was okay when "it" referred to "ALTER TABLE", but now that there > are two commands mentioned in the previous sentence, it doesn't match. > Perhaps "These commands also avoid the ...". Yeah, fixed. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] [WIP] cache estimates, cache access cost
2011/6/14 Robert Haas : > On Tue, Jun 14, 2011 at 12:06 PM, Cédric Villemain > wrote: >>> 1. ANALYZE happens far too infrequently to believe that any data taken >>> at ANALYZE time will still be relevant at execution time. >> >> ANALYZE happens when people execute it, else it is auto-analyze and I >> am not providing auto-analyze-oscache. >> ANALYZE OSCACHE is just a very simple wrapper to update pg_class. The >> frequency is not important here, I believe. > > Well, I'm not saying you have to have all the answers to post a WIP > patch, certainly. But in terms of getting something committable, it > seems like we need to have at least an outline of what the long-term > plan is. If ANALYZE OSCACHE is an infrequent operation, then the data > isn't going to be a reliable guide to what will happen at execution > time... Ok. > >>> 2. Using data gathered by ANALYZE will make plans less stable, and our >>> users complain not infrequently about the plan instability we already >>> have, therefore we should not add more. > > ...and if it is a frequent operation then it's going to result in > unstable plans (and maybe pg_class bloat). There's a fundamental > tension here that I don't think you can just wave your hands at. I don't want to hide that point, which is just correct. The idea is not to have something (which need to be) updated too much but it needs to be taken into account. > >> I was trying to split the patch size by group of features to reduce >> its size. The work is in progress. > > Totally reasonable, but I can't see committing any of it without some > evidence that there's light at the end of the tunnel. No performance > tests *whatsoever* have been done. We can debate the exact amount of > evidence that should be required to prove that something is useful > from a performance perspective, but we at least need some. I'm > beating on this point because I believe that the whole idea of trying > to feed this information back into the planner is going to turn out to > be something that we don't want to do. I think it's going to turn out > to have downsides that are far larger than the upsides. it is possible, yes. I try to do changes in a way that if the reloscache values is the one by default then the planner keep the same behavior than in the past. > I am > completely willing to be be proven wrong, but right now I think this > will make things worse and you think it will make things better and I > don't see any way to bridge that gap without doing some measurements. correct. > > For example, if you run this patch on a system and subject that system > to a relatively even workload, how much do the numbers bounce around > between runs? What if you vary the workload, so that you blast it > with OLTP traffic at some times and then run reporting queries at > other times? Or different tables become hot at different times? This is all true, this is *already* true. Like the thread about random_page_cost vs index_page_cost where the good option is to change the parameters at certain moment in the day (IIRC the use case). I mean that I agree that those benchs need to be done, hopefully I can fix some usecases, while not breaking others too much or not at all, or ... > > Once you've written code to make the planner do something with the > caching % values, then you can start to explore other questions. Can > you generate plan instability, especially on complex queries, which > are more prone to change quickly based on small changes in the cost > estimates? Can you demonstrate a workload where bad performance is > inevitable with the current code, but with your code, the system My next step is cost estimation changes. I have already some very small usecases where the minimum changes I did so far are interesting but it is not enought to come with that as evidences. > becomes self-tuning and ends up with good performance? What happens > if you have a large cold table with a small hot end where all activity > is concentrated? We are at step 3 here :-) I have already some ideas to handle those situations but not yet polished. The current idea is to be conservative, like PostgreSQL used to be, for example: /* * disk and cache costs * this assumes an agnostic knowledge of the data repartition and query * usage despite large tables may have a hot part of 10% which is the only * requested part or that we select only (c)old data so the cache useless. * We keep the original strategy to not guess too much and just ponderate * the cost globaly. */ run_cost += baserel->pages * ( spc_seq_page_cost * (1 - baserel->oscache) + cache_page_cost * baserel->oscache); > > -- > Robert Haas > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > -- Cédric Villemain 2ndQuadrant http://2ndQuadrant.fr/ PostgreSQL : Expertise, For
Re: [HACKERS] use less space in xl_xact_commit patch
On Tue, Jun 14, 2011 at 2:31 PM, Simon Riggs wrote: > We don't need to be in a hurry here. As the reviewer I'm happy to give > Leonardo some time, obviously no more than the end of the commit fest. Well, we certainly have the option to review and commit the patch any time up until feature freeze. However, I don't want the CommitFest application to be full of entries for patches that are not actually being worked on, because it makes it hard for reviewers to figure out which patches in a state where they can be usefully looked at. AIUI, this one is currently not, because it was reviewed three weeks ago and hasn't been updated. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] SSI patch renumbered existing 2PC resource managers??
Bruce Momjian wrote: > > This argument seems a tad peculiar, since the *entire* *point* of > > pg_upgrade is to push physical files from one installation into another > > even though compatibility isn't guaranteed. It is the program's duty to > > understand enough to know whether it can transport the cluster's state > > safely. Not to arbitrarily discard state because it might possibly not > > be transportable. > > Well, pg_upgrade succeeds because it does as little as necessary to do > the migration, relying on pg_dump to do much of the migration work at > the catalog level. pg_upgrade tries to be involved as little as > possible with the Postgres code so it doesn't have to be changed > regularly between major versions. > > The prepared transaction case seems ugly enough that we don't want > pg_upgrade to have to check every major release if anything changed > about the data stored in prepared transactions. This is the same reason > pg_upgrade doesn't transfer WAL files from the old cluster, just pg_clog > files (which rarely changes its format). I have applied the attached pg_upgrade patch to head and 9.1 to fail if prepared transactions are in the old or new cluster. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + diff --git a/contrib/pg_upgrade/check.c b/contrib/pg_upgrade/check.c new file mode 100644 index fdec6e3..376d25a *** a/contrib/pg_upgrade/check.c --- b/contrib/pg_upgrade/check.c *** static void check_old_cluster_has_new_cl *** 16,21 --- 16,22 static void check_locale_and_encoding(ControlData *oldctrl, ControlData *newctrl); static void check_is_super_user(ClusterInfo *cluster); + static void check_for_prepared_transactions(ClusterInfo *cluster); static void check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster); static void check_for_reg_data_type_usage(ClusterInfo *cluster); *** check_old_cluster(bool live_check, *** 65,70 --- 66,72 * Check for various failure cases */ check_is_super_user(&old_cluster); + check_for_prepared_transactions(&old_cluster); check_for_reg_data_type_usage(&old_cluster); check_for_isn_and_int8_passing_mismatch(&old_cluster); *** check_new_cluster(void) *** 117,122 --- 119,125 get_db_and_rel_infos(&new_cluster); check_new_cluster_is_empty(); + check_for_prepared_transactions(&new_cluster); check_old_cluster_has_new_cluster_dbs(); check_loadable_libraries(); *** check_is_super_user(ClusterInfo *cluster *** 507,512 --- 510,545 /* + * check_for_prepared_transactions() + * + * Make sure there are no prepared transactions because the storage format + * might have changed. + */ + static void + check_for_prepared_transactions(ClusterInfo *cluster) + { + PGresult *res; + PGconn *conn = connectToServer(cluster, "template1"); + + prep_status("Checking for prepared transactions"); + + res = executeQueryOrDie(conn, + "SELECT * " + "FROM pg_catalog.pg_prepared_xact()"); + + if (PQntuples(res) != 0) + pg_log(PG_FATAL, "The %s cluster contains prepared transactions\n", + CLUSTER_NAME(cluster)); + + PQclear(res); + + PQfinish(conn); + + check_ok(); + } + + + /* * check_for_isn_and_int8_passing_mismatch() * * contrib/isn relies on data type int8, and in 8.4 int8 can now be passed -- 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] ITYM DROP TABLE
On Mon, Jun 13, 2011 at 10:44 PM, David E. Wheeler wrote: > I was reading the partitioning docs when I spotted this. I think it means to > highlight the advantages of DROP TABLE over DELETE rather than ALTER TABLE. > > Best, > > David > > diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml > index 4c9fc5d..0cdb800 100644 > *** a/doc/src/sgml/ddl.sgml > --- b/doc/src/sgml/ddl.sgml > *** VALUES ('New York', NULL, NULL, 'NY'); > *** 2332,2338 > > Bulk loads and deletes can be accomplished by adding or removing > partitions, if that requirement is planned into the partitioning > design. > ! ALTER TABLE is far faster than a bulk operation. > It also entirely avoids the VACUUM > overhead caused by a bulk DELETE. > > --- 2332,2338 > > Bulk loads and deletes can be accomplished by adding or removing > partitions, if that requirement is planned into the partitioning > design. > ! DROP TABLE is far faster than a bulk operation. > It also entirely avoids the VACUUM > overhead caused by a bulk DELETE. > That looks weird. I'm sure that *used* to say DROP TABLE. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- 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] One-Shot Plans
Simon Riggs wrote: > On Tue, Jun 14, 2011 at 7:28 PM, Bruce Momjian wrote: > > Simon Riggs wrote: > >> Currently, the planner and executor are mostly independent of each > >> other: the planner doesn't really know when the plan will be executed, > >> and the executor doesn't know how recently the plan was made. > >> > >> We can work out the various paths through the traffic cop to see when > >> a plan will be a "one-shot" - planned and then executed immediately, > >> then discarded. > > > > I was also hoping someday allow plans that are to be immediately > > executed to probe the buffer cache to determine how expensive index > > scans would be. > > Yes, it opens up many optimizations, both for cache sensitivity and > dynamic data access. > > But those are later ideas based on the existence of this first step. Agreed. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Crash dumps
Hello, Because, I work a little bit on streaming protocol and from time to time I have crashes. I want ask if you wont crash reporting (this is one of minors products from mmap playing) those what I have there is mmaped areas, and call stacks, and some other stuff. This based reports works for Linux with gdb, but there is some pluggable architecture, which connects with segfault - one thing that should be considered is to kill other processes immediately when reporting started (as taking report takes some time) so some IPC will be required. I may polish this a little bit, and send patch for this (currently without IPC killing of others). Regards, Radosław Smogura -- 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] One-Shot Plans
On Tue, Jun 14, 2011 at 7:28 PM, Bruce Momjian wrote: > Simon Riggs wrote: >> Currently, the planner and executor are mostly independent of each >> other: the planner doesn't really know when the plan will be executed, >> and the executor doesn't know how recently the plan was made. >> >> We can work out the various paths through the traffic cop to see when >> a plan will be a "one-shot" - planned and then executed immediately, >> then discarded. > > I was also hoping someday allow plans that are to be immediately > executed to probe the buffer cache to determine how expensive index > scans would be. Yes, it opens up many optimizations, both for cache sensitivity and dynamic data access. But those are later ideas based on the existence of this first step. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- 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] use less space in xl_xact_commit patch
On Tue, Jun 14, 2011 at 4:21 PM, Robert Haas wrote: > On Wed, May 25, 2011 at 3:05 PM, Simon Riggs wrote: >> Yes, that's correct. We can remove them from the normal commit record >> when nmsgs == 0. > > Leonardo, can you submit an updated version of this patch today that > incorporates Simon's suggestion? The CommitFest starts tomorrow. If > not, please feel free to resubmit to the next CommitFest. Simon or I > may find the time to review it sooner rather than later even if it > misses the deadline for this CommitFest, because I think it's an > important optimization and I know you have other work that depends on > it. But for now we should mark it Returned with Feedback if you don't > have an updated version ready to go. We don't need to be in a hurry here. As the reviewer I'm happy to give Leonardo some time, obviously no more than the end of the commit fest. If he doesn't respond at all, I'll do it, but I'd like to give him the chance and the experience if possible. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- 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] One-Shot Plans
Simon Riggs wrote: > Currently, the planner and executor are mostly independent of each > other: the planner doesn't really know when the plan will be executed, > and the executor doesn't know how recently the plan was made. > > We can work out the various paths through the traffic cop to see when > a plan will be a "one-shot" - planned and then executed immediately, > then discarded. I was also hoping someday allow plans that are to be immediately executed to probe the buffer cache to determine how expensive index scans would be. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] One-Shot Plans
Currently, the planner and executor are mostly independent of each other: the planner doesn't really know when the plan will be executed, and the executor doesn't know how recently the plan was made. We can work out the various paths through the traffic cop to see when a plan will be a "one-shot" - planned and then executed immediately, then discarded. In those cases we can take advantage of better optimisations. Most interestingly, we can evaluate stable functions at plan time, to allow us to handle partitioning and partial indexes better. Patch attached. Works... SET constraint_exclusion = on; ALTER TABLE ADD CHECK (dt < current_date - 5); SELECT * FROM WHERE datecolumn >= current_date - 1; QUERY PLAN -- Result (cost=0.00..0.01 rows=1 width=0) One-Time Filter: false (2 rows) WIP in the sense that we might want to change the special case parameter handling as well. Comments? -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services oneshot_plans.v2.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
Re: [HACKERS] procpid?
Greg Smith wrote: > Doing this presumes the existence of a large number of tools where > the author is unlikely to be keeping up with PostgreSQL > development. I don't believe that theorized set of users actually > exists. There could be a number of queries used for monitoring or administration which will be affected. Just on our Wiki pages we have some queries available for copy/paste which would need multiple versions while both column names were in supported versions of the software: http://wiki.postgresql.org/wiki/Lock_dependency_information http://wiki.postgresql.org/wiki/Lock_Monitoring http://wiki.postgresql.org/wiki/Backend_killer_function I agree that these are manageable, but not necessarily trivial. (You should see how long it can take to get them to install new monitoring software to our centralized system here.) I think that's consistent with the "save up our breaking changes to do them all at once" approach. -Kevin -- 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] procpid?
On Tue, Jun 14, 2011 at 1:43 PM, Jaime Casanova wrote: > On Tue, Jun 14, 2011 at 12:25 PM, Greg Smith wrote: >> >> Anyway, I want a larger change to pg_stat_activity than this one > > Well, Simon recomended to have a big bag of changes that justify break > tools... and you have presented a good one item for that bag... > Maybe we should start a wiki page for this and put there all the > changes we want to see before break anything? > > for example, a change i want to see is in csvlog: i want a duration > field there because tools like pgfouine, pgsi and others parse the > message field for a "duration" string which is only usefull if the > message is in english which non-english dba's won't have There are real problems with the idea of having one release where we break everything that we want to break - mostly from a process standpoint. We aren't always good at being organized and disciplined, and coming up with a multi-year plan to break everything all at once in 2014 for release in 2015 may be difficult, because it requires a consensus on release management to hold together for years, and sometimes we can't even manage "days". But I don't think it's a bad idea to try. So +1 for creating a list of things that we think we might like to break at some point. It might be worth trying to do this in the context of the Todo list - come up with some special badge or flag that we can put on items that require a compatibility break, so that we can scan for them there easily. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] startupBufferPinWaitBufId vs. ProcGlobalShmemSize
ProcGlobalShmemSize() currently includes code to allow space for startupBufferPinWaitBufId. But I think that's redundant, because that variable is stored in PROC_HDR *ProcGlobal, for which this function is separately allocating space. So I propose to apply the attached patch, barring objections. I see no reason to back-patch this, since allocating an extra 4 bytes of shared memory is hardly anything to get excited about. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company procglobalshmemsize-cleanup.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
Re: [HACKERS] procpid?
On Tue, Jun 14, 2011 at 12:25 PM, Greg Smith wrote: > > Anyway, I want a larger change to pg_stat_activity than this one Well, Simon recomended to have a big bag of changes that justify break tools... and you have presented a good one item for that bag... Maybe we should start a wiki page for this and put there all the changes we want to see before break anything? for example, a change i want to see is in csvlog: i want a duration field there because tools like pgfouine, pgsi and others parse the message field for a "duration" string which is only usefull if the message is in english which non-english dba's won't have -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación -- 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] [WIP] cache estimates, cache access cost
On Tue, Jun 14, 2011 at 12:06 PM, Cédric Villemain wrote: >> 1. ANALYZE happens far too infrequently to believe that any data taken >> at ANALYZE time will still be relevant at execution time. > > ANALYZE happens when people execute it, else it is auto-analyze and I > am not providing auto-analyze-oscache. > ANALYZE OSCACHE is just a very simple wrapper to update pg_class. The > frequency is not important here, I believe. Well, I'm not saying you have to have all the answers to post a WIP patch, certainly. But in terms of getting something committable, it seems like we need to have at least an outline of what the long-term plan is. If ANALYZE OSCACHE is an infrequent operation, then the data isn't going to be a reliable guide to what will happen at execution time... >> 2. Using data gathered by ANALYZE will make plans less stable, and our >> users complain not infrequently about the plan instability we already >> have, therefore we should not add more. ...and if it is a frequent operation then it's going to result in unstable plans (and maybe pg_class bloat). There's a fundamental tension here that I don't think you can just wave your hands at. > I was trying to split the patch size by group of features to reduce > its size. The work is in progress. Totally reasonable, but I can't see committing any of it without some evidence that there's light at the end of the tunnel. No performance tests *whatsoever* have been done. We can debate the exact amount of evidence that should be required to prove that something is useful from a performance perspective, but we at least need some. I'm beating on this point because I believe that the whole idea of trying to feed this information back into the planner is going to turn out to be something that we don't want to do. I think it's going to turn out to have downsides that are far larger than the upsides. I am completely willing to be be proven wrong, but right now I think this will make things worse and you think it will make things better and I don't see any way to bridge that gap without doing some measurements. For example, if you run this patch on a system and subject that system to a relatively even workload, how much do the numbers bounce around between runs? What if you vary the workload, so that you blast it with OLTP traffic at some times and then run reporting queries at other times? Or different tables become hot at different times? Once you've written code to make the planner do something with the caching % values, then you can start to explore other questions. Can you generate plan instability, especially on complex queries, which are more prone to change quickly based on small changes in the cost estimates? Can you demonstrate a workload where bad performance is inevitable with the current code, but with your code, the system becomes self-tuning and ends up with good performance? What happens if you have a large cold table with a small hot end where all activity is concentrated? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] procpid?
On 06/14/2011 11:44 AM, Jim Nasby wrote: Wouldn't it be better still to have both the new and old columns available for a while? That would produce the minimum amount of disruption to tools, etc. Doing this presumes the existence of a large number of tools where the author is unlikely to be keeping up with PostgreSQL development. I don't believe that theorized set of users actually exists. There are people who use pg_stat_activity simply, and there are tool authors who are heavily involved enough that they will see a change here coming far enough in advance to adopt it without disruption. If there's a large base of "casual" tool authors, who wrote something using pg_stat_activity once and will never update it again, I don't know where they are. Anyway, I want a larger change to pg_stat_activity than this one, and I would just roll fixing this column name into that more disruptive and positive change. Right now the biggest problem with this view is that you have to parse the text of the query to figure out what state the connection is in. This is silly; there should be boolean values exposed for "idle" and "in transaction". I want to be able to write things like this: SELECT idle,in_trans,count(*) FROM pg_stat_activity GROUP BY idle,in_trans; SELECT min(backend_start) FROM pg_stat_activity WHERE idle; Right now the standard approach to this is to turn current_query into a derived state value using CASE statements. It's quite unfriendly, and a bigger problem than this procpid mismatch. Fix that whole mess at once, and now you've got something useful enough to justify breaking tools. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us -- 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] [WIP] cache estimates, cache access cost
On Tue, Jun 14, 2011 at 1:10 PM, Greg Smith wrote: > On 06/14/2011 11:04 AM, Robert Haas wrote: >> Even if the data were accurate and did not cause plan stability, we >> have no evidence that using it will improve real-world performance. > > That's the dependency Cédric has provided us a way to finally make progress > on. Everyone says there's no evidence that this whole approach will improve > performance. But we can't collect such data, to prove or disprove it helps, > without a proof of concept patch that implements *something*. You may not > like the particular way the data is collected here, but it's a working > implementation that may be useful for some people. I'll take "data > collected at ANALYZE time" as a completely reasonable way to populate the > new structures with realistic enough test data to use initially. But there's no reason that code (which may or may not eventually prove useful) has to be incorporated into the main tree. We don't commit code so people can go benchmark it; we ask for the benchmarking to be done first, and then if the results are favorable, we commit the code. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] [WIP] cache estimates, cache access cost
On 06/14/2011 11:04 AM, Robert Haas wrote: Even if the data were accurate and did not cause plan stability, we have no evidence that using it will improve real-world performance. That's the dependency Cédric has provided us a way to finally make progress on. Everyone says there's no evidence that this whole approach will improve performance. But we can't collect such data, to prove or disprove it helps, without a proof of concept patch that implements *something*. You may not like the particular way the data is collected here, but it's a working implementation that may be useful for some people. I'll take "data collected at ANALYZE time" as a completely reasonable way to populate the new structures with realistic enough test data to use initially. Surely at least one other way to populate the statistics, and possibly multiple other ways that the user selects, will be needed eventually. I commented a while ago on this thread: every one of these discussions always gets dragged into the details of how the cache statistics data will be collected and rejects whatever is suggested as not good enough. Until that stops, no progress will ever get made on the higher level details. By its nature, developing toward integrating cached percentages is going to lurch forward on both "collecting the cache data" and "using the cache knowledge in queries" fronts almost independently. This is not a commit candidate; it's the first useful proof of concept step for something we keep talking about but never really doing. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us -- 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] ITYM DROP TABLE
Alvaro Herrera writes: > Done that way (9.0 and beyond). Re-reading the actual commit, I notice that there's now a grammatical problem: the following sentence says It also entirely avoids the VACUUM overhead caused by a bulk DELETE. which was okay when "it" referred to "ALTER TABLE", but now that there are two commands mentioned in the previous sentence, it doesn't match. Perhaps "These commands also avoid the ...". 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] Why polecat and colugos are failing to build back branches
On Jun 13, 2011, at 6:05 PM, Tom Lane wrote: > I looked into $SUBJECT. There appear to be two distinct issues: > > 1. On colugos (OS X with LLVM), the ... > However, because when using gcc that only results in a warning, > we didn't back-patch it. Now it appears that it's an error when using > LLVM, so maybe we oughta back-patch it, at least to whichever releases > we think will build with LLVM. FYI, in the latest beta developer XCode release, gcc is llvm. That's not my test machine though. > > 2. Pre-9.0, the installation step is failing like this: ... > alternatives seem to be > to ask Robert to rename the volume, or stop testing pre-9.0 branches on > that machine. Any change is no problem, just let me know. Later, Rob -- 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] procpid?
Jim Nasby wrote: > On Jun 13, 2011, at 10:56 AM, Simon Riggs wrote: > > If we were going to make changes like this, I'd suggest we save them > > up in a big bag for when we change major version number. Everybody in > > the world thinks that PostgreSQL v8 is compatible across all versions > > (8.0, 8.1, 8.2, 8.3, 8.4), and it will be same with v9. That way we > > would still have forward progress, but in more sensible sized steps. > > Otherwise we just break the code annually for all the people that > > support us. If we had a more stable environment for tools vendors, > > maybe people wouldn't need to be manually typing procpid anyway... > > Wouldn't it be better still to have both the new and old columns > available for a while? That would produce the minimum amount of > disruption to tools, etc. The only downside is some potential confusion, > but that would just serve to drive people to the documentation to see > why there were two fields, where they would find out one was deprecated. Well, someone doing SELECT *, which is probably 90% of the users, are going to be pretty confused by duplicate columns, asking, "What is the difference"? For those people this would make things worse than they are now. I would say 90% of users are doing SELECT *, and 10% are joining to other tables or displaying specific columns. We want to help that 10% without making that 90% confused. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] procpid?
On Jun 13, 2011, at 10:56 AM, Simon Riggs wrote: > If we were going to make changes like this, I'd suggest we save them > up in a big bag for when we change major version number. Everybody in > the world thinks that PostgreSQL v8 is compatible across all versions > (8.0, 8.1, 8.2, 8.3, 8.4), and it will be same with v9. That way we > would still have forward progress, but in more sensible sized steps. > Otherwise we just break the code annually for all the people that > support us. If we had a more stable environment for tools vendors, > maybe people wouldn't need to be manually typing procpid anyway... Wouldn't it be better still to have both the new and old columns available for a while? That would produce the minimum amount of disruption to tools, etc. The only downside is some potential confusion, but that would just serve to drive people to the documentation to see why there were two fields, where they would find out one was deprecated. -- Jim C. Nasby, Database Architect j...@nasby.net 512.569.9461 (cell) http://jim.nasby.net -- 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] ITYM DROP TABLE
Excerpts from David E. Wheeler's message of mar jun 14 12:33:27 -0400 2011: > On Jun 14, 2011, at 8:03 AM, Tom Lane wrote: > > >> - ALTER TABLE is far faster than a bulk operation. > >> + ALTER TABLE (to split out a sub-table from the > >> partitioned > >> + table) and DROP TABLE (to remove a partition > >> altogether) are > >> + both far faster than a bulk operation. > > > > I think you need to spell out "ALTER TABLE NO INHERIT" if you are going > > to do that. This formulation seems to imply that *any* form of ALTER > > TABLE is fast, which surely ain't the truth. > > > >> However, this introductory text is supposed to be very brief; maybe we > >> should remove mention of specific commands here. > > > > No, I don't think it needs to be that brief. But if you think your > > version is too long, remove the parenthetical remarks. > > +1 I think that would be perfect. Done that way (9.0 and beyond). -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] ITYM DROP TABLE
On Jun 14, 2011, at 8:03 AM, Tom Lane wrote: >> - ALTER TABLE is far faster than a bulk operation. >> + ALTER TABLE (to split out a sub-table from the partitioned >> + table) and DROP TABLE (to remove a partition altogether) >> are >> + both far faster than a bulk operation. > > I think you need to spell out "ALTER TABLE NO INHERIT" if you are going > to do that. This formulation seems to imply that *any* form of ALTER > TABLE is fast, which surely ain't the truth. > >> However, this introductory text is supposed to be very brief; maybe we >> should remove mention of specific commands here. > > No, I don't think it needs to be that brief. But if you think your > version is too long, remove the parenthetical remarks. +1 I think that would be perfect. Best, David -- 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] psql describe.c cleanup
On Sat, May 7, 2011 at 2:40 PM, Josh Kupershmidt wrote: > Hi all, > > I use psql's -E mode every now and then, copy-and-pasting and further > tweaking the SQL displayed. Most queries are displayed terminated by a > semicolon, but quite a few aren't, making copy-and-paste just a bit > more tedious. > > Attached is a patch to fix every SQL query I saw in describe.c. There > were also a few queries with trailing newlines, and I fixed those too. I did a quick review and test of your patch. It didn't quite apply cleanly due to recent non-related describe.c changes -- updated patch attached. First, I'll give you a thumbs up on the original inspiration for the patch. The output should be standardized, and I see no reason not to append a semicolon on usability basis. Beyond that, the changes are mostly cosmetic and I can't see how it will break things outside of terminating a query early by accident (I didn't see any). What I do wonder though is if the ; appending should really be happening in printQuery() instead of in each query -- the idea being that formatting for external consumption should be happening in one place. Maybe that's over-thinking it though. merlin psql_describe.v3.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
Re: [HACKERS] [WIP] cache estimates, cache access cost
2011/6/14 Robert Haas : > On Tue, Jun 14, 2011 at 10:29 AM, Cédric Villemain > wrote: >> 0001-Add-reloscache-column-to-pg_class.patch >> 0002-Add-a-function-to-update-the-new-pg_class-cols.patch >> 0003-Add-ANALYZE-OSCACHE-VERBOSE-relation.patch >> 0004-Add-a-Hook-to-handle-OSCache-stats.patch >> 0005-Add-reloscache-to-Index-Rel-OptInfo.patch >> 0006-Add-cache_page_cost-GUC.patch > > It seems to me that posting updated versions of this patch gets us no > closer to addressing the concerns I (and Tom, on other threads) > expressed about this idea previously. Specifically: > > 1. ANALYZE happens far too infrequently to believe that any data taken > at ANALYZE time will still be relevant at execution time. ANALYZE happens when people execute it, else it is auto-analyze and I am not providing auto-analyze-oscache. ANALYZE OSCACHE is just a very simple wrapper to update pg_class. The frequency is not important here, I believe. > 2. Using data gathered by ANALYZE will make plans less stable, and our > users complain not infrequently about the plan instability we already > have, therefore we should not add more. Again, it is hard to do a UPDATE pg_class SET reloscache, so I used ANALYZE logic. Also I have taken into account the fact that someone may want to SET the values like it was also suggested, so my patches allow to do : 'this table is 95% in cache, the DBA said' (it is stable, not based on OS stats). This case has been suggested several times and is covered by my patch. > 3. Even if the data were accurate and did not cause plan stability, we > have no evidence that using it will improve real-world performance. I have not finish my work on cost estimation and I believe this work will take some time and can be done in another commitfest. At the moment my patches do not change anything on the dcision of the planner, just offers the tools I need to hack cost estimates. > > Now, it's possible that you or someone else could provide some > experimental evidence refuting these points. But right now there > isn't any, and until there is, -1 from me on applying any of this. I was trying to split the patch size by group of features to reduce its size. The work is in progress. > > -- > Robert Haas > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > -- Cédric Villemain 2ndQuadrant http://2ndQuadrant.fr/ PostgreSQL : Expertise, Formation et Support -- 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: CreateComments: use explicit indexing for ``values''
I have left update_attstat which I'm unsure about, and have attached the updated patch handling the other cases. This will be linked in via the commitfest page. I picked commands/comment.c randomly and found the i = 0, i++ method of initializing the array made it harder for me to visualize it's structure, or verify each value was in the correct place in the array. Obviously we can assume each value is in the correct place because it's been like that in working code. This patch makes the intent of each initialization clear by using the constants directly instead of in a comment, and has the effect of being able to verify each line on it's own. The original requires verification of the preceding i++'s. I expanded upon my original patch of only handling CreateComments to include the other cases for consistency - but only so far as update_attstats as I found it too complex for me and left it. I don't like to break anything. I'm not going to push for it if most people prefer the original code for readability, or maintainability across versions; I just thought I'd post my thoughts with a patch. An easier route, might be for me to submit a patch which just changes comment.c by adding in the constants via a comment like the other places. What do you think ? Richard --- On Tue, 14/6/11, Alvaro Herrera wrote: > From: Alvaro Herrera > Subject: Re: [HACKERS] PATCH: CreateComments: use explicit indexing for > ``values'' > To: "richhguard-monotone" > Cc: "pgsql-hackers" > Date: Tuesday, 14 June, 2011, 15:20 > Excerpts from richhguard-monotone's > message of lun jun 13 16:10:17 -0400 2011: > > Apologies - I meant to CC in the list but forgot. > > > > I have gone through and changed all the related > functions except ``update_attstats''. > > > > Do you have any advice of how to handle the inner > loops, such as those initializing ``stakindN''. The entries > before can be handled just like in this patch, by using the > symbolic constants. > > Based on Tom's comments, I'd submit the patch without that > bit, at least > as a first step. > > > Again, this is based on master and all existing tests > pass. > > Please post the patch and add it here: > https://commitfest.postgresql.org/action/commitfest_view?id=10 > > -- > Álvaro Herrera > The PostgreSQL Company - Command Prompt, Inc. > PostgreSQL Replication, Consulting, Custom Development, > 24x7 support >commit 7a689fc96b624de5de589cd9e2ef1c42ae542a16 Author: Richard Hopkins Date: Mon Jun 13 20:44:15 2011 +0100 Initialize arrays using Anum_pg symbolic constants This clarifies the intent of the code by using the already defined symbolic constants; the constants were referenced in the related comments for each column anyway. However, ``update_attstats'' still uses the old style of i=0, and then i++ with each element initialization. I have left this for now as it seems too tricky to include with these changes. ``update_attstats'' has a few loops using ``STATISTIC_NUM_SLOTS'' and I'm unsure whether to remove them, and use the symbolic constants such as ``Anum_pg_statistic_stakind1'' at the expense of adding extra lines. diff --git a/src/backend/catalog/pg_operator.c b/src/backend/catalog/pg_operator.c index ccd0fe1..4d2d7b7 100644 --- a/src/backend/catalog/pg_operator.c +++ b/src/backend/catalog/pg_operator.c @@ -234,22 +234,21 @@ OperatorShellMake(const char *operatorName, * initialize values[] with the operator name and input data types. Note * that oprcode is set to InvalidOid, indicating it's a shell. */ - i = 0; namestrcpy(&oname, operatorName); - values[i++] = NameGetDatum(&oname); /* oprname */ - values[i++] = ObjectIdGetDatum(operatorNamespace); /* oprnamespace */ - values[i++] = ObjectIdGetDatum(GetUserId()); /* oprowner */ - values[i++] = CharGetDatum(leftTypeId ? (rightTypeId ? 'b' : 'r') : 'l'); /* oprkind */ - values[i++] = BoolGetDatum(false); /* oprcanmerge */ - values[i++] = BoolGetDatum(false); /* oprcanhash */ - values[i++] = ObjectIdGetDatum(leftTypeId); /* oprleft */ - values[i++] = ObjectIdGetDatum(rightTypeId); /* oprright */ - values[i++] = ObjectIdGetDatum(InvalidOid); /* oprresult */ - values[i++] = ObjectIdGetDatum(InvalidOid); /* oprcom */ - values[i++] = ObjectIdGetDatum(InvalidOid); /* oprnegate */ - values[i++] = ObjectIdGetDatum(InvalidOid); /* oprcode */ - values[i++] = ObjectIdGetDatum(InvalidOid); /* oprrest */ - values[i++] = ObjectIdGetDatum(InvalidOid); /* oprjoin */ + values[Anum_pg_operator_oprname - 1] = NameGetDatum(&oname); + values[Anum_pg_operator_oprnamespace - 1] = ObjectIdGetDatum(operatorNamespace); + values[Anum_pg_operator_oprowner - 1] = ObjectIdGetDatum(GetUserId()); + values[Anum_pg_operator_oprkind - 1] = CharGetDatum(leftTypeId ? (rightTypeId ? 'b' : 'r') : 'l'); + values[Anum_pg_operator_oprcanmerge - 1] = BoolGetDatum(false); + values[Anum_pg_operator_oprcanhash - 1] = BoolGetDatum(false); + values[Anum_pg_operator_op
Re: [HACKERS] Patch: add GiST support for BOX @> POINT queries
On Fri, Jun 10, 2011 at 6:16 AM, Hitoshi Harada wrote: > 2011/2/24 Andrew Tipton : >> While playing around with the BOX and POINT datatypes, I was surprised to >> note that BOX @> POINT (and likewise POINT <@ BOX) queries were not using >> the GiST index I had created on the BOX column. The attached patch adds a >> new strategy @>(BOX,POINT) to the box_ops opclass. Internally, >> gist_box_consistent simply transforms the POINT into its corresponding BOX. >> This is my first Postgres patch, and I wasn't able to figure out how to go >> about creating a regression test for this change. (All existing tests do >> pass, but none of them seem to specifically test index behaviour.) > > I reviewed the patch and worried about hard-wired magic number as > StrategyNumber. At least you should use #define to indicate the > number's meaning. > > In addition, the modified gist_box_consistent() is too dangerous; > q_box is declared in the if block locally and is referenced, which > pointer is passed to the outer process of the block. AFAIK if the > local memory of each block is alive outside if block is > platform-dependent. > > Isn't it worth adding new consistent function for those purposes? The > approach in the patch as stands looks kludge to me. Andrew - in case it's not clear, we're waiting on you to respond to Hitoshi's comments or provide an updated patch. Thanks, -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] use less space in xl_xact_commit patch
On Wed, May 25, 2011 at 3:05 PM, Simon Riggs wrote: > Yes, that's correct. We can remove them from the normal commit record > when nmsgs == 0. Leonardo, can you submit an updated version of this patch today that incorporates Simon's suggestion? The CommitFest starts tomorrow. If not, please feel free to resubmit to the next CommitFest. Simon or I may find the time to review it sooner rather than later even if it misses the deadline for this CommitFest, because I think it's an important optimization and I know you have other work that depends on it. But for now we should mark it Returned with Feedback if you don't have an updated version ready to go. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] [WIP] cache estimates, cache access cost
On Tue, Jun 14, 2011 at 10:29 AM, Cédric Villemain wrote: > 0001-Add-reloscache-column-to-pg_class.patch > 0002-Add-a-function-to-update-the-new-pg_class-cols.patch > 0003-Add-ANALYZE-OSCACHE-VERBOSE-relation.patch > 0004-Add-a-Hook-to-handle-OSCache-stats.patch > 0005-Add-reloscache-to-Index-Rel-OptInfo.patch > 0006-Add-cache_page_cost-GUC.patch It seems to me that posting updated versions of this patch gets us no closer to addressing the concerns I (and Tom, on other threads) expressed about this idea previously. Specifically: 1. ANALYZE happens far too infrequently to believe that any data taken at ANALYZE time will still be relevant at execution time. 2. Using data gathered by ANALYZE will make plans less stable, and our users complain not infrequently about the plan instability we already have, therefore we should not add more. 3. Even if the data were accurate and did not cause plan stability, we have no evidence that using it will improve real-world performance. Now, it's possible that you or someone else could provide some experimental evidence refuting these points. But right now there isn't any, and until there is, -1 from me on applying any of this. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] ITYM DROP TABLE
Alvaro Herrera writes: > Excerpts from David E. Wheeler's message of lun jun 13 17:44:05 -0400 2011: >> I was reading the partitioning docs when I spotted this. I think it means to >> highlight the advantages of DROP TABLE over DELETE rather than ALTER TABLE. > I think the point of the existing wording is to point out > ALTER TABLE / NO INHERIT. I wonder if it's worth expanding the text to > mention both, such as > - ALTER TABLE is far faster than a bulk operation. > + ALTER TABLE (to split out a sub-table from the partitioned > + table) and DROP TABLE (to remove a partition altogether) > are > + both far faster than a bulk operation. I think you need to spell out "ALTER TABLE NO INHERIT" if you are going to do that. This formulation seems to imply that *any* form of ALTER TABLE is fast, which surely ain't the truth. > However, this introductory text is supposed to be very brief; maybe we > should remove mention of specific commands here. No, I don't think it needs to be that brief. But if you think your version is too long, remove the parenthetical remarks. 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] SSI work for 9.1
Heikki Linnakangas wrote: > I did some further changes, refactoring SkipSerialization so that > it's hopefully more readable, and added a comment about the > side-effects. See attached. Let me know if I'm missing something. I do think the changes improve readability. I don't see anything missing, but there's something we can drop. Now that you've split the read and write tests, this part can be dropped from the SerializationNeededForWrite function: + + /* Check if we have just become "RO-safe". */ + if (SxactIsROSafe(MySerializableXact)) + { + ReleasePredicateLocks(false); + return false; + } If it's doing a write, it can't be a read-only transaction -Kevin -- 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: patch review : Add ability to constrain backend temporary file space
2011/6/3 Mark Kirkwood : > On 02/06/11 18:34, Jaime Casanova wrote: >> >> >> - the patch adds this to serial_schedule but no test has been added... >> >> diff --git a/src/test/regress/serial_schedule >> b/src/test/regress/serial_schedule >> index bb654f9..325cb3d 100644 >> --- a/src/test/regress/serial_schedule >> +++ b/src/test/regress/serial_schedule >> @@ -127,3 +127,4 @@ test: largeobject >> test: with >> test: xml >> test: stats >> +test: resource >> > > Corrected v4 patch with the test files, for completeness. Note that > discussion has moved on and there will be a v5 :-) > Mark, can you submit your updated patch ? -- Cédric Villemain 2ndQuadrant http://2ndQuadrant.fr/ PostgreSQL : Expertise, Formation et Support -- 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: CreateComments: use explicit indexing for ``values''
Excerpts from Tom Lane's message of mar jun 14 10:30:28 -0400 2011: > Alvaro Herrera writes: > > Excerpts from richhguard-monotone's message of lun jun 13 16:10:17 -0400 > > 2011: > >> Do you have any advice of how to handle the inner loops, such as those > >> initializing ``stakindN''. The entries before can be handled just like in > >> this patch, by using the symbolic constants. > > > Based on Tom's comments, I'd submit the patch without that bit, at least > > as a first step. > > He already did no? I don't see the patch attached anywhere ... > I did think of a possible way to rewrite update_attstats: instead of > > for (k = 0; k < STATISTIC_NUM_SLOTS; k++) > { > values[i++] = ObjectIdGetDatum(stats->staop[k]);/* staopN */ > } > > do > > for (k = 0; k < STATISTIC_NUM_SLOTS; k++) > { > values[Anum_pg_statistic_staop1 - 1 + k] = > ObjectIdGetDatum(stats->staop[k]); > } > > etc. However, it's not clear to me whether this is really an > improvement. Opinions? I guess the other option is i = Anum_pg_statistic_staop1 - 1; for (k = 0; k < STATISTIC_NUM_SLOTS; k++) { values[i++] = ObjectIdGetDatum(stats->staop[k]); } (I also tried moving the i initialization to the "for" first arg, but it seems better this way) Not sure what's better. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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: CreateComments: use explicit indexing for ``values''
On Tue, Jun 14, 2011 at 10:30 AM, Tom Lane wrote: > Alvaro Herrera writes: >> Excerpts from richhguard-monotone's message of lun jun 13 16:10:17 -0400 >> 2011: >>> Do you have any advice of how to handle the inner loops, such as those >>> initializing ``stakindN''. The entries before can be handled just like in >>> this patch, by using the symbolic constants. > >> Based on Tom's comments, I'd submit the patch without that bit, at least >> as a first step. > > He already did no? > > I did think of a possible way to rewrite update_attstats: instead of > > for (k = 0; k < STATISTIC_NUM_SLOTS; k++) > { > values[i++] = ObjectIdGetDatum(stats->staop[k]); /* staopN */ > } > > do > > for (k = 0; k < STATISTIC_NUM_SLOTS; k++) > { > values[Anum_pg_statistic_staop1 - 1 + k] = > ObjectIdGetDatum(stats->staop[k]); > } > > etc. However, it's not clear to me whether this is really an > improvement. Opinions? I don't care that much, but IMV that's just gilding the lily. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] ITYM DROP TABLE
Excerpts from David E. Wheeler's message of lun jun 13 17:44:05 -0400 2011: > I was reading the partitioning docs when I spotted this. I think it means to > highlight the advantages of DROP TABLE over DELETE rather than ALTER TABLE. I think the point of the existing wording is to point out ALTER TABLE / NO INHERIT. I wonder if it's worth expanding the text to mention both, such as --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -2320,7 +2320,9 @@ VALUES ('New York', NULL, NULL, 'NY'); Bulk loads and deletes can be accomplished by adding or removing partitions, if that requirement is planned into the partitioning design. - ALTER TABLE is far faster than a bulk operation. + ALTER TABLE (to split out a sub-table from the partitioned + table) and DROP TABLE (to remove a partition altogether) are + both far faster than a bulk operation. It also entirely avoids the VACUUM overhead caused by a bulk DELETE. However, this introductory text is supposed to be very brief; maybe we should remove mention of specific commands here. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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: CreateComments: use explicit indexing for ``values''
Alvaro Herrera writes: > Excerpts from richhguard-monotone's message of lun jun 13 16:10:17 -0400 2011: >> Do you have any advice of how to handle the inner loops, such as those >> initializing ``stakindN''. The entries before can be handled just like in >> this patch, by using the symbolic constants. > Based on Tom's comments, I'd submit the patch without that bit, at least > as a first step. He already did no? I did think of a possible way to rewrite update_attstats: instead of for (k = 0; k < STATISTIC_NUM_SLOTS; k++) { values[i++] = ObjectIdGetDatum(stats->staop[k]);/* staopN */ } do for (k = 0; k < STATISTIC_NUM_SLOTS; k++) { values[Anum_pg_statistic_staop1 - 1 + k] = ObjectIdGetDatum(stats->staop[k]); } etc. However, it's not clear to me whether this is really an improvement. Opinions? 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] [WIP] cache estimates, cache access cost
2011/5/16 Greg Smith : > Cédric Villemain wrote: >> >> >> http://git.postgresql.org/gitweb?p=users/c2main/postgres.git;a=shortlog;h=refs/heads/analyze_cache >> > > This rebases easily to make Cedric's changes move to the end; I just pushed > a version with that change to > https://github.com/greg2ndQuadrant/postgres/tree/analyze_cache if anyone > wants a cleaner one to browse. I've attached a patch too if that's more > your thing. > > I'd recommend not getting too stuck on the particular hook Cédric has added > here to compute the cache estimate, which uses mmap and mincore to figure it > out. It's possible to compute similar numbers, albeit less accurate, using > an approach similar to how pg_buffercache inspects things. And I even once > wrote a background writer extension that collected this sort of data as it > was running the LRU scan anyway. Discussions of this idea seem to focus on > how the "what's in the cache?" data is collected, which as far as I'm > concerned is the least important part. There are multiple options, some > work better than others, and there's no reason that can't be swapped out > later. The more important question is how to store the data collected and > then use it for optimizing queries. Attached are updated patches without the plugin itself. I've also added the cache_page_cost GUC, this one is not per tablespace, like others page_cost. There are 6 patches: 0001-Add-reloscache-column-to-pg_class.patch 0002-Add-a-function-to-update-the-new-pg_class-cols.patch 0003-Add-ANALYZE-OSCACHE-VERBOSE-relation.patch 0004-Add-a-Hook-to-handle-OSCache-stats.patch 0005-Add-reloscache-to-Index-Rel-OptInfo.patch 0006-Add-cache_page_cost-GUC.patch I have some comments on my own code: * I am not sure of the best datatype to use for 'reloscache' * I didn't include the catalog number change in the patch itself. * oscache_update_relstats() is very similar to vac_update_relstats(), maybe better to merge them but reloscache should not be updated at the same time than other stats. * There is probably too much work done in do_oscache_analyze_rel() because I kept vac_open_indexes() (not a big drama atm) * I don't know so much how gram.y works, so I am not sure my changes cover all cases. * No tests; similar columns and GUC does not have test either, but it lacks a test for ANALYZE OSCACHE -- Cédric Villemain 2ndQuadrant http://2ndQuadrant.fr/ PostgreSQL : Expertise, Formation et Support From d2fe7e85aea31cfe8cd6559a060f71c424fe03af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Villemain?= Date: Wed, 25 May 2011 23:17:36 +0200 Subject: [PATCH 1/7] Add reloscache column to pg_class 1 column reloscache to contain the percentage of pages in cache per relation. May be used by the planner and updated with ANALYZE OSCACHE; (not done yet, see next commits) --- doc/src/sgml/catalogs.sgml | 11 + src/backend/catalog/heap.c |1 + src/backend/utils/cache/relcache.c |2 + src/include/catalog/pg_class.h | 44 ++- 4 files changed, 37 insertions(+), 21 deletions(-) diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml new file mode 100644 index 8504555..4cfad39 *** a/doc/src/sgml/catalogs.sgml --- b/doc/src/sgml/catalogs.sgml *** *** 1634,1639 --- 1634,1650 + reloscache + float4 + + +Percentage of the files in OS cache. This is only an estimate used by +the planner. It is updated by ANALYZE OSCACHE. +By default, the value is not updated and an extension is required. + + + + reltoastrelid oid pg_class.oid diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c new file mode 100644 index a6e541d..2043c40 *** a/src/backend/catalog/heap.c --- b/src/backend/catalog/heap.c *** InsertPgClassTuple(Relation pg_class_des *** 764,769 --- 764,770 values[Anum_pg_class_reltablespace - 1] = ObjectIdGetDatum(rd_rel->reltablespace); values[Anum_pg_class_relpages - 1] = Int32GetDatum(rd_rel->relpages); values[Anum_pg_class_reltuples - 1] = Float4GetDatum(rd_rel->reltuples); + values[Anum_pg_class_reloscache - 1] = Float4GetDatum(rd_rel->reloscache); values[Anum_pg_class_reltoastrelid - 1] = ObjectIdGetDatum(rd_rel->reltoastrelid); values[Anum_pg_class_reltoastidxid - 1] = ObjectIdGetDatum(rd_rel->reltoastidxid); values[Anum_pg_class_relhasindex - 1] = BoolGetDatum(rd_rel->relhasindex); diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c new file mode 100644 index d7e94ff..ca09e3b *** a/src/backend/utils/cache/relcache.c --- b/src/backend/utils/cache/relcache.c *** formrdesc(const char *relationName, Oid *** 1417,1422 --- 1417,1423 relation->rd_rel->relpages = 1; relation->rd_rel->reltuples = 1; + relation->rd_rel->reloscache = 0; relation->rd_rel->relki
Re: [HACKERS] WIP: collect frequency statistics for arrays
Version of patch with few more comments and some fixes. -- With best regards, Alexander Korotkov. arrayanalyze-0.4.patch.gz Description: GNU Zip compressed data -- 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: CreateComments: use explicit indexing for ``values''
Excerpts from richhguard-monotone's message of lun jun 13 16:10:17 -0400 2011: > Apologies - I meant to CC in the list but forgot. > > I have gone through and changed all the related functions except > ``update_attstats''. > > Do you have any advice of how to handle the inner loops, such as those > initializing ``stakindN''. The entries before can be handled just like in > this patch, by using the symbolic constants. Based on Tom's comments, I'd submit the patch without that bit, at least as a first step. > Again, this is based on master and all existing tests pass. Please post the patch and add it here: https://commitfest.postgresql.org/action/commitfest_view?id=10 -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] Boolean operators without commutators vs. ALL/ANY
On Jun14, 2011, at 14:29 , Robert Haas wrote: > On Tue, Jun 14, 2011 at 6:10 AM, Florian Pflug wrote: >> On Jun13, 2011, at 05:44 , Tom Lane wrote: >>> Robert Haas writes: On Sun, Jun 12, 2011 at 7:46 AM, Florian Pflug wrote: > (B) There should be a way to use ANY()/ALL() with the > array elements becoming the left arguments of the operator. >>> It seems to me that if we provided some way of handling this, your first proposal would be moot; and I have to say I like the idea of allowing this a lot more than tinkering with the operator names. >>> >>> There are syntactic reasons not to do that. It'd be a lot easier just >>> to provide a commutator operator for ~. >> >> My suggestion would be the add a commutator for "~" as a short-term >> solution (preferably in 9.1). > > I don't think we want to bump catversion again before release if we > can avoid it. And I don't see this as being a terribly urgent problem > - it's not like this is a new regression, and I can't remember hearing > any complaints about it prior to two days ago. Hm, OK, that makes sense... >> Since "~" doesn't inspire any obvious names for a possible commutator, >> I suggest adding "=~" and "~=". >> >> Is there any support for that proposal? > > I'm OK with adding a commutator but I guess I don't see the point of > adding a synonym for ~ along the way. The existing use of ~ is > consistent with, for example, awk, so it's not like we've dreamed up > something utterly crazy that we now need to fix. I'd suggest we just > come up with some arbitrary variant, like ~~ or <~ or #~ or > !#!%@~bikeshed++!. That, however, I'm not at all happy with. Quite frankly, operator naming is already a bit of a mess, and readability of queries suffers as a result. The geometric types are especially vile offenders in this regard, but the various array-related operators aren't poster children either. I think we should try to work towards more mnemonic operator naming, not add to the mess by defining commutator pairs whose names bear no visual resemblance whatsoever to one each other. I'm not wedded to "=~", it's just the only name I could come up which (a) has a natural commutator (b) gives visual indication of which argument constitutes the text and which the pattern (c) there is precedent for. BTW, there's actually precedent for a commutator of "~", namely "@". Some of the geometric types (polygon, box, circle, point, path) use "~" as a commutator for "@" (which stands for "contains"). But IMHO that mainly proves that the geometric types are vile offenders when it comes to readability... The pair ("@", "~" ) is also the only pair of commutators whose names are totally unrelated to each other. Given a suitable definition of a reverse() function for text [1], the following query select o1.oprleft::regtype || ' ' || o1.oprname || ' ' || o1.oprright::regtype as opr, o2.oprleft::regtype || ' ' || o2.oprname || ' ' || o2.oprright::regtype as com, o1.oprcode as opr_code, o2.oprcode as com_code from pg_operator o1 join pg_operator o2 on o1.oprcom = o2.oid or o2.oprcom = o1.oid where o1.oid < o2.oid and o1.oprname <> reverse(translate(o2.oprname, '<>', '><')) and o1.oprname <> translate(o2.oprname, '<>', '><'); produces opr|com| opr_code | com_code ---+---+-+--- polygon @ polygon | polygon ~ polygon | poly_contained | poly_contain box @ box | box ~ box | box_contained | box_contain circle @ circle | circle ~ circle | circle_contained| circle_contain point @ path | path ~ point | on_ppath| path_contain_pt point @ polygon | polygon ~ point | pt_contained_poly | poly_contain_pt point @ circle| circle ~ point| pt_contained_circle | circle_contain_pt (6 rows) best regards, Florian Pflug [1] I used create or replace function reverse(text) returns text as $$ select string_agg(substring($1, i, 1), '') from generate_series(length($1), 1, -1) i $$ language sql immutable; -- 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] SSI patch renumbered existing 2PC resource managers??
Tom Lane wrote: > Bruce Momjian writes: > > Tom Lane wrote: > >> No, pg_upgrade should not be unilaterally refusing that. > > > Uh, isn't there some physical files in pg_twophase/ that stick around to > > keep prepared transactions --- if so, pg_upgrade does not copy them from > > the old cluster to the new one. I am also hesistant to do so because > > there might be data in there that isn't portable. > > This argument seems a tad peculiar, since the *entire* *point* of > pg_upgrade is to push physical files from one installation into another > even though compatibility isn't guaranteed. It is the program's duty to > understand enough to know whether it can transport the cluster's state > safely. Not to arbitrarily discard state because it might possibly not > be transportable. Well, pg_upgrade succeeds because it does as little as necessary to do the migration, relying on pg_dump to do much of the migration work at the catalog level. pg_upgrade tries to be involved as little as possible with the Postgres code so it doesn't have to be changed regularly between major versions. The prepared transaction case seems ugly enough that we don't want pg_upgrade to have to check every major release if anything changed about the data stored in prepared transactions. This is the same reason pg_upgrade doesn't transfer WAL files from the old cluster, just pg_clog files (which rarely changes its format). -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] ITYM DROP TABLE
On Mon, Jun 13, 2011 at 5:44 PM, David E. Wheeler wrote: > I was reading the partitioning docs when I spotted this. I think it means to > highlight the advantages of DROP TABLE over DELETE rather than ALTER TABLE. I guess they might mean ALTER TABLE .. NO INHERIT. But I think I agree that DROP TABLE would be better. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Creating new remote branch in git?
On Tue, Jun 14, 2011 at 14:40, Bruce Momjian wrote: > Robert Haas wrote: >> On Mon, Jun 13, 2011 at 10:54 PM, Bruce Momjian wrote: >> > Alvaro Herrera wrote: >> >> Excerpts from Bruce Momjian's message of lun jun 13 18:38:46 -0400 2011: >> >> > Andrew Dunstan wrote: >> >> >> >> > > Is putting remotes in your ~/.gitconfig ?good practice? I certainly >> >> > > don't have any in mine. >> >> > >> >> > Putting 'github' in there allows me to push/pull from github branches >> >> > without having to specify the github URL. >> >> >> >> I think his point is that they are more properly specified in each >> >> repo's .git/config file, not the global $HOME/.gitconfig. ?If you were >> >> to check out some other, unrelated project, you could end up pushing >> >> unrelated branches to PG's repo ... ?Not sure if this is really >> >> possible, but it certainly seems scary to do things that way. >> > >> > I understand now --- that it is risky to create an "origin" branch in >> > ~/.gitconfig. ?I am now using an alias: >> > >> > ? ? ? ?[alias] >> > ? ? ? ? ? ? ? ?pgclone = clone >> > ssh://g...@gitmaster.postgresql.org/postgresql.git >> > >> > I assume the 'github' branch in ~/.gitconfig is fine. >> >> That, too, would better off inside $REPO/.git/config, although it's >> certainly less risky than the other one. It doesn't make much sense >> to have an upstream that applies across every repository you have >> checked out. > > Wouldn't I conceivably use github with a variety of projects? I try to > use ~/.gitconfig so I don't have to redo a lot of stuff when I reinstall > my PG git tree. Yes, but your reference goes to a specific repository at github (bmomjian/postgres). Which wouldn't be correct for any other project... -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] Creating new remote branch in git?
Magnus Hagander wrote: > >> > I understand now --- that it is risky to create an "origin" branch in > >> > ~/.gitconfig. ?I am now using an alias: > >> > > >> > ? ? ? ?[alias] > >> > ? ? ? ? ? ? ? ?pgclone = clone > >> > ssh://g...@gitmaster.postgresql.org/postgresql.git > >> > > >> > I assume the 'github' branch in ~/.gitconfig is fine. > >> > >> That, too, would better off inside $REPO/.git/config, although it's > >> certainly less risky than the other one. ?It doesn't make much sense > >> to have an upstream that applies across every repository you have > >> checked out. > > > > Wouldn't I conceivably use github with a variety of projects? ?I try to > > use ~/.gitconfig so I don't have to redo a lot of stuff when I reinstall > > my PG git tree. > > Yes, but your reference goes to a specific repository at github > (bmomjian/postgres). Which wouldn't be correct for any other > project... Ah, I see your point. Thanks. I renamed it 'pggithub'. I think I need to read Pro Git (http://progit.org/book/), though I am not sure that would have helped me in this exact case. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers