Re: [HACKERS] procpid?
On Sat, Jun 11, 2011 at 12:02 AM, Jim Nasby j...@nasby.net wrote: It's damn annoying... enough so that I'd personally be in favor of creating a pid column that has the same data so we can deprecate procpid and eventually remove it... well, if we will start changing bad picked names we will have a *lot* of work to do... starting by the project's name ;) -- 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 1 June 2011 23:47, Alvaro Herrera alvhe...@commandprompt.com wrote: Here's a complete patch with all this stuff, plus doc additions and simple regression tests for the new ALTER DOMAIN commands. Enable CHECK constraints to be declared NOT VALID This means that they can initially be added to a large existing table without checking its initial contents, but new tuples must comply to them; a separate pass invoked by ALTER TABLE / VALIDATE can verify existing data and ensure it complies with the constraint, at which point it is marked validated and becomes a normal part of the table ecosystem. 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. Regards, Dean -- 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 11 June 2011 14:32, Dean Rasheed dean.a.rash...@gmail.com wrote: On 1 June 2011 23:47, Alvaro Herrera alvhe...@commandprompt.com wrote: Here's a complete patch with all this stuff, plus doc additions and simple regression tests for the new ALTER DOMAIN commands. Enable CHECK constraints to be declared NOT VALID This means that they can initially be added to a large existing table without checking its initial contents, but new tuples must comply to them; a separate pass invoked by ALTER TABLE / VALIDATE can verify existing data and ensure it complies with the constraint, at which point it is marked validated and becomes a normal part of the table ecosystem. 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. Do the standards explicitly stipulate an expected behaviour for this? And does such a problem affect the invalid foreign key change too? -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 EnterpriseDB UK: 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 CHECK constraints as NOT VALID
On 11 June 2011 14:40, Thom Brown t...@linux.com wrote: On 11 June 2011 14:32, Dean Rasheed dean.a.rash...@gmail.com wrote: On 1 June 2011 23:47, Alvaro Herrera alvhe...@commandprompt.com wrote: Here's a complete patch with all this stuff, plus doc additions and simple regression tests for the new ALTER DOMAIN commands. Enable CHECK constraints to be declared NOT VALID This means that they can initially be added to a large existing table without checking its initial contents, but new tuples must comply to them; a separate pass invoked by ALTER TABLE / VALIDATE can verify existing data and ensure it complies with the constraint, at which point it is marked validated and becomes a normal part of the table ecosystem. 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. Do the standards explicitly stipulate an expected behaviour for this? No I believe that this is a PostgreSQL-specific optimisation, and we need to ensure that queries return the correct results with constraint_exclusion on. And does such a problem affect the invalid foreign key change too? No only CHECK constraints (and possibly NOT NULL constraints in the future). Regards, Dean -- 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 11 June 2011 16:40, Dean Rasheed dean.a.rash...@gmail.com wrote: On 11 June 2011 14:40, Thom Brown t...@linux.com wrote: On 11 June 2011 14:32, Dean Rasheed dean.a.rash...@gmail.com wrote: On 1 June 2011 23:47, Alvaro Herrera alvhe...@commandprompt.com wrote: Here's a complete patch with all this stuff, plus doc additions and simple regression tests for the new ALTER DOMAIN commands. Enable CHECK constraints to be declared NOT VALID This means that they can initially be added to a large existing table without checking its initial contents, but new tuples must comply to them; a separate pass invoked by ALTER TABLE / VALIDATE can verify existing data and ensure it complies with the constraint, at which point it is marked validated and becomes a normal part of the table ecosystem. 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. Do the standards explicitly stipulate an expected behaviour for this? No I believe that this is a PostgreSQL-specific optimisation, and we need to ensure that queries return the correct results with constraint_exclusion on. And does such a problem affect the invalid foreign key change too? No only CHECK constraints (and possibly NOT NULL constraints in the future). Regards, Dean Since you've mentioned the SQL spec, its worth noting that whilst I think that this feature will be very useful, it's not the feature in the SQL spec (at least not in my version). The SQL spec feature is to mark a constraint as NOT ENFORCED, which means that no data (existing or new) is checked against the constraint. It's as if the constraint were not present at all. In Oracle this corresponds to the syntax ALTER TABLE mytable ENABLE/DISABLE myconstraint which is actually quite handy during a bulk load/update - disable all your constraints, do the bulk operation and then re-enable them (automatically re-validating them). This is better than dropping and re-creating the constraints because you don't need to remember all the constraint definitions. I can see both features being quite useful in different situations. Regards, Dean -- 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] Core Extensions relocation
Peter Eisentraut wrote: For the directory name, I'd prefer either src/extensions (since there is more than one), or if you want to go for short somehow, src/ext. (Hmm, I guess the installation subdirectory is also called extension. But it felt wrong on first reading anyway.) I jumped between those two a couple of times myself, settling on extension to match the installation location as you figured out. Assuming that name shouldn't change at this point, this seemed the best way to name the new directory, even though I agree it seems weird at first. What version did you branch this off? :) Long enough ago that apparently I've missed some major changes; Magnus already pointed out I needed to revisit how MODULEDIR was used. Looks like I need to rebuild the first patch in this series yet again, which shouldn't be too bad. The second time I did that, I made the commits atomic enough that the inevitable third one would be easy. -- 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
[HACKERS] REL9_1_STABLE branch created
Please remember to double-patch anything that should go into 9.1. For the record, the correct formula seems to be $ git pull Current branch master is up to date. $ git push origin master:refs/heads/REL9_1_STABLE Total 0 (delta 0), reused 0 (delta 0) To ssh://g...@gitmaster.postgresql.org/postgresql.git * [new branch] master - REL9_1_STABLE after which you can check out the branch locally according to whatever formula you're already using (a separate workdir in my case). 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?
On 6/11/2011 1:02 AM, Jaime Casanova wrote: On Sat, Jun 11, 2011 at 12:02 AM, Jim Nasbyj...@nasby.net wrote: It's damn annoying... enough so that I'd personally be in favor of creating a pid column that has the same data so we can deprecate procpid and eventually remove it... well, if we will start changing bad picked names we will have a *lot* of work to do... starting by the project's name ;) There is a difference between a project name and something that directly affects usability. +1 on fixing this. IMO, we don't create a new pid column, we just fix the problem. If we do it for 9.2, we have 18 months to communicate the change. Joshua D. Drake -- 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] Identifying no-op length coercions
Noah Misch n...@leadboat.com writes: We originally discussed having the transform function take and return Expr nodes. It turns out that simplify_function() does not have the Expr, probably because the particular choice of node type among the many that can convey a function call does not matter to it. The same should be true of a transform function -- it should do the same thing whether the subject call arrives from a FuncExpr or an OpExpr. Therefore, I changed the transform function signature to Expr *protransform(List *args). That seems like the wrong thing. For one thing, it makes it quite impossible to share one transform support function among multiple functions/operators, since it won't know which one the argument list is for. More generally, I foresee the transform needing to know most of the other information that might be in the FuncExpr or OpExpr. It certainly would need access to the collation, and it would probably like to be able to copy the parse location to whatever it outputs, and so forth and so on. So I really think passing the node to the function is the most future-proof way to do it. If that means you need to rejigger things a bit in clauses.c, so be it. The large pg_proc.h diff mostly just adds protransform = 0 to every function. Due to the resulting patch size, I've compressed it. There are new/otherwise changed DATA lines for OIDs 669 and 3097 only. The chances that this part will still apply cleanly by the time anyone gets around to committing it are nil. I'd suggest you break it into two separate patches, one that modifies the existing lines to add the protransform = 0 column and then one to apply the remaining deltas in the file. I envision the eventual committer doing the first step on-the-fly (perhaps with an emacs macro, at least that's the way I usually do it) and then wanting a patchable diff for the rest. Or if you wanted to be really helpful, you could provide a throwaway perl script to do the modifications of the existing lines ... I haven't read the actual patch, these are just some quick reactions to your description. 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] Identifying no-op length coercions
On Sat, Jun 11, 2011 at 02:11:55PM -0400, Tom Lane wrote: Noah Misch n...@leadboat.com writes: We originally discussed having the transform function take and return Expr nodes. It turns out that simplify_function() does not have the Expr, probably because the particular choice of node type among the many that can convey a function call does not matter to it. The same should be true of a transform function -- it should do the same thing whether the subject call arrives from a FuncExpr or an OpExpr. Therefore, I changed the transform function signature to Expr *protransform(List *args). That seems like the wrong thing. For one thing, it makes it quite impossible to share one transform support function among multiple functions/operators, since it won't know which one the argument list is for. More generally, I foresee the transform needing to know most of the other information that might be in the FuncExpr or OpExpr. It certainly would need access to the collation, and it would probably like to be able to copy the parse location to whatever it outputs, and so forth and so on. So I really think passing the node to the function is the most future-proof way to do it. If that means you need to rejigger things a bit in clauses.c, so be it. Good points. I'm thinking, then, add an Expr argument to simplify_function() and have the CoerceViaIO branch of eval_const_expressions_mutator() pass NULL for both its simplify_function() calls. If simplify_function() gets a NULL Expr for a function that has a protransform, synthesize a FuncExpr based on its other arguments before calling the transform function. Seem reasonable? Granted, that would be dead code until someone applies a transform function to a type I/O function, which could easily never happen. Perhaps just ignore the transform function when we started with a CoerceViaIO node? The large pg_proc.h diff mostly just adds protransform = 0 to every function. Due to the resulting patch size, I've compressed it. There are new/otherwise changed DATA lines for OIDs 669 and 3097 only. The chances that this part will still apply cleanly by the time anyone gets around to committing it are nil. I'd suggest you break it into two separate patches, one that modifies the existing lines to add the protransform = 0 column and then one to apply the remaining deltas in the file. I envision the eventual committer doing the first step on-the-fly (perhaps with an emacs macro, at least that's the way I usually do it) and then wanting a patchable diff for the rest. Or if you wanted to be really helpful, you could provide a throwaway perl script to do the modifications of the existing lines ... That would be better; I'll do it for the next version. I haven't read the actual patch, these are just some quick reactions to your description. Thanks, nm -- 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] Small SSI issues
Dan Ports wrote: On Fri, Jun 10, 2011 at 09:43:58PM +0300, Heikki Linnakangas wrote: Do checks such as that argue for keeping the volatile flag, or do you think we can drop it if we make those changes? (That would also allow dropping a number of casts which exist just to avoid warnings.) I believe we can drop it, I'll double-check. Yes, dropping it seems like the thing to do. It's been on my list for a while. We are not really getting anything out of declaring it volatile since we cast the volatile qualifier away most of the time. I'm not concerned about references covered by SerializableXactHashLock. I am more concerned about some of the tests for whether the (MySerializableXact == InvalidSerializableXact) checks and any other tests not covered by that lock are OK without it (and OK with it). Since my knowledge of weak memory ordering behavior is, well, weak I didn't want to try to make that call. -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] Identifying no-op length coercions
Noah Misch n...@leadboat.com writes: Good points. I'm thinking, then, add an Expr argument to simplify_function() and have the CoerceViaIO branch of eval_const_expressions_mutator() pass NULL for both its simplify_function() calls. If simplify_function() gets a NULL Expr for a function that has a protransform, synthesize a FuncExpr based on its other arguments before calling the transform function. Seem reasonable? Granted, that would be dead code until someone applies a transform function to a type I/O function, which could easily never happen. Perhaps just ignore the transform function when we started with a CoerceViaIO node? Until we actually have a use-case for simplifying I/O functions like this, I can't see going out of our way for it ... 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] [BUG] Denormal float values break backup/restore
On 2011-06-11 01:57, Tom Lane wrote: (5) Lobby your libc providers to make strtod accept denormals without throwing ERANGE. There is no obvious reason why it shouldn't. (On at least one of my boxes, it does.) The standard either explicitly allows or requires this behaviour (depending on which text I look at) for underflow. AIUI that is defined to be a little vague, but includes denormalized numbers that would undergo any rounding at all. It says that on overflow the conversion should return the appropriate HUGE_VAL variant, and set ERANGE. On underflow it returns a reasonably appropriate value (and either may or must set ERANGE, which is the part that isn't clear to me). ISTM the appropriate response to ERANGE combined with a small return value is to either ignore or report the rounding error, but accept the return value. The current code in float.c doesn't check the return value at all and treats all ERANGE conditions as equal. Jeroen -- 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] pgbench--new transaction type
On Sun, May 29, 2011 at 7:04 PM, Greg Smith g...@2ndquadrant.com wrote: On 05/29/2011 03:11 PM, Jeff Janes wrote: If you use pgbench -S -M prepared at a scale where all data fits in memory, most of what you are benchmarking is network/IPC chatter, and table locking. If you profile it, you'll find a large amount of the time is actually spent doing more mundane things, like memory allocation. The network and locking issues are really not the bottleneck at all in a surprising number of these cases. I wouldn't expect IPC chatter to show up in profiling, because it costs wall time, but not CPU time. The time spent might be attributed to the kernel, or to pgbench, or to nothing at all. As part of the Eviscerating the parser discussion, I made a hack that had exec_simple_query do nothing but return a dummy completionTag. So there was no parsing, planning, or execution. Under this mode, I got 44758 TPS, or 22.3 microsecons/transaction, which should represent the cost of IPC chatter and pgbench overhead. The breakdown I get, in microseconds per item, are: 53.70 cost of a select and related overhead via -S -M prepared. of which: 22.34 cost of IPC and pgbench roundtrip, estimated via above discussion 16.91 cost of the actual execution of the select statement, estimated via the newly suggested -P mode. 14.45 residual usec cost, covering table locking, transaction begin and end, plus measurement errors. Because all my tests were single-client, the cost of locking would be much lower than they would be in contended cases. However, I wouldn't trust profiling to accurate reflect the locking time anyway, for the same reason I don't trust it for IPC chatter--wall time is consumed but not spent on the CPU, so is not counted by profiling. As you note memory allocation consumes much profiling time. However, memory allocation is a low level operation which is always in support of some higher purpose, such as parsing, execution, or taking locks. My top down approach attempts to assign these bottom-level costs to the proper higher level purpose. Your patch isn't really dependent on your being right about the cause here, which means this doesn't impact your submissions any. Just wanted to clarify that what people expect are slowing things down in this situation and what actually shows up when you profile are usually quite different. I'm not sure whether this feature makes sense to add to pgbench, but it's interesting to have it around for developer testing. Yes, this is what I thought the opinion might be. But there is no repository of such useful for developer testing patches, other than this mailing list. That being the case, would it even be worthwhile to fix it up more and submit it to commitfest? some numbers for single client runs on 64-bit AMD Opteron Linux: 12,567 sps under -S 19,646 sps under -S -M prepared 58,165 sps under -P 10,000 is too big of a burst to run at once. The specific thing I'm concerned about is what happens if you try this mode when using -T to enforce a runtime limit, and your SELECT rate isn't high. If you're only doing 100 SELECTs/second because your scale is big enough to be seek bound, you could overrun by nearly two minutes. OK. I wouldn't expect someone to want to use -P under scales that cause that to happen, but perhaps it should deal with it more gracefully. I picked 10,000 just because it obviously large enough for any hardware I have, or will have for the foreseeable future. I think this is just a matter of turning the optimization around a bit. Rather than starting with a large batch size and presuming that's ideal, in this case a different approach is really needed. You want the smallest batch size that gives you a large win here. My guess is that most of the gain here comes from increasing batch size to something in the 10 to 100 range; jumping to 10K is probably overkill. Could you try some smaller numbers and see where the big increases start falling off at? I've now used a variety of sizes (powers of 2 up to 8192, plus 1); and the results fit very well to a linear equation, with 17.3 usec/inner select plus 59.0 usec/outer invocation. So at a loop of 512, you would have overhead of 59.0/512=0.115 out of total time of 17.4, or 0.7% overhead. So that should be large enough. Thanks for the other suggestions. 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] Small SSI issues
On Sat, Jun 11, 2011 at 01:38:31PM -0500, Kevin Grittner wrote: I'm not concerned about references covered by SerializableXactHashLock. I am more concerned about some of the tests for whether the (MySerializableXact == InvalidSerializableXact) checks and any other tests not covered by that lock are OK without it (and OK with it). Since my knowledge of weak memory ordering behavior is, well, weak I didn't want to try to make that call. Oh, those checks are definitely not an issue -- MySerializableXact itself (the pointer, not the thing it's pointing to) is in backend-local memory, so it won't change under us. The volatile qualifier (as written) doesn't help with that anyway, it attaches to the data being pointed to, not the pointer itself. Dan -- Dan R. K. Ports MIT CSAILhttp://drkp.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] procpid?
Joshua D. Drake wrote: On 6/11/2011 1:02 AM, Jaime Casanova wrote: On Sat, Jun 11, 2011 at 12:02 AM, Jim Nasbyj...@nasby.net wrote: It's damn annoying... enough so that I'd personally be in favor of creating a pid column that has the same data so we can deprecate procpid and eventually remove it... well, if we will start changing bad picked names we will have a *lot* of work to do... starting by the project's name ;) There is a difference between a project name and something that directly affects usability. +1 on fixing this. IMO, we don't create a new pid column, we just fix the problem. If we do it for 9.2, we have 18 months to communicate the change. Uh, I am the first one I remember complaining about this so I don't see why we should break compatibility for such a low-level problem. -- Bruce Momjian br...@momjian.ushttp://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] Identifying no-op length coercions
On Sat, Jun 11, 2011 at 03:03:18PM -0400, Tom Lane wrote:
Noah Misch n...@leadboat.com writes:
Good points. I'm thinking, then, add an Expr argument to
simplify_function()
and have the CoerceViaIO branch of eval_const_expressions_mutator() pass
NULL
for both its simplify_function() calls. If simplify_function() gets a NULL
Expr
for a function that has a protransform, synthesize a FuncExpr based on its
other
arguments before calling the transform function. Seem reasonable? Granted,
that would be dead code until someone applies a transform function to a
type I/O
function, which could easily never happen. Perhaps just ignore the
transform
function when we started with a CoerceViaIO node?
Until we actually have a use-case for simplifying I/O functions like this,
I can't see going out of our way for it ...
Sounds good. Updated patch attached, incorporating your ideas. Before applying
it, run this command to update the uninvolved pg_proc.h DATA entries:
perl -pi -e 's/PGUID(\s+\d+){4}/$ 0/' src/include/catalog/pg_proc.h
Thanks,
nm
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 8504555..5d1a447 100644
*** a/doc/src/sgml/catalogs.sgml
--- b/doc/src/sgml/catalogs.sgml
***
*** 4337,4342
--- 4337,4349
/row
row
+ entrystructfieldprotransform/structfield/entry
+ entrytyperegproc/type/entry
+ entryliterallink
linkend=catalog-pg-procstructnamepg_proc/structname/link.oid/literal/entry
+ entryCalls to function can be simplified by this other
function/entry
+ /row
+
+ row
entrystructfieldproisagg/structfield/entry
entrytypebool/type/entry
entry/entry
diff --git a/src/backend/catalog/index 6250b07..92be0a7 100644
*** a/src/backend/catalog/pg_proc.c
--- b/src/backend/catalog/pg_proc.c
***
*** 304,309 ProcedureCreate(const char *procedureName,
--- 304,310
values[Anum_pg_proc_procost - 1] = Float4GetDatum(procost);
values[Anum_pg_proc_prorows - 1] = Float4GetDatum(prorows);
values[Anum_pg_proc_provariadic - 1] = ObjectIdGetDatum(variadicType);
+ values[Anum_pg_proc_protransform - 1] = ObjectIdGetDatum(InvalidOid);
values[Anum_pg_proc_proisagg - 1] = BoolGetDatum(isAgg);
values[Anum_pg_proc_proiswindow - 1] = BoolGetDatum(isWindowFunc);
values[Anum_pg_proc_prosecdef - 1] = BoolGetDatum(security_definer);
diff --git a/src/backend/commands/taindex 2c9f855..563a1b2 100644
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***
*** 56,61
--- 56,62
#include nodes/nodeFuncs.h
#include nodes/parsenodes.h
#include optimizer/clauses.h
+ #include optimizer/planner.h
#include parser/parse_clause.h
#include parser/parse_coerce.h
#include parser/parse_collate.h
***
*** 3495,3501 ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap,
LOCKMODE lockmode)
{
NewColumnValue *ex = lfirst(l);
! ex-exprstate = ExecPrepareExpr((Expr *) ex-expr, estate);
}
notnull_attrs = NIL;
--- 3496,3503
{
NewColumnValue *ex = lfirst(l);
! /* expr already planned */
! ex-exprstate = ExecInitExpr((Expr *) ex-expr, NULL);
}
notnull_attrs = NIL;
***
*** 4398,4404 ATExecAddColumn(List **wqueue, AlteredTableInfo *tab,
Relation rel,
newval = (NewColumnValue *)
palloc0(sizeof(NewColumnValue));
newval-attnum = attribute.attnum;
! newval-expr = defval;
tab-newvals = lappend(tab-newvals, newval);
tab-rewrite = true;
--- 4400,4406
newval = (NewColumnValue *)
palloc0(sizeof(NewColumnValue));
newval-attnum = attribute.attnum;
! newval-expr = expression_planner(defval);
tab-newvals = lappend(tab-newvals, newval);
tab-rewrite = true;
***
*** 6706,6711 ATPrepAlterColumnType(List **wqueue,
--- 6708,6716
/* Fix collations after all else */
assign_expr_collations(pstate, transform);
+ /* Plan the expr now so we can accurately assess the need to
rewrite. */
+ transform = (Node *) expression_planner((Expr *) transform);
+
/*
* Add a work queue item to make ATRewriteTable update the
column
* contents.
diff --git a/src/backend/optimizer/utilindex 2914c39..e9fb750 100644
*** a/src/backend/optimizer/util/clauses.c
--- b/src/backend/optimizer/util/clauses.c
***
*** 106,114 static List *simplify_and_arguments(List *args,
eval_const_expressions_context
*context,
Re: [HACKERS] deadlock_timeout at PGC_SIGHUP?
On Wed, Mar 30, 2011 at 04:48:26PM -0400, Robert Haas wrote:
On Tue, Mar 29, 2011 at 2:29 PM, Noah Misch n...@leadboat.com wrote:
It's actually not
clear to me what the user could usefully do other than trying to
preserve his transaction by setting a high deadlock_timeout - what is
the use case, other than that?
The other major use case is reducing latency in deadlock-prone
transactions. ?By
reducing deadlock_timeout for some or all involved transactions, the error
will
arrive earlier.
Good point.
Is it worth thinking about having an explicit setting for deadlock
priority? ?That'd be more work, of course, and I'm not sure it it's
worth it, but it'd also provide stronger guarantees than you can get
with this proposal.
That is a better UI for the first use case. ?I have only twice wished to
tweak
deadlock_timeout: once for the use case you mention, another time for that
second use case. ?Given that, I wouldn't have minded a rough UI. ?If you'd
use
this often and assign more than two or three distinct priorities, you'd
probably
appreciate the richer UI. ?Not sure how many shops fall in that group.
Me neither. If making the deadlock timeout PGC_SUSET is independently
useful, I don't object to doing that first, and then we can wait and
see if anyone feels motivated to do more.
Here's the patch for that. Not much to it.
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 3981969..f94782f 100644
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***
*** 5258,5264 dynamic_library_path =
'C:\tools\postgresql;H:\my_project\lib;$libdir'
practice. On a heavily loaded server you might want to raise it.
Ideally the setting should exceed your typical transaction time,
so as to improve the odds that a lock will be released before
! the waiter decides to check for deadlock.
/para
para
--- 5258,5265
practice. On a heavily loaded server you might want to raise it.
Ideally the setting should exceed your typical transaction time,
so as to improve the odds that a lock will be released before
! the waiter decides to check for deadlock. Only superusers can change
! this setting.
/para
para
diff --git a/src/backend/utils/index 92391ed..48ffe95 100644
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***
*** 1532,1539 static struct config_int ConfigureNamesInt[] =
},
{
! /* This is PGC_SIGHUP so all backends have the same value. */
! {deadlock_timeout, PGC_SIGHUP, LOCK_MANAGEMENT,
gettext_noop(Sets the time to wait on a lock before
checking for deadlock.),
NULL,
GUC_UNIT_MS
--- 1532,1539
},
{
! /* This is PGC_SUSET to prevent hiding from log_lock_waits. */
! {deadlock_timeout, PGC_SUSET, LOCK_MANAGEMENT,
gettext_noop(Sets the time to wait on a lock before
checking for deadlock.),
NULL,
GUC_UNIT_MS
--
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?
Greg Smith wrote: On 06/10/2011 12:19 AM, Alex Hunsaker wrote: It looks like if you push the remote branch first everything should work nicely: git checkout master git push origin origin:refs/heads/REL9_1_STABLE git fetch # fetch the new branch git checkout REL9_1_STABLE This is basically the state of the art right now for the most frequently deployed versions of git. I don't think checking out master first is necessary though. Potentially useful automation/trivia for alternate approaches includes: 1) Write a little script to do this messy chore, so you don't have to remember this weird create a new branch using a full refspec syntax. There is an example named git-create-branch along with a short tutorial on this subject at http://www.zorched.net/2008/04/14/start-a-new-branch-on-your-remote-git-repository/ 2) Use git_remote_branch https://github.com/webmat/git_remote_branch which is the swiss army knife of remote branch hackery automation. 3) Rather than manually hack the config files, use git config to do it. Not sure if this is completely workable, but something like this might connect the newly created branch to your local one after pushing it out, without actually opening the config with an editor: git config branch.REL9_1_STABLE.remote origin git config branch.REL9_1_STABLE.merge refs/heads/REL9_1_STABLE 4) Use a system with git=1.7.0, which adds: git branch --set-upstream REL9_1_STABLE origin/REL9_1_STABLE Uh, I think someone needs to add this to our wiki: http://wiki.postgresql.org/wiki/Working_with_Git http://wiki.postgresql.org/wiki/Committing_with_Git I needed this when using git-new-workdir so at least it is needed there; I am unclear how wide this is needed so I cannot add it. -- Bruce Momjian br...@momjian.ushttp://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] On-the-fly index tuple deletion vs. hot_standby
On Fri, Apr 22, 2011 at 11:10 AM, Noah Misch n...@leadboat.com wrote: On Tue, Mar 15, 2011 at 10:22:59PM -0400, Noah Misch wrote: On Mon, Mar 14, 2011 at 01:56:22PM +0200, Heikki Linnakangas wrote: On 12.03.2011 12:40, Noah Misch wrote: The installation that inspired my original report recently upgraded from 9.0.1 to 9.0.3, and your fix did significantly decrease its conflict frequency. The last several conflicts I have captured involve XLOG_BTREE_REUSE_PAGE records. (FWIW, the index has generally been pg_attribute_relid_attnam_index.) I've attached a test script demonstrating the behavior. _bt_page_recyclable approves any page deleted no more recently than RecentXmin, because we need only ensure that every ongoing scan has witnessed the page as dead. For the hot standby case, we need to account for possibly-ongoing standby transactions. Using RecentGlobalXmin covers that, albeit with some pessimism: we really only need LEAST(RecentXmin, PGPROC-xmin of walsender_1, .., PGPROC-xmin of walsender_N) - vacuum_defer_cleanup_age. Not sure the accounting to achieve that would pay off, though. Thoughts? Hmm, instead of bloating the master, I wonder if we could detect more accurately if there are any on-going scans, in the standby. For example, you must hold a lock on the index to scan it, so only transactions holding the lock need to be checked for conflict. That would be nice. Do you have an outline of an implementation in mind? In an attempt to resuscitate this thread, here's my own shot at that. Apologies in advance if it's just an already-burning straw man. I didn't see any way to take advantage of checking for the heavyweight lock that any index scan would need to hold. Have you looked at the logic in ResolveRecoveryConflictWithLock(), and at GetLockConflicts()? I am a little fuzzy on how the btree stuff works, but it seems to me that you are looking for transactions that both have an xmin before some threshold and also hold an AccessShareLock on some relation. GetLockConflicts() will provide the latter, at least. -- 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] wrong message on REASSIGN OWNED
On Thu, Jun 9, 2011 at 1:26 PM, Jaime Casanova ja...@2ndquadrant.com wrote: on shdepReassignOwned() we have this message, which is obviously wrong we are not dropping objects just reassigning them... ereport(ERROR, (errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST), errmsg(cannot drop objects owned by %s because they are required by the database system, getObjectDescription(obj; but haven't thought of a good way of rephrase it can't reassign objects owned by %s because this user is internal to the database system ? -- 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 6/11/2011 1:23 PM, Bruce Momjian wrote: There is a difference between a project name and something that directly affects usability. +1 on fixing this. IMO, we don't create a new pid column, we just fix the problem. If we do it for 9.2, we have 18 months to communicate the change. Uh, I am the first one I remember complaining about this so I don't see why we should break compatibility for such a low-level problem. Because it is a very real problem with an easy fix. We have 18 months to publicize that fix. I mean really? This is a no-brainer. JD -- 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] libpq SSL with non-blocking sockets
On Thu, Jun 9, 2011 at 10:39 AM, Martin Pihlak martin.pih...@gmail.com wrote: In PQputCopyData #2 it is visible that the first SSL_write called from pqFlush failed with SSL_ERROR_WANT_WRITE. The next SSL_write should have been a retry with the same parameters, but instead was passed a buffer with a different address and length. Hence the bad write retry. Some googling turned out similar issues for other projects using SSL with non-blocking sockets. I'm not surprised. Setting the API so that it requires the same buffer (and not just a buffer pointing to the same data, or with the same initial contents) was maybe not the greatest design decision. The possible workarounds are to disable SSL or to disable non-blocking libpq connections. Both are not always possible - security reasons, 3rd party applications, drivers, etc. So I think this should be fixed in libpq. Not sure exactly how though. It would seem that for the PQputCopyData the best would be to return 0 to indicate that the operation should be retried. No idea for the other possible cases of SSL_write() retry though. What do you think? It seems to me we should try to paper over the problem within libpq rather than allowing it to percolate any higher up the call stack. One idea is that we could add outBuffer2/outBufSize2 to struct pg_conn, or something along those lines with less obviously stupid naming. Normally those would be unused, but in the special case where SSL indicates that we must retry the call with the same arguments, we set a flag that freezes the out buffer and forces any further data to be stuffed into outBuffer2. If or when the operation finally succeeds, we then move the data from outBuffer2 into outBuffer. -- 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 Sat, Jun 11, 2011 at 9:15 PM, Joshua D. Drake j...@commandprompt.com wrote: On 6/11/2011 1:23 PM, Bruce Momjian wrote: There is a difference between a project name and something that directly affects usability. +1 on fixing this. IMO, we don't create a new pid column, we just fix the problem. If we do it for 9.2, we have 18 months to communicate the change. Uh, I am the first one I remember complaining about this so I don't see why we should break compatibility for such a low-level problem. Because it is a very real problem with an easy fix. We have 18 months to publicize that fix. I mean really? This is a no-brainer. I really don't see what the big deal with calling it the process PID rather than just the PID is. Changing something like this forces pgAdmin and every other application out there that is built to work with PG to make a code change to keep working with PG. That seems like pushing a lot of unnecessary work on other people for what is basically a minor cosmetic issue. -- 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] [v9.1] sepgsql - userspace access vector cache
On Thu, Jun 9, 2011 at 3:09 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote: Here is two level lookups. The first is from object identifiers to security label; it can be boosted using syscache mechanism. The second is from security labels to access control decision; it can be boosted using userspace avc. OK. Let's have two separate patches, then. Thinking a bit more about the issue of adding a syscache, I suspect it's probably OK to use that mechanism rather than inventing something more complicated that can kick out entries on an LRU basis. Even if you accessed a few tens of thousands of entries, the cache shouldn't grow to more than a few megabytes. And that's presumably going to be rare, and could happen with other types of objects (such as tables) using the existing system caches. So I guess it's probably premature optimization to worry about the issue now. I would, however, like to see some performance results indicating how much the cache helps, and how much memory it eats up in the process. -- 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?
2011/6/12 Robert Haas robertmh...@gmail.com: On Sat, Jun 11, 2011 at 9:15 PM, Joshua D. Drake j...@commandprompt.com wrote: On 6/11/2011 1:23 PM, Bruce Momjian wrote: There is a difference between a project name and something that directly affects usability. +1 on fixing this. IMO, we don't create a new pid column, we just fix the problem. If we do it for 9.2, we have 18 months to communicate the change. Uh, I am the first one I remember complaining about this so I don't see why we should break compatibility for such a low-level problem. Because it is a very real problem with an easy fix. We have 18 months to publicize that fix. I mean really? This is a no-brainer. I really don't see what the big deal with calling it the process PID rather than just the PID is. Changing something like this forces pgAdmin and every other application out there that is built to work with PG to make a code change to keep working with PG. That seems like pushing a lot of unnecessary work on other people for what is basically a minor cosmetic issue. I agree. This is at least a use-case for something^Wfeature like 'create synonym', allowing smooth end-user's application upgrade on schema update. I am not claiming that we need that, it just seems a good usecase for column alias/synonym. -- 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 -- 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] hot standby startup, visibility map, clog
On Thu, Jun 9, 2011 at 5:14 AM, Daniel Farina dan...@heroku.com wrote: The first fact is that turning off hot standby will let the cluster start up, but only after seeing a spate of messages like these (dozen or dozens, not thousands): 2011-06-09 08:02:32 UTC LOG: restored log file 0002002C00C0 from archive 2011-06-09 08:02:33 UTC WARNING: xlog min recovery request 2C/C1F09658 is past current point 2C/C037B278 2011-06-09 08:02:33 UTC CONTEXT: writing block 0 of relation base/16385/16784_vm xlog redo insert: rel 1663/16385/128029; tid 114321/63 2011-06-09 08:02:33 UTC LOG: restartpoint starting: xlog Most importantly, *all* such messages are in visibility map forks (_vm). I reasonably confident that my code does not start reading data until pg_start_backup() has returned, and blocks on pg_stop_backup() after having read all the data. Also, the mailing list correspondence at http://archives.postgresql.org/pgsql-hackers/2010-11/msg02034.php suggests that the visibility map is not flushed at checkpoints, so perhaps with some poor timing an old page can wander onto disk even after a checkpoint barrier that pg_start_backup waits for. (I have not yet found the critical section that makes visibilitymap buffers immune to checkpoint though). I don't believe there's anything that makes visibilitymap buffers immune to checkpoint. I might be wrong. You could probably find out by inserting a few lines of code into the buffer manager to inspect each buffer tag as the buffer is written out and elog() if the fork number is VISIBILITYMAP_FORKNUM. There is some weirdness around how the LSNs of visibility map pages are set, however. See visibilitymap_set()/clear(). Clearing a visibility map bit is done without touching the LSN at all; setting the bit advances the LSN to that of the heap page, if it currently precedes that value. It's not obvious to me whether or how that's related. I'm a bit puzzled by the warning message itself, too. I think that in order to generate that warning, you need to try to flush a page to disk whose LSN is higher than the current minimum recovery point. And the minimum recovery point is initially set to point to the redo pointer from the last checkpoint. But during an online backup, it's expected that there may be pages on disk with LSNs higher than the redo pointer from the last checkpoint. So actually now I'm not sure why people don't see this warning whenever a lengthy recovery is required to reach consitency, especially with small values of shared_buffers. I'm probably missing something here... One thing that might make it easier to understand what's going on here is to crank the log level up to DEBUG2 and post the full output from a recovery that exhibits this problem, rather than just a few lines. -- 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 Sat, Jun 11, 2011 at 9:56 PM, Cédric Villemain cedric.villemain.deb...@gmail.com wrote: 2011/6/12 Robert Haas robertmh...@gmail.com: On Sat, Jun 11, 2011 at 9:15 PM, Joshua D. Drake j...@commandprompt.com wrote: On 6/11/2011 1:23 PM, Bruce Momjian wrote: There is a difference between a project name and something that directly affects usability. +1 on fixing this. IMO, we don't create a new pid column, we just fix the problem. If we do it for 9.2, we have 18 months to communicate the change. Uh, I am the first one I remember complaining about this so I don't see why we should break compatibility for such a low-level problem. Because it is a very real problem with an easy fix. We have 18 months to publicize that fix. I mean really? This is a no-brainer. I really don't see what the big deal with calling it the process PID rather than just the PID is. Changing something like this forces pgAdmin and every other application out there that is built to work with PG to make a code change to keep working with PG. That seems like pushing a lot of unnecessary work on other people for what is basically a minor cosmetic issue. I agree. This is at least a use-case for something^Wfeature like 'create synonym', allowing smooth end-user's application upgrade on schema update. I am not claiming that we need that, it just seems a good usecase for column alias/synonym. I had the same thought. I'm not sure that this particular example would be worthwhile even if we had a column synonym facility. But at least if we were bent on changing it we could do it without breaking things. -- 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] Range Types and extensions
On Thu, Jun 9, 2011 at 6:26 PM, Florian Pflug f...@phlo.org wrote: On Jun8, 2011, at 17:46 , Jeff Davis wrote: It looks like the type input function may be a problem, because it doesn't look like it knows what the collation is yet. In other words, PG_GET_COLLATION() is zero for the type input function. But I need to do a comparison to find out if the range is valid or not. For instance: '[a, Z)'::textrange is valid in en_US but not C. Maybe that check should just be removed? If one views the range '[L, U)' as a concise way of expressing L = x AND x U for some x, then allowing the case L U seems quite natural. There won't be any such x of course, but the range is still valid, just empty. Actually, thinking for this a bit, I believe this is the only way text ranges can support collations. If the validity of a range depends on the collation, then changing the collation after creation seems weird, since it can make previous valid ranges invalid and vice versa. There could be a function RANGE_EMPTY() which people can put into their CHECK constraints if they don't want such ranges to sneak into their tables... I think the collation is going to have to be baked into the type definition, no? You can't just up and change the collation of the column as you could for a straight text column, if that might cause the contents of some rows to be viewed as invalid. -- 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] On-the-fly index tuple deletion vs. hot_standby
Hi Robert, On Sat, Jun 11, 2011 at 08:55:28PM -0400, Robert Haas wrote: On Fri, Apr 22, 2011 at 11:10 AM, Noah Misch n...@leadboat.com wrote: On Tue, Mar 15, 2011 at 10:22:59PM -0400, Noah Misch wrote: On Mon, Mar 14, 2011 at 01:56:22PM +0200, Heikki Linnakangas wrote: On 12.03.2011 12:40, Noah Misch wrote: The installation that inspired my original report recently upgraded from 9.0.1 to 9.0.3, and your fix did significantly decrease its conflict frequency. ?The last several conflicts I have captured involve XLOG_BTREE_REUSE_PAGE records. (FWIW, the index has generally been pg_attribute_relid_attnam_index.) ?I've attached a test script demonstrating the behavior. ?_bt_page_recyclable approves any page deleted no more recently than RecentXmin, because we need only ensure that every ongoing scan has witnessed the page as dead. ?For the hot standby case, we need to account for possibly-ongoing standby transactions. ?Using RecentGlobalXmin covers that, albeit with some pessimism: we really only need LEAST(RecentXmin, PGPROC-xmin of walsender_1, .., PGPROC-xmin of walsender_N) - vacuum_defer_cleanup_age. ?Not sure the accounting to achieve that would pay off, though. ?Thoughts? Hmm, instead of bloating the master, I wonder if we could detect more accurately if there are any on-going scans, in the standby. For example, you must hold a lock on the index to scan it, so only transactions holding the lock need to be checked for conflict. That would be nice. ?Do you have an outline of an implementation in mind? In an attempt to resuscitate this thread, here's my own shot at that. ?Apologies in advance if it's just an already-burning straw man. I didn't see any way to take advantage of checking for the heavyweight lock that any index scan would need to hold. Have you looked at the logic in ResolveRecoveryConflictWithLock(), and at GetLockConflicts()? I am a little fuzzy on how the btree stuff works, but it seems to me that you are looking for transactions that both have an xmin before some threshold and also hold an AccessShareLock on some relation. GetLockConflicts() will provide the latter, at least. Thanks for taking a look. For the purpose of B-tree page reuse, we don't directly care about the xmin of any active snapshot. We need only prove that no active scan is paused adjacent to the page, holding a right-link to it. We currently achieve that wait-free by first marking the page with the next available xid and then reusing it when that mark (btpo.xact) predates the oldest running xid (RecentXmin). (At the moment, I'm failing to work out why this is OK with scans from transactions that haven't allocated an xid, but I vaguely recall convincing myself it was fine at one point.) It would indeed also be enough to call GetLockConflicts(locktag-of-index, AccessExclusiveLock) and check whether any of the returned transactions have PGPROC.xmin below the mark. That's notably more expensive than just comparing RecentXmin, so I'm not sure how well it would pay off overall. However, it could only help us on the master. (Not strictly true, but any way I see to extend it to the standby has critical flaws.) On the master, we can see a conflicting transaction and put off reusing the page. By the time the record hits the standby, we have to apply it, and we might have a running transaction that will hold a lock on the index for the next, say, 72 hours. At such times, vacuum_defer_cleanup_age or hot_standby_feedback ought to prevent the recovery stall. This did lead me to realize that what we do in this regard on the standby can be considerably independent from what we do on the master. If fruitful, the standby can prove the absence of a scan holding a right-link in a completely different fashion. So, we *could* take the cleanup-lock approach on the standby without changing very much on the master. nm -- 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: missing tab completions for COMMENT ON
On Sat, May 28, 2011 at 10:38 PM, Josh Kupershmidt schmi...@gmail.com wrote:
Hi all,
psql's auto-complete support for COMMENT ON was missing support for a
few object types:
1.) EXTENSION and PROCEDURAL LANGUAGE are now auto-complete candidates
for COMMENT ON [TAB]. Lists of extensions and procedural languages
should also be filled in when a user types
COMMENT ON EXTENSION [TAB]
COMMENT ON PROCEDURAL LANGUAGE [TAB]
I don't see much point in auto-completing COMMENT ON PROCEDURAL
LANGUAGE. We already complete COMMENT ON LANGUAGE. I agree with
adding EXTENSION (so done).
2.) This part of tab-complete.c looked like a spurious leftover:
*** psql_completion(char *text, int start, i
*** 1580,1592
COMPLETE_WITH_LIST(list_TRANS2);
}
else if ((pg_strcasecmp(prev4_wd, COMMENT) == 0
pg_strcasecmp(prev3_wd, ON) == 0) ||
(pg_strcasecmp(prev6_wd, COMMENT) == 0
! pg_strcasecmp(prev5_wd, ON) == 0) ||
! (pg_strcasecmp(prev5_wd, ON) == 0
! pg_strcasecmp(prev4_wd, TEXT) == 0
! pg_strcasecmp(prev3_wd, SEARCH) == 0))
COMPLETE_WITH_CONST(IS);
Since we want these choices to be filled in for COMMENT ON TEXT SEARCH [TAB]:
{CONFIGURATION, DICTIONARY, PARSER, TEMPLATE, NULL};
which were already being handled correctly in an above block.
Hmm, yeah. But while I'm looking at it, I notice that this will
handle object-type names that are one word or three words, but not two
words. And we now have FOREIGN TABLE. Committed your change plus a
bit of additional hackery to address that issue.
One piece that I gave up on trying to fix is the auto-completion for
{OPERATOR, OPERATOR CLASS, OPERATOR FAMILY}, since getting it working
correctly would be a real hassle. There's the trouble of whether to
auto-complete operators for OPERATOR [TAB], or whether to fill in
{CLASS, FAMILY} instead. Plus the auto-completes for 'USING
index_method'.
While wasting time on OPERATOR [TAB], I realized we're being a bit
overeager with this bit:
else if ((pg_strcasecmp(prev4_wd, COMMENT) == 0
pg_strcasecmp(prev3_wd, ON) == 0) ||
(pg_strcasecmp(prev6_wd, COMMENT) == 0
pg_strcasecmp(prev5_wd, ON) == 0))
COMPLETE_WITH_CONST(IS);
which will auto-complete e.g.
COMMENT ON AGGREGATE avg [TAB]
with 'IS', when instead we'd want the possible argument types to avg,
or nothing at all. Same deal with a few other object types, but it's
probably not worth worrying about (at least, I'm not worrying about it
at the moment).
The whole tab completion machinery is pretty much just throwing darts
while blindfolded, but the effort to replace it with something better
is so large that we just keep poking at it the way it is...
--
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] On-the-fly index tuple deletion vs. hot_standby
On Sat, Jun 11, 2011 at 11:40 PM, Noah Misch n...@leadboat.com wrote: We currently achieve that wait-free by first marking the page with the next available xid and then reusing it when that mark (btpo.xact) predates the oldest running xid (RecentXmin). (At the moment, I'm failing to work out why this is OK with scans from transactions that haven't allocated an xid, but I vaguely recall convincing myself it was fine at one point.) It would indeed also be enough to call GetLockConflicts(locktag-of-index, AccessExclusiveLock) and check whether any of the returned transactions have PGPROC.xmin below the mark. That's notably more expensive than just comparing RecentXmin, so I'm not sure how well it would pay off overall. However, it could only help us on the master. (Not strictly true, but any way I see to extend it to the standby has critical flaws.) On the master, we can see a conflicting transaction and put off reusing the page. By the time the record hits the standby, we have to apply it, and we might have a running transaction that will hold a lock on the index for the next, say, 72 hours. At such times, vacuum_defer_cleanup_age or hot_standby_feedback ought to prevent the recovery stall. This did lead me to realize that what we do in this regard on the standby can be considerably independent from what we do on the master. If fruitful, the standby can prove the absence of a scan holding a right-link in a completely different fashion. So, we *could* take the cleanup-lock approach on the standby without changing very much on the master. Well, I'm generally in favor of trying to fix this problem without changing what the master does. It's a weakness of our replication technology that the standby has no better way to cope with a cleanup operation on the master than to start killing queries, but then again it's a weakness of our MVCC technology that we don't reuse space quickly enough and end up with bloat. I hear a lot more complaints about the second weakness than I do about the first. At any rate, if taking a cleanup lock on the right-linked page on the standby is sufficient to fix the problem, that seems like a far superior solution in any case. Presumably the frequency of someone having a pin on that particular page will be far lower than any matching based on XID or heavyweight locks. And the vast majority of such pins should disappear before the startup process feels obliged to get out its big hammer. -- 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] FOREIGN TABLE doc fix
2011/6/9 Shigeru Hanada han...@metrosystems.co.jp: Attached patch includes fixes for FOREIGN TABLE documents: I committed the changes to ALTER FOREIGN TABLE, but I think the changes to CREATE FOREIGN TABLE need more thought. The first of the two hunks you've proposed to add doesn't seem necessary to me, and the second one seems like it belongs in a chapter on how to write a foreign data wrapper correctly, rather than here. -- 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
