Re: [HACKERS] Initial review of xslt with no limits patch
On Fri, Aug 06, 2010 at 05:57:37AM +0200, Pavel Stehule wrote: 2010/8/6 Andrew Dunstan and...@dunslane.net: On 08/05/2010 06:56 PM, Mike Fowler wrote: SELECT xslt_process('employeenamecim/nameage30/agepay400/pay/employee'::text, $$xsl:stylesheet xmlns:xsl=http://www.w3.org/1999/XSL/Transform; version=1.0 xsl:output method=xml omit-xml-declaration=yes indent=yes/ [snip] /xsl:stylesheet$$::text, 'n1=v1,n2=v2,n3=v3,n4=v4,n5=v5'::text) I haven't been paying attention to this, so sorry if this has been discussed before, but it just caught my eye. Why are we passing these params as a comma-separated list rather than as an array or as a variadic list of params? This looks rather ugly. What if you want to have a param that includes a comma? There is probably problem in pairs - label = value. Can be nice, if we can use a variadic functions for this, but I am afraid, ... using a variadic function isn't too much nice now some xslt_process(xmlsrc, 'n1=v1','n2=v2','n3=v3' This sounds like the perfect case for pulling hstore into core code. :) Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Initial review of xslt with no limits patch
2010/8/6 David Fetter da...@fetter.org: On Fri, Aug 06, 2010 at 05:57:37AM +0200, Pavel Stehule wrote: 2010/8/6 Andrew Dunstan and...@dunslane.net: On 08/05/2010 06:56 PM, Mike Fowler wrote: SELECT xslt_process('employeenamecim/nameage30/agepay400/pay/employee'::text, $$xsl:stylesheet xmlns:xsl=http://www.w3.org/1999/XSL/Transform; version=1.0 xsl:output method=xml omit-xml-declaration=yes indent=yes/ [snip] /xsl:stylesheet$$::text, 'n1=v1,n2=v2,n3=v3,n4=v4,n5=v5'::text) I haven't been paying attention to this, so sorry if this has been discussed before, but it just caught my eye. Why are we passing these params as a comma-separated list rather than as an array or as a variadic list of params? This looks rather ugly. What if you want to have a param that includes a comma? There is probably problem in pairs - label = value. Can be nice, if we can use a variadic functions for this, but I am afraid, ... using a variadic function isn't too much nice now some xslt_process(xmlsrc, 'n1=v1','n2=v2','n3=v3' This sounds like the perfect case for pulling hstore into core code. :) I afraid so integration of hstore can break and block work on real hash support. I would to have hash tables in core, but with usual features and usual syntax - like Perl or PHP Regards Pavel Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] MERGE Specification
On 06/08/10 04:39, Boxuan Zhai wrote: I have seen a lively discussion about the DO NOTING action in MERGE command. And, I think most people want it. So it will be added to my next patch. Before the implementation, I still have some questions to confirm: 1. If we have a DO NOTHING action specified, it should be the last WHEN clause. It must be of the NOT MATCHED cases, and it CANNOT have any additional action qualifications. Am I correct? It would be useful to specify it in WHEN MATCHED sometimes, and not necessarily the last. For example: MERGE INTO Stock S USING DailySales DS ON S.Item = DS.Item WHEN MATCHED AND (QtyOnHand ‐ QtySold = 0) THEN DELETE WHEN MATCHED THEN UPDATE SET QtyOnHand = QtyOnHand ‐ QtySold -- Don't add new inactive items to stock if not there already WHEN MATCHED AND (itemtype = 'inactive') THEN DO NOTHING WHEN NOT MATCHED THEN INSERT VALUES (Item, QtySold); It shouldn't be difficult to support DO NOTHING in all cases, right? 2. If no DO NOTHING specified, we will imply a INSERT DEFAULT VALUES action as the end of MERGE. My question is, is this action taken only for the NOT MATCHED tuples? If this is the case, then what about the MATCHED tuples that match not previous actions? Ignore them? That means we are in fact going to add two implicit WHEN clause: a) WHEN NOT MATCHED INSERT default values; b) WHEN MATCHED THEN DO NOTHING. OR, is the INSERT DEFAULT VALUES applied to ALL tuples not matter they are MATCHED or not? We'll need to figure out what the SQL standard says about this. I tried reading the spec but couldn't readily understand what the default action should be. Does someone else know that? What do other DBMSs do? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.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] MERGE Specification
On Fri, 2010-08-06 at 09:39 +0800, Boxuan Zhai wrote: Besides, (I mean no offense, but) can this method really avoid losing row? Not as you just specified, no. You need *both* actions of RAISE ERROR and DO NOTHING, or you may as well have neither. (1) Natural style allows missing rows if you are not careful - and also allows missing rows in future when COL is allowed to take value 'C', which may not have been originally considered when SQL first written WHEN NOT MATCHED AND COL = 'A' INSERT... WHEN NOT MATCHED AND COL = 'B' INSERT... (2) Shows code style required to explicitly avoid missing rows WHEN NOT MATCHED AND COL = 'A' INSERT... WHEN NOT MATCHED AND COL = 'B' INSERT... WHEN NOT MATCHED RAISE ERROR (3) More complex example, with explicit DO NOTHING, showing how it can provide well structured code WHEN NOT MATCHED AND COL = 'A' DO NOTHING WHEN NOT MATCHED AND COL = 'B' INSERT... WHEN NOT MATCHED RAISE ERROR So DO NOTHING is the default and implies silently ignoring rows. RAISE ERROR is the opposite. Coding for those seems very easy, its just a question of should we do it?. DB2 has it; SQL:2008 does not. But then SQL:2008 followed the DB2 introduction of AND clauses, and SQL:2011 has so far followed the DB2 introduction of DELETE action also. Given that Peter is now attending SQL Standards meetings, I would suggest we leave out my suggestion above, for now. We have time to raise this at standards meetings and influence the outcome and then follow later. There is a workaround: WHEN NOT MATCHED AND COL = 'A' DO NOTHING WHEN NOT MATCHED AND COL = 'B' INSERT... WHEN NOT MATCHED AND TRUE INSERT INTO ERROR_TABLE (errortext); where ERROR_TABLE has an INSERT trigger which throws an ERROR with given text. SQL:2011 makes no mention of how MERGE should react to statement level triggers. MERGE is not a trigger action even. Given considerable confusion in this area, IMHO we should just say the MERGE does not call statement triggers at all, of any kind. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] pgsql-hack...@news.hub.org 21% OFF on Pfizer!
http://groups.yahoo.com/group/igodsinkma/message die ihre Stellung als Herren der Erde nur der Genialitat und dem Mute verdanken, mit dem sie sich diese zu erkampfen und zu wahren wissen; vor unserer deutschen Nachwelt aber, insofern wir keines Burgers Blut vergossen, aus dem nicht tausend andere der Nachwelt geschenkt werden. Der Grund und Boden, auf dem dereinst deutsche Bauerngeschlecht -- 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] MERGE Specification
On 06/08/10 10:12, Simon Riggs wrote: So DO NOTHING is the default and implies silently ignoring rows. RAISE ERROR is the opposite. Coding for those seems very easy, its just a question of should we do it?. DB2 has it; SQL:2008 does not. But then SQL:2008 followed the DB2 introduction of AND clauses, and SQL:2011 has so far followed the DB2 introduction of DELETE action also. I see neither DO NOTHING or RAISE ERROR in the documentation of DB2, Oracle, or MSSQL server. Given that Peter is now attending SQL Standards meetings, I would suggest we leave out my suggestion above, for now. We have time to raise this at standards meetings and influence the outcome and then follow later. Ok, fair enough. SQL:2011 makes no mention of how MERGE should react to statement level triggers. MERGE is not a trigger action even. Given considerable confusion in this area, IMHO we should just say the MERGE does not call statement triggers at all, of any kind. IMO the UPDATE/DELETE/INSERT actions should fire the respective statement level triggers, but the MERGE itself should not. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.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] PL/pgSQL EXECUTE '..' USING with unknown
On 06/08/10 01:13, Tom Lane wrote: Andrew Dunstanand...@dunslane.net writes: On 08/05/2010 05:11 PM, Tom Lane wrote: This example doesn't seem terribly compelling. Why would you bother using USING with constants? In a more complex example you might use $1 in more than one place in the query. Well, that's better than no justification, but it's still pretty weak. A bigger problem is that doing anything like this will require reversing the logical path of causation in EXECUTE USING. Right now, we evaluate the USING expressions first, and then their types feed forward into parsing the EXECUTE string. What Heikki is suggesting requires reversing that, at least to some extent. I'm not convinced it's possible without breaking other cases that are more important. One approach is to handle the conversion from unknown to the right data type transparently in the backend. Attached patch adds a coerce-param-hook for fixed params that returns a CoerceViaIO node to convert the param to the right type at runtime. That's quite similar to the way unknown constants are handled. The patch doesn't currently check that a parameter is only resolved to one type in the same query, but that can be added. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com diff --git a/src/backend/parser/parse_param.c b/src/backend/parser/parse_param.c index 20abf00..f4381fc 100644 --- a/src/backend/parser/parse_param.c +++ b/src/backend/parser/parse_param.c @@ -55,6 +55,9 @@ static Node *variable_paramref_hook(ParseState *pstate, ParamRef *pref); static Node *variable_coerce_param_hook(ParseState *pstate, Param *param, Oid targetTypeId, int32 targetTypeMod, int location); +static Node *fixed_coerce_param_hook(ParseState *pstate, Param *param, + Oid targetTypeId, int32 targetTypeMod, + int location); static bool check_parameter_resolution_walker(Node *node, ParseState *pstate); @@ -71,7 +74,7 @@ parse_fixed_parameters(ParseState *pstate, parstate-numParams = numParams; pstate-p_ref_hook_state = (void *) parstate; pstate-p_paramref_hook = fixed_paramref_hook; - /* no need to use p_coerce_param_hook */ + pstate-p_coerce_param_hook = fixed_coerce_param_hook; } /* @@ -170,6 +173,43 @@ variable_paramref_hook(ParseState *pstate, ParamRef *pref) return (Node *) param; } +static Node * +fixed_coerce_param_hook(ParseState *pstate, Param *param, + Oid targetTypeId, int32 targetTypeMode, + int location) +{ + if (param-paramkind == PARAM_EXTERN param-paramtype == UNKNOWNOID) + { + /* + * Input is a Param of previously undetermined type, and we want to + * update our knowledge of the Param's type. + */ + FixedParamState *parstate = (FixedParamState *) pstate-p_ref_hook_state; + int paramno = param-paramid; + CoerceViaIO *iocoerce; + + if (paramno = 0 || /* shouldn't happen, but... */ + paramno parstate-numParams) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_PARAMETER), + errmsg(there is no parameter $%d, paramno), + parser_errposition(pstate, param-location))); + + /* Build a CoerceViaIO node */ + iocoerce = makeNode(CoerceViaIO); + iocoerce-arg = (Expr *) param; + iocoerce-resulttype = targetTypeId; + iocoerce-coerceformat = COERCE_IMPLICIT_CAST; + iocoerce-location = location; + + return (Node *) iocoerce; + } + + /* Else signal to proceed with normal coercion */ + return NULL; +} + + /* * Coerce a Param to a query-requested datatype, in the varparams case. */ -- 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] PL/pgSQL EXECUTE '..' USING with unknown
On 06/08/10 01:13, Tom Lane wrote: Andrew Dunstanand...@dunslane.net writes: On 08/05/2010 05:11 PM, Tom Lane wrote: This example doesn't seem terribly compelling. Why would you bother using USING with constants? In a more complex example you might use $1 in more than one place in the query. Well, that's better than no justification, but it's still pretty weak. A bigger problem is that doing anything like this will require reversing the logical path of causation in EXECUTE USING. Right now, we evaluate the USING expressions first, and then their types feed forward into parsing the EXECUTE string. What Heikki is suggesting requires reversing that, at least to some extent. I'm not convinced it's possible without breaking other cases that are more important. One approach is to handle the conversion from unknown to the right data type transparently in the backend. Attached patch adds a coerce-param-hook for fixed params that returns a CoerceViaIO node to convert the param to the right type at runtime. That's quite similar to the way unknown constants are handled. The patch doesn't currently check that a parameter is only resolved to one type in the same query, but that can be added. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com diff --git a/src/backend/parser/parse_param.c b/src/backend/parser/parse_param.c index 20abf00..f4381fc 100644 --- a/src/backend/parser/parse_param.c +++ b/src/backend/parser/parse_param.c @@ -55,6 +55,9 @@ static Node *variable_paramref_hook(ParseState *pstate, ParamRef *pref); static Node *variable_coerce_param_hook(ParseState *pstate, Param *param, Oid targetTypeId, int32 targetTypeMod, int location); +static Node *fixed_coerce_param_hook(ParseState *pstate, Param *param, + Oid targetTypeId, int32 targetTypeMod, + int location); static bool check_parameter_resolution_walker(Node *node, ParseState *pstate); @@ -71,7 +74,7 @@ parse_fixed_parameters(ParseState *pstate, parstate-numParams = numParams; pstate-p_ref_hook_state = (void *) parstate; pstate-p_paramref_hook = fixed_paramref_hook; - /* no need to use p_coerce_param_hook */ + pstate-p_coerce_param_hook = fixed_coerce_param_hook; } /* @@ -170,6 +173,43 @@ variable_paramref_hook(ParseState *pstate, ParamRef *pref) return (Node *) param; } +static Node * +fixed_coerce_param_hook(ParseState *pstate, Param *param, + Oid targetTypeId, int32 targetTypeMode, + int location) +{ + if (param-paramkind == PARAM_EXTERN param-paramtype == UNKNOWNOID) + { + /* + * Input is a Param of previously undetermined type, and we want to + * update our knowledge of the Param's type. + */ + FixedParamState *parstate = (FixedParamState *) pstate-p_ref_hook_state; + int paramno = param-paramid; + CoerceViaIO *iocoerce; + + if (paramno = 0 || /* shouldn't happen, but... */ + paramno parstate-numParams) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_PARAMETER), + errmsg(there is no parameter $%d, paramno), + parser_errposition(pstate, param-location))); + + /* Build a CoerceViaIO node */ + iocoerce = makeNode(CoerceViaIO); + iocoerce-arg = (Expr *) param; + iocoerce-resulttype = targetTypeId; + iocoerce-coerceformat = COERCE_IMPLICIT_CAST; + iocoerce-location = location; + + return (Node *) iocoerce; + } + + /* Else signal to proceed with normal coercion */ + return NULL; +} + + /* * Coerce a Param to a query-requested datatype, in the varparams case. */ -- 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] MERGE Specification
On Fri, 2010-08-06 at 10:28 +0300, Heikki Linnakangas wrote: SQL:2011 makes no mention of how MERGE should react to statement level triggers. MERGE is not a trigger action even. Given considerable confusion in this area, IMHO we should just say the MERGE does not call statement triggers at all, of any kind. IMO the UPDATE/DELETE/INSERT actions should fire the respective statement level triggers, but the MERGE itself should not. When, and how? If an UPDATE is mentioned 5 times, do we call the trigger 5 times? What happens if none of the UPDATEs are ever executed? Best explain exactly what you mean. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Training and 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] Concurrent MERGE
On Thu, 2010-08-05 at 19:39 +0300, Heikki Linnakangas wrote: On 05/08/10 18:43, Simon Riggs wrote: Do we still need me to work on concurrent MERGE, or is that included in the current MERGE patch (can't see it), or ... It's not in the current MERGE patch. OK, I will work on it, eventually, in this release. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Training and 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] Proposal / proof of concept: Triggers on VIEWs
On 4 August 2010 15:08, Marko Tiikkaja marko.tiikk...@cs.helsinki.fi wrote: On 8/4/10 5:03 PM +0300, Dean Rasheed wrote: On 4 August 2010 14:43, Marko Tiikkajamarko.tiikk...@cs.helsinki.fi wrote: I'm not sure I understand. RETURNING in DELETE on a table fetches the old value after it was DELETEd, so it really is what the tuple was before the DLETE, not what is seen by the snapshot. In a BEFORE DELETE trigger, the row is always locked so it can't change after the trigger is fired. Ah, I think I mis-understood. If I understand what you're saying correctly, you're worried that the row might have been modified in the same query, prior to being deleted, and you want RETURNING to return the updated value, as it was when it was deleted. I'm mainly concerned about concurrently running transactions. Sorry for the delay replying. Once again, I think I mis-understood your point. I think that the database can't really lock anything before firing the trigger because the view might contain grouping/aggregates or even not be based on any real tables at all, so it would be impossible to work out what to lock. Thus it would be up to the trigger function to get this right. In the simplest case, for a DELETE, this might look something like: CREATE OR REPLACE FUNCTION instead_of_delete_trig_fn() RETURNS trigger AS $$ BEGIN DELETE FROM base_table WHERE pk = OLD.pk; IF NOT FOUND THEN RETURN NULL; END IF; RETURN OLD; END; $$ LANGUAGE plpgsql; If 2 users try to delete the same row, the second would block until the first user's transaction finished, and if the first user committed, the second user's trigger would return NULL, which the database would signal as no rows deleted. 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] MERGE Specification
On Fri, 2010-08-06 at 10:28 +0300, Heikki Linnakangas wrote: On 06/08/10 10:12, Simon Riggs wrote: So DO NOTHING is the default and implies silently ignoring rows. RAISE ERROR is the opposite. Coding for those seems very easy, its just a question of should we do it?. DB2 has it; SQL:2008 does not. But then SQL:2008 followed the DB2 introduction of AND clauses, and SQL:2011 has so far followed the DB2 introduction of DELETE action also. I see neither DO NOTHING or RAISE ERROR in the documentation of DB2, Oracle, or MSSQL server. Agreed, Oracle and MSSQL server does not have these. However, DB2 very clearly does have these features * SIGNAL which raises an error and can be used in place of any action, at any point in sequence of WHEN clauses. DB2 already supports SIGNAL as part of SQL/PSM, which we do not, so RAISE ERROR was the nearest equivalent command for PostgreSQL. * ELSE IGNORE which does same thing as DO NOTHING, except it must always be last statement in a sequence of WHEN clauses. DO NOTHING is already a phrase with exactly this meaning in PostgreSQL, so I suggest that. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Training and 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] Proposal / proof of concept: Triggers on VIEWs
aoa how can i connect my postgres database to manifold? Regards? On Fri, Aug 6, 2010 at 12:49 PM, Dean Rasheed dean.a.rash...@gmail.comwrote: On 4 August 2010 15:08, Marko Tiikkaja marko.tiikk...@cs.helsinki.fi wrote: On 8/4/10 5:03 PM +0300, Dean Rasheed wrote: On 4 August 2010 14:43, Marko Tiikkajamarko.tiikk...@cs.helsinki.fi wrote: I'm not sure I understand. RETURNING in DELETE on a table fetches the old value after it was DELETEd, so it really is what the tuple was before the DLETE, not what is seen by the snapshot. In a BEFORE DELETE trigger, the row is always locked so it can't change after the trigger is fired. Ah, I think I mis-understood. If I understand what you're saying correctly, you're worried that the row might have been modified in the same query, prior to being deleted, and you want RETURNING to return the updated value, as it was when it was deleted. I'm mainly concerned about concurrently running transactions. Sorry for the delay replying. Once again, I think I mis-understood your point. I think that the database can't really lock anything before firing the trigger because the view might contain grouping/aggregates or even not be based on any real tables at all, so it would be impossible to work out what to lock. Thus it would be up to the trigger function to get this right. In the simplest case, for a DELETE, this might look something like: CREATE OR REPLACE FUNCTION instead_of_delete_trig_fn() RETURNS trigger AS $$ BEGIN DELETE FROM base_table WHERE pk = OLD.pk; IF NOT FOUND THEN RETURN NULL; END IF; RETURN OLD; END; $$ LANGUAGE plpgsql; If 2 users try to delete the same row, the second would block until the first user's transaction finished, and if the first user committed, the second user's trigger would return NULL, which the database would signal as no rows deleted. 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: Review: Re: [PATCH] Re: [HACKERS] Adding xpath_exists function
On 06/08/10 05:38, Peter Eisentraut wrote: On tis, 2010-07-27 at 16:33 -0700, David Fetter wrote: * Do we already have it? Not really. There are kludges to accomplish these things, but they're available mostly in the sense that a general-purpose language allows you to write code to do anything a Turing machine can do. I think this has been obsoleted by the xmlexists patch In many ways yes. The only surviving difference is that xpath_exists has support for namespaces and xmlexists does not as the grammar expects namespaces to be handled in the xquery. So if people expect namespace support to be useful that having both functions is useful until I (or someone who works faster than me) get xquery going. If the patch is to be committed, does it make sense for me to refine it such that it uses the new xpath internal function you extracted in the xmlexists patch? Regards, -- Mike Fowler Registered Linux user: 379787 -- 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] MERGE Specification
On Fri, Aug 6, 2010 at 3:41 PM, Simon Riggs si...@2ndquadrant.com wrote: On Fri, 2010-08-06 at 10:28 +0300, Heikki Linnakangas wrote: SQL:2011 makes no mention of how MERGE should react to statement level triggers. MERGE is not a trigger action even. Given considerable confusion in this area, IMHO we should just say the MERGE does not call statement triggers at all, of any kind. IMO the UPDATE/DELETE/INSERT actions should fire the respective statement level triggers, but the MERGE itself should not. When, and how? If an UPDATE is mentioned 5 times, do we call the trigger 5 times? My current process for BEFOR / AFTER STATEMENT trigger on MERGE is to fire the triggers for all action types that appears in the command, unless it is replaced by a INSTEAD rule. But the triggers for one action type will be fired only once. That means you will get both UPDATE and INSERT triggers be activated for only once if you are executing a MERGE command with 5 UPDATEs and 10 INSERTs. What happens if none of the UPDATEs are ever executed? The triggers (I mean triggers for statement) will be fired anyway even the UPDATE action matches no tuple. This is not for MERGE only. If you update a table with the command UPDATE foo SET ... WHERE false; It will also fire the STATEMENT triggers of UPDATE type on foo (I think so). And, even not been asked, I want to say that, in current implementation of MERGE, the row level triggers are fired by the actions that take the tuples. If one tuple is caught by an UPDATE action, then the UPDATE row trigger will be fired on this tuple. If it is handled by INSERT action, then the INSRET row triggers are on. Hope you agree with my designs. Best explain exactly what you mean. -- Simon Riggs www.2ndQuadrant.com http://www.2ndquadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services
Re: [HACKERS] MERGE Specification
On Fri, Aug 6, 2010 at 3:53 PM, Simon Riggs si...@2ndquadrant.com wrote: On Fri, 2010-08-06 at 10:28 +0300, Heikki Linnakangas wrote: On 06/08/10 10:12, Simon Riggs wrote: So DO NOTHING is the default and implies silently ignoring rows. RAISE ERROR is the opposite. Coding for those seems very easy, its just a question of should we do it?. DB2 has it; SQL:2008 does not. But then SQL:2008 followed the DB2 introduction of AND clauses, and SQL:2011 has so far followed the DB2 introduction of DELETE action also. I see neither DO NOTHING or RAISE ERROR in the documentation of DB2, Oracle, or MSSQL server. Agreed, Oracle and MSSQL server does not have these. However, DB2 very clearly does have these features * SIGNAL which raises an error and can be used in place of any action, at any point in sequence of WHEN clauses. DB2 already supports SIGNAL as part of SQL/PSM, which we do not, so RAISE ERROR was the nearest equivalent command for PostgreSQL. * ELSE IGNORE which does same thing as DO NOTHING, except it must always be last statement in a sequence of WHEN clauses. DO NOTHING is already a phrase with exactly this meaning in PostgreSQL, so I suggest that. So, we need to add both DO NOTHING and RAISE ERROR actions in the MERGE command now !? What will RAISE ERROR do? To stop the whole MERGE command OR, just throw an error notice for the row and move on. -- Simon Riggs www.2ndQuadrant.com http://www.2ndquadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services
Re: [HACKERS] review: xml_is_well_formed
On 03/08/10 16:15, Peter Eisentraut wrote: On lör, 2010-07-31 at 13:40 -0400, Robert Haas wrote: Well-formedness should probably only allow XML documents. I think the point of this function is to determine whether a cast to xml will throw an error. The behavior should probably match exactly whatever test would be applied there. Maybe there should be xml_is_well_formed() xml_is_well_formed_document() xml_is_well_formed_content() I agree that consistency with SQL/XML is desirable, but for someone coming from the outside, the unqualified claim that 'foo' is well-formed XML might sound suspicious. What about making the function sensitive to the XML OPTION, such that: test=# SET xmloption TO DOCUMENT; SET text=# SELECT xml_is_well_formed('foo'); xml_is_well_formed f (1 row) test=# SET xmloption TO CONTENT; SET text=# SELECT xml_is_well_formed('foo'); xml_is_well_formed t (1 row) with the inverse for DOCUMENTS? To me this makes the most sense as it makes the function behave much more like the other xml functions. -- Mike Fowler Registered Linux user: 379787 -- 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] MERGE Specification
On Fri, 2010-08-06 at 16:26 +0800, Boxuan Zhai wrote: So, we need to add both DO NOTHING and RAISE ERROR actions in the MERGE command now !? What will RAISE ERROR do? Let's get the rest of it working first. This would be a later extension, though I think an important one for our developers. To stop the whole MERGE command OR, just throw an error notice for the row and move on. As you say, it would be even better to be able to report errors in some way and move onto next row. NOTICE is not the way though. Maybe one of the actions would be to EXECUTE a procedure, so we can call an error logging function. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Training and 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] review: xml_is_well_formed
On Fri, Aug 6, 2010 at 4:28 AM, Mike Fowler m...@mlfowler.com wrote: On 03/08/10 16:15, Peter Eisentraut wrote: On lör, 2010-07-31 at 13:40 -0400, Robert Haas wrote: Well-formedness should probably only allow XML documents. I think the point of this function is to determine whether a cast to xml will throw an error. The behavior should probably match exactly whatever test would be applied there. Maybe there should be xml_is_well_formed() xml_is_well_formed_document() xml_is_well_formed_content() I agree that consistency with SQL/XML is desirable, but for someone coming from the outside, the unqualified claim that 'foo' is well-formed XML might sound suspicious. What about making the function sensitive to the XML OPTION, such that: test=# SET xmloption TO DOCUMENT; SET text=# SELECT xml_is_well_formed('foo'); xml_is_well_formed f (1 row) That will make using this function a huge hassle, won't it? Functions that do different things depending on GUC settings are usually troublesome. Having three functions would be more sensible if we need all three behaviors, but I don't see why we do. Or perhaps it could return a string instead of a boolean: content, document, or NULL if it's neither. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] default of max_stack_depth
Hi, The document says that the max_stack_depth is 2MB by default. OTOH, in the source code, the variable max_stack_depth is initialized with 100 (kB), and guc.c also uses 100 as the default. Why? This seems confusing to me though I know that InitializeGUCOptions() sets max_stack_depth to 2MB. 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] review: xml_is_well_formed
On 06/08/10 12:31, Robert Haas wrote: Maybe there should be xml_is_well_formed() xml_is_well_formed_document() xml_is_well_formed_content() I agree that consistency with SQL/XML is desirable, but for someone coming from the outside, the unqualified claim that 'foo' is well-formed XML might sound suspicious. What about making the function sensitive to the XML OPTION, such that: test=# SET xmloption TO DOCUMENT; SET text=# SELECT xml_is_well_formed('foo'); xml_is_well_formed f (1 row) That will make using this function a huge hassle, won't it? Functions that do different things depending on GUC settings are usually troublesome. Having three functions would be more sensible if we need all three behaviors, but I don't see why we do. Or perhaps it could return a string instead of a boolean: content, document, or NULL if it's neither. I like the sound of that. In fact this helps workaround the IS DOCUMENT and IS CONTENT limitations such that you can you can select only content, only documents or both is you use IS NOT NULL. Unless anyone sees a reason that this function needs to remain a boolean function, I'll rework the patch over the weekend. Regards, -- Mike Fowler Registered Linux user: 379787 -- 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] default of max_stack_depth
Fujii Masao masao.fu...@gmail.com writes: The document says that the max_stack_depth is 2MB by default. OTOH, in the source code, the variable max_stack_depth is initialized with 100 (kB), and guc.c also uses 100 as the default. Why? The initial value needs to be small until we have been able to probe rlimit and figure out what is 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
Re: [HACKERS] Initial review of xslt with no limits patch
On 08/06/2010 02:29 AM, Pavel Stehule wrote: 2010/8/6 David Fetterda...@fetter.org: On Fri, Aug 06, 2010 at 05:57:37AM +0200, Pavel Stehule wrote: 2010/8/6 Andrew Dunstanand...@dunslane.net: On 08/05/2010 06:56 PM, Mike Fowler wrote: SELECT xslt_process('employeenamecim/nameage30/agepay400/pay/employee'::text, $$xsl:stylesheet xmlns:xsl=http://www.w3.org/1999/XSL/Transform; version=1.0 xsl:output method=xml omit-xml-declaration=yes indent=yes/ [snip] /xsl:stylesheet$$::text, 'n1=v1,n2=v2,n3=v3,n4=v4,n5=v5'::text) I haven't been paying attention to this, so sorry if this has been discussed before, but it just caught my eye. Why are we passing these params as a comma-separated list rather than as an array or as a variadic list of params? This looks rather ugly. What if you want to have a param that includes a comma? There is probably problem in pairs - label = value. Can be nice, if we can use a variadic functions for this, but I am afraid, ... using a variadic function isn't too much nice now some xslt_process(xmlsrc, 'n1=v1','n2=v2','n3=v3' This sounds like the perfect case for pulling hstore into core code. :) I afraid so integration of hstore can break and block work on real hash support. I would to have hash tables in core, but with usual features and usual syntax - like Perl or PHP Can we just keep this discussion within reasonable bounds? The issue is not hstore or other hashes, but how to do the param list for xslt sanely given what we have now. A variadic list will be much nicer than what is currently proposed. 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] Initial review of xslt with no limits patch
On 06/08/10 15:08, Andrew Dunstan wrote: On 08/06/2010 02:29 AM, Pavel Stehule wrote: 2010/8/6 David Fetterda...@fetter.org: On Fri, Aug 06, 2010 at 05:57:37AM +0200, Pavel Stehule wrote: 2010/8/6 Andrew Dunstanand...@dunslane.net: On 08/05/2010 06:56 PM, Mike Fowler wrote: SELECT xslt_process('employeenamecim/nameage30/agepay400/pay/employee'::text, $$xsl:stylesheet xmlns:xsl=http://www.w3.org/1999/XSL/Transform; version=1.0 xsl:output method=xml omit-xml-declaration=yes indent=yes/ [snip] /xsl:stylesheet$$::text, 'n1=v1,n2=v2,n3=v3,n4=v4,n5=v5'::text) I haven't been paying attention to this, so sorry if this has been discussed before, but it just caught my eye. Why are we passing these params as a comma-separated list rather than as an array or as a variadic list of params? This looks rather ugly. What if you want to have a param that includes a comma? There is probably problem in pairs - label = value. Can be nice, if we can use a variadic functions for this, but I am afraid, ... using a variadic function isn't too much nice now some xslt_process(xmlsrc, 'n1=v1','n2=v2','n3=v3' This sounds like the perfect case for pulling hstore into core code. :) I afraid so integration of hstore can break and block work on real hash support. I would to have hash tables in core, but with usual features and usual syntax - like Perl or PHP Can we just keep this discussion within reasonable bounds? The issue is not hstore or other hashes, but how to do the param list for xslt sanely given what we have now. A variadic list will be much nicer than what is currently proposed. cheers andrew +1 Variadic seems the most sensible to me anyways. However the more urgent problem is, pending someone spotting an error in my ways, neither the existing code or the patched code appear able to evaluate the parameters. Note than in my example I supplied a default value for the fifth parameter and not even that value is appearing in the outputted XML. Regards, -- Mike Fowler Registered Linux user: 379787 -- 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] default of max_stack_depth
On Fri, Aug 6, 2010 at 11:02 PM, Tom Lane t...@sss.pgh.pa.us wrote: Fujii Masao masao.fu...@gmail.com writes: The document says that the max_stack_depth is 2MB by default. OTOH, in the source code, the variable max_stack_depth is initialized with 100 (kB), and guc.c also uses 100 as the default. Why? The initial value needs to be small until we have been able to probe rlimit and figure out what is safe. Thanks! How about adding the comment about that as follows? *** a/src/backend/utils/misc/guc.c --- b/src/backend/utils/misc/guc.c *** *** 1520,1525 static struct config_int ConfigureNamesInt[] = --- 1520,1531 16384, 1024, MAX_KILOBYTES, NULL, NULL }, + /* +* Note: the real default of max_stack_depth is calculated in +* InitializeGUCOptions(). We use 100 as the sufficiently small +* initial value until we have been able to probe rlimit and +* figure out what is safe. +*/ { {max_stack_depth, PGC_SUSET, RESOURCES_MEM, gettext_noop(Sets the maximum stack depth, in kilobytes.), 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] [ADMIN] postgres 9.0 crash when bringing up hot standby
On Fri, Aug 6, 2010 at 10:10 PM, Alanoly Andrews alano...@invera.com wrote: I’m testing “hot standby” using “streaming WAL records”. On trying to bring up the hot standby, I see the following error in the log: Thanks for the report! LOG: database system was interrupted; last known up at 2010-08-05 14:46:36 LOG: entering standby mode LOG: restored log file 00010007 from archive LOG: redo starts at 0/720 LOG: consistent recovery state reached at 0/800 LOG: database system is ready to accept read only connections cp: /pgarclog/pg1/00010008: A file or directory in the path name does not exist. LOG: WAL receiver process (PID 1073206) was terminated by signal 11 LOG: terminating any other active server processes There is a core dump. The debugger indicates the crash sequence as follows: (dbx) where _alloc_initial_pthread(??) at 0x949567c __pth_init(??) at 0x9493ba4 uload(??, ??, ??, ??, ??, ??, ??, ??) at 0x9fff0001954 load_64.load(??, ??, ??) at 0x904686c loadAndInit() at 0x947bd7c dlopen(??, ??) at 0x911cc4c internal_load_library(libname = /apps/pg_9.0_b4/lib/postgresql/libpqwalreceiver.so), line 234 in dfmgr.c load_file(filename = libpqwalreceiver, restricted = '\0'), line 156 in dfmgr.c WalReceiverMain(), line 248 in walreceiver.c AuxiliaryProcessMain(argc = 2, argv = 0x0fffa8b8), line 428 in bootstrap.c StartChildProcess(type = WalReceiverProcess), line 4405 in postmaster.c sigusr1_handler(postgres_signal_arg = 30), line 4227 in postmaster.c __fd_select(??, ??, ??, ??, ??) at 0x911805c postmaster.select(__fds = 5, __readlist = 0x0fffd0a8, __writelist = (nil), __exceptlist = (nil), __timeout = 0x00c0), line 229 in time.h unnamed block in ServerLoop(), line 1391 in postmaster.c unnamed block in ServerLoop(), line 1391 in postmaster.c ServerLoop(), line 1391 in postmaster.c PostmasterMain(argc = 1, argv = 0x0001102aa4b0), line 1092 in postmaster.c main(argc = 1, argv = 0x0001102aa4b0), line 188 in main.c Any pointers on how to resolve the issue will be much appreciated. Sorry, I have no idea what's wrong :( Is the simple LOAD command successful on your AIX? 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] CommitFest 2010-07 week three progress report
Kevin Grittner kevin.gritt...@wicourts.gov wrote: With only ten days to go, in order to leave time for committers to do their thing, we need to be wrapping up the remaining patches. I think we look pretty good. Of the remaining six patches, two are Work in Progress, so are not expected to go to a committer; three involve a committer, so I figure they can decide when and if it's time to return or move them, which just leaves one which is down to tweaking docs. How embarrassing -- I miscounted. It appears I counted Peter's WiP patch in two categories and missed counting the unlimited parameters for xslt_process in the above paragraph. (An omission which jumped out at me when reading this morning's posts.) Mike Fowler's latest post says neither the existing code or the patched code appear able to evaluate the parameters. Is it time to mark this Returned with Feedback and hope for a new patch in the next CF? -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] default of max_stack_depth
Fujii Masao masao.fu...@gmail.com writes: On Fri, Aug 6, 2010 at 11:02 PM, Tom Lane t...@sss.pgh.pa.us wrote: The initial value needs to be small until we have been able to probe rlimit and figure out what is safe. Thanks! How about adding the comment about that as follows? I added this: /* * We use the hopefully-safely-small value of 100kB as the compiled-in * default for max_stack_depth. InitializeGUCOptions will increase it if * possible, depending on the actual platform-specific stack limit. */ although I don't entirely see the point. We are certainly not going to comment every variable whose compiled-in default gets changed by later processing. 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] Review: Row-level Locks SERIALIZABLE transactions, postgres vs. Oracle
On Aug3, 2010, at 00:43 , Florian Pflug wrote: Sounds good. That'll also give me some time to test the RI trigger infrastructure now that I've removed the crosscheck code. Ok, I've found some time do run some additional tests. I've created a small test suite to compare the behaviour of native (cascading) FK constraints to an PLPGSQL implementation. There is a dedicated child table (native_child respectively plpgsql_child) for each of the two FK implementation. Into both child rows for random parents are inserted, creating the parent if it does not already exists. Concurrently, random parent rows are deleted. The number of successful inserts into native_child respectively plpgsql_child and the number of successfull deletes are counted, as well as the number of transactions failed due to either a serialization failure or a race condition during the insert (unique_violation or foreign_key_violation). To verify that things behave as expected, the test suite reports a histogram of the number of parents over the child count after the test completes. The expected distribution is output alongside that histogram, assuming that the number of parents with N children follows an exponential distribution. I've pushed the test suite to g...@github.com:fgp/fk_concurrency.git and put a summary of the results of my test runs on http://wiki.github.com/fgp/fk_concurrency/results. The results look good. There is no significant change in the behaviour of FKs between HEAD and HEAD+serializable_row_locks, and no significant difference between the native and plpgsql implementation. As expected, the plpgsql implementation fails on an unpatched HEAD, aborting with the error database inconsistent pretty quickly. I'd be interesting to run the plpgsql implementation with the SHARE locking removed against HEAD+true_serializability to see if that changes the number of serialization_failures that occur. I'll do that, but I'll have to wait a bit for another spare time window to open. best regards, Florian Pflug -- 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] [ADMIN] postgres 9.0 crash when bringing up hot standby
Thanks. Yes, the LOAD command does work, on another database cluster on the same AIX machine. -Original Message- From: Fujii Masao [mailto:masao.fu...@gmail.com] Sent: Friday, August 06, 2010 10:31 AM To: Alanoly Andrews Cc: pgsql-ad...@postgresql.org; PostgreSQL-development Subject: Re: [ADMIN] postgres 9.0 crash when bringing up hot standby On Fri, Aug 6, 2010 at 10:10 PM, Alanoly Andrews alano...@invera.com wrote: I'm testing hot standby using streaming WAL records. On trying to bring up the hot standby, I see the following error in the log: Thanks for the report! LOG: database system was interrupted; last known up at 2010-08-05 14:46:36 LOG: entering standby mode LOG: restored log file 00010007 from archive LOG: redo starts at 0/720 LOG: consistent recovery state reached at 0/800 LOG: database system is ready to accept read only connections cp: /pgarclog/pg1/00010008: A file or directory in the path name does not exist. LOG: WAL receiver process (PID 1073206) was terminated by signal 11 LOG: terminating any other active server processes There is a core dump. The debugger indicates the crash sequence as follows: (dbx) where _alloc_initial_pthread(??) at 0x949567c __pth_init(??) at 0x9493ba4 uload(??, ??, ??, ??, ??, ??, ??, ??) at 0x9fff0001954 load_64.load(??, ??, ??) at 0x904686c loadAndInit() at 0x947bd7c dlopen(??, ??) at 0x911cc4c internal_load_library(libname = /apps/pg_9.0_b4/lib/postgresql/libpqwalreceiver.so), line 234 in dfmgr.c load_file(filename = libpqwalreceiver, restricted = '\0'), line 156 in dfmgr.c WalReceiverMain(), line 248 in walreceiver.c AuxiliaryProcessMain(argc = 2, argv = 0x0fffa8b8), line 428 in bootstrap.c StartChildProcess(type = WalReceiverProcess), line 4405 in postmaster.c sigusr1_handler(postgres_signal_arg = 30), line 4227 in postmaster.c __fd_select(??, ??, ??, ??, ??) at 0x911805c postmaster.select(__fds = 5, __readlist = 0x0fffd0a8, __writelist = (nil), __exceptlist = (nil), __timeout = 0x00c0), line 229 in time.h unnamed block in ServerLoop(), line 1391 in postmaster.c unnamed block in ServerLoop(), line 1391 in postmaster.c ServerLoop(), line 1391 in postmaster.c PostmasterMain(argc = 1, argv = 0x0001102aa4b0), line 1092 in postmaster.c main(argc = 1, argv = 0x0001102aa4b0), line 188 in main.c Any pointers on how to resolve the issue will be much appreciated. Sorry, I have no idea what's wrong :( Is the simple LOAD command successful on your AIX? Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center This e-mail may be privileged and/or confidential, and the sender does not waive any related rights and obligations. Any distribution, use or copying of this e-mail or the information it contains by other than an intended recipient is unauthorized. If you received this e-mail in error, please advise me (by return e-mail or otherwise) immediately. Ce courriel est confidentiel et protégé. L'expéditeur ne renonce pas aux droits et obligations qui s'y rapportent. Toute diffusion, utilisation ou copie de ce message ou des renseignements qu'il contient par une personne autre que le (les) destinataire(s) désigné(s) est interdite. Si vous recevez ce courriel par erreur, veuillez m'en aviser immédiatement, par retour de courriel ou par un autre moyen. -- 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] refactoring comment.c
Excerpts from Robert Haas's message of vie ago 06 11:02:58 -0400 2010: Any comments? (ha ha ha...) Interesting idea. The patch looks fine on a quick once-over. Two small things: this comment +/* + * Databases, tablespaces, and roles are cluster-wide objects, so any + * comments on those objects are record in the shared pg_shdescription + * catalog. Comments on all other objects are recorded in pg_description. + */ says record where it should say recorded. Also, it strikes me that perhaps the ObjectAddress struct definition should be moved to the new header file; seems a more natural place for it (but then, it seems like a lot of files will need to include the new header, so perhaps that should be left to a subsequent patch). Thirdly, is it just me or just could replace a lot of code in DropFoo functions with this new auxiliary code? Seems it's just missing missing_ok support ... -- Álvaro Herrera alvhe...@commandprompt.com 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] refactoring comment.c
On Fri, Aug 6, 2010 at 11:36 AM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Robert Haas's message of vie ago 06 11:02:58 -0400 2010: Any comments? (ha ha ha...) Interesting idea. The patch looks fine on a quick once-over. Thanks for taking a look. Two small things: this comment + /* + * Databases, tablespaces, and roles are cluster-wide objects, so any + * comments on those objects are record in the shared pg_shdescription + * catalog. Comments on all other objects are recorded in pg_description. + */ says record where it should say recorded. Thanks, good eye. Also, it strikes me that perhaps the ObjectAddress struct definition should be moved to the new header file; seems a more natural place for it (but then, it seems like a lot of files will need to include the new header, so perhaps that should be left to a subsequent patch). I thought about that, but erred on the side of being conservative and didn't move it. I like the idea, though. Thirdly, is it just me or just could replace a lot of code in DropFoo functions with this new auxiliary code? Seems it's just missing missing_ok support ... I am not sure how much code it would save you at the level of the individual DropFoo() functions; I'd have to look through them more carefully. But now that you mention it, what about getting rid of all of the individual parse nodes for drop statements? Right now we have: DropTableSpaceStmt DropFdwStmt DropForeignServerStmt DropUserMappingStmt DropPLangStmt DropRoleStmt DropStmt (table, sequence, view, index, domain, conversion, schema, text search {parser, dictionary, template, configuration} DropPropertyStmt (rules and triggers) DropdbStmt (capitalized differently, just for fun) DropCastStmt RemoveFuncStmt (so you can't find it by grepping for Drop!) RemoveOpClassStmt RemoveOpFamilyStmt It seems like we could probably unify all of these into a single DropStmt, following the same pattern as CommentStmt, although I think perhaps that should be a follow-on patch rather than doing it as part of this one. GRANT also has some code to translate object names to OIDs, which I thought might be able to use this machinery as well, though I haven't really checked whether it makes sense. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] Initial review of xslt with no limits patch
Mike Fowler m...@mlfowler.com writes: SELECT xslt_process( ... , ... , 'n1=v1,n2=v2,n3=v3,n4=v4,n5=v5'::text) produces samples samplev1/sample samplev2/sample samplev3/sample samplev4/sample samplev5/sample /samples Sadly I get the following in both versions: samples sample/ sample/ sample/ sample/ sample/ /samples Some examination of http://www.xmlsoft.org/XSLT/tutorial/libxslttutorial.html suggests that the parameter values need to be single-quoted, and indeed when I change the last part of your example to 'n1=''v1'',n2=''v2'',n3=''v3'',n4=''v4'',n5=''v5'''::text); I get xslt_process --- samples+ samplev1/sample+ samplev2/sample+ samplev3/sample+ samplev4/sample+ samplev5/sample+ /samples + (1 row) So this seems to be a documentation problem more than a code problem. (It's a bit distressing to notice that the regression tests for the module fail to exercise 3-parameter xslt_process at all, though.) 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] refactoring comment.c
On Fri, 2010-08-06 at 11:02 -0400, Robert Haas wrote: At PGCon, we discussed the possibility that a minimal SE-PostgreSQL implementation would need little more than a hook in ExecCheckRTPerms() [which we've since added] and a security label facility [for which KaiGai has submitted a patch]. I actually sat down to write the security label patch myself while we were in Ottawa, but quickly ran into difficulties: while the hook we have now can't do anything useful with objects other than relations, it's pretty clear from previous discussions on this topic that the demand for labels on other kinds of objects is not going to go away. Rather than adding additional syntax to every object type in the system (some of which don't even have ALTER commands at present), I suggested basing the syntax on the existing COMMENT syntax. After some discussion[1], we seem to have settled on the following: SECURITY LABEL [ FOR provider ] ON object class object name IS 'label'; I understand the concept and it seems like it might work. Not too keen on pretending a noun is a verb. That leads to erroring. verb SECURITY LABEL? verb = CREATE, ADD, ... Can't objects have more than one label? How will you set default security labels on objects? Where do you define labels? Will there be a new privilege to define this? Presumably object owners would not be able to set that themselves, otherwise you could create an object, add a security label to it and then use it to see other things at that level. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Training and 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] refactoring comment.c
On Fri, Aug 6, 2010 at 12:26 PM, Simon Riggs si...@2ndquadrant.com wrote: I understand the concept and it seems like it might work. Not too keen on pretending a noun is a verb. That leads to erroring. verb SECURITY LABEL? verb = CREATE, ADD, ... Can't objects have more than one label? How will you set default security labels on objects? Where do you define labels? Will there be a new privilege to define this? Presumably object owners would not be able to set that themselves, otherwise you could create an object, add a security label to it and then use it to see other things at that level. Uh, these are all good questions, but I think they'd be more appropriate on the thread about the security label patch, to which I linked in my previous email. Many of them have already been discussed there. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] Initial review of xslt with no limits patch
Hello attached updated patch with regression test 2010/8/6 Tom Lane t...@sss.pgh.pa.us: Mike Fowler m...@mlfowler.com writes: SELECT xslt_process( ... , ... , 'n1=v1,n2=v2,n3=v3,n4=v4,n5=v5'::text) produces samples samplev1/sample samplev2/sample samplev3/sample samplev4/sample samplev5/sample /samples Sadly I get the following in both versions: samples sample/ sample/ sample/ sample/ sample/ /samples Some examination of http://www.xmlsoft.org/XSLT/tutorial/libxslttutorial.html suggests that the parameter values need to be single-quoted, and indeed when I change the last part of your example to 'n1=''v1'',n2=''v2'',n3=''v3'',n4=''v4'',n5=''v5'''::text); I get xslt_process --- samples + samplev1/sample+ samplev2/sample+ samplev3/sample+ samplev4/sample+ samplev5/sample+ /samples + (1 row) So this seems to be a documentation problem more than a code problem. (It's a bit distressing to notice that the regression tests for the module fail to exercise 3-parameter xslt_process at all, though.) ??? I don't see it Regards Pavel Stehule regards, tom lane *** ./contrib/xml2/expected/xml2.out.orig 2010-02-28 22:31:57.0 +0100 --- ./contrib/xml2/expected/xml2.out 2010-08-06 18:46:41.0 +0200 *** *** 145,147 --- 145,215 Value/attribute/attributes'); create index idx_xpath on t1 ( xpath_string ('/attributes/attribu...@name=attr_1]/text()', xml_data::text)); + SELECT xslt_process('employeenamecim/nameage30/agepay400/pay/employee'::text, $$xsl:stylesheet xmlns:xsl=http://www.w3.org/1999/XSL/Transform; version=1.0 + xsl:output method=xml omit-xml-declaration=yes indent=yes/ + xsl:strip-space elements=*/ + xsl:param name=n1/ + xsl:param name=n2/ + xsl:param name=n3/ + xsl:param name=n4/ + xsl:param name=n5 select='me'/ + xsl:template match=* + xsl:element name=samples + xsl:element name=sample + xsl:value-of select=$n1/ + /xsl:element + xsl:element name=sample + xsl:value-of select=$n2/ + /xsl:element + xsl:element name=sample + xsl:value-of select=$n3/ + /xsl:element + xsl:element name=sample + xsl:value-of select=$n4/ + /xsl:element + xsl:element name=sample + xsl:value-of select=$n5/ + /xsl:element + xsl:element name=sample + xsl:value-of select=$n6/ + /xsl:element + xsl:element name=sample + xsl:value-of select=$n7/ + /xsl:element + xsl:element name=sample + xsl:value-of select=$n8/ + /xsl:element + xsl:element name=sample + xsl:value-of select=$n9/ + /xsl:element + xsl:element name=sample + xsl:value-of select=$n10/ + /xsl:element + xsl:element name=sample + xsl:value-of select=$n11/ + /xsl:element + xsl:element name=sample + xsl:value-of select=$n12/ + /xsl:element + /xsl:element + /xsl:template + /xsl:stylesheet$$::text, 'n1=v1,n2=v2,n3=v3,n4=v4,n5=v5,n6=v6,n7=v7,n8=v8,n9=v9,n10=v10,n11=v11,n12=v12'::text); + xslt_process + + samples + +samplev1/sample + +samplev2/sample + +samplev3/sample + +samplev4/sample + +samplev5/sample + +samplev6/sample + +samplev7/sample + +samplev8/sample + +samplev9/sample + +samplev10/sample+ +samplev11/sample+ +samplev12/sample+ + /samples+ + + (1 row) + *** ./contrib/xml2/sql/xml2.sql.orig 2010-08-06 18:30:00.0 +0200 --- ./contrib/xml2/sql/xml2.sql 2010-08-06 18:30:57.0 +0200 *** *** 80,82 --- 80,132 create index idx_xpath on t1 ( xpath_string ('/attributes/attribu...@name=attr_1]/text()', xml_data::text)); + + SELECT xslt_process('employeenamecim/nameage30/agepay400/pay/employee'::text, $$xsl:stylesheet xmlns:xsl=http://www.w3.org/1999/XSL/Transform; version=1.0 + xsl:output method=xml omit-xml-declaration=yes indent=yes/ + xsl:strip-space elements=*/ + xsl:param name=n1/ + xsl:param name=n2/ + xsl:param name=n3/ + xsl:param name=n4/ + xsl:param name=n5 select='me'/ + xsl:template match=* + xsl:element name=samples + xsl:element name=sample + xsl:value-of select=$n1/ + /xsl:element + xsl:element name=sample + xsl:value-of select=$n2/ + /xsl:element + xsl:element name=sample + xsl:value-of select=$n3/ + /xsl:element + xsl:element name=sample + xsl:value-of select=$n4/ + /xsl:element + xsl:element name=sample + xsl:value-of select=$n5/ + /xsl:element + xsl:element name=sample + xsl:value-of select=$n6/ + /xsl:element + xsl:element name=sample + xsl:value-of select=$n7/ +
Re: [HACKERS] Initial review of xslt with no limits patch
On 08/06/2010 12:15 PM, Tom Lane wrote: Some examination of http://www.xmlsoft.org/XSLT/tutorial/libxslttutorial.html suggests that the parameter values need to be single-quoted, and indeed when I change the last part of your example to 'n1=''v1'',n2=''v2'',n3=''v3'',n4=''v4'',n5=''v5'''::text); Which would look a whole lot nicer with dollar quoting ;-) 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] including backend ID in relpath of temp rels - updated patch
On Thu, Aug 5, 2010 at 7:05 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: [ BackendRelFileNode patch ] One thing that I find rather distressing about this is the 25% bloat in sizeof(SharedInvalidationMessage). Couldn't we avoid that? Is it really necessary to *ever* send an SI message for a backend-local rel? It can be dropped or unlinked by another backend, so, yes. I agree that one needs to send relcache inval sometimes for temp rels, but I don't see why each backend couldn't interpret that as a flush on its own local version. The problem is that receipt of the inval message results in a call to smgrclosenode(), which has to look up the (Backend)RelFileNode in SMgrRelationHash. If the backend ID isn't present, we can't search the hash table efficiently. This is not only a problem for smgrclosenode(), either; that hash is used in a bunch of places, so changing the hash key isn't really an option. One possibility would be to have two separate shared-invalidation message types for smgr. One would indicate that a non-temporary relation was flushed, so the backend ID field is known to be -1 and we can search the hash quickly. The other would indicate that a temporary relation was flushed and you'd have to scan the whole hash table to find and flush all matching entries. That still doesn't sound great, though. Another thought I had was that if we could count on the backend ID to fit into an int16 we could fit it in to what's currently padding space. That would require rather dramatically lowering the maximum number of backends (currently INT_MAX/4), but it's a little hard to imagine that we can really support more than 32,767 simultaneous backends anyway. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] Initial review of xslt with no limits patch
Andrew Dunstan and...@dunslane.net writes: On 08/06/2010 12:15 PM, Tom Lane wrote: Some examination of http://www.xmlsoft.org/XSLT/tutorial/libxslttutorial.html suggests that the parameter values need to be single-quoted, and indeed when I change the last part of your example to 'n1=''v1'',n2=''v2'',n3=''v3'',n4=''v4'',n5=''v5'''::text); Which would look a whole lot nicer with dollar quoting ;-) No doubt. But one would assume that constant parameters aren't going to be the normal use-case, and dollar quoting isn't helpful for nonconstant text. I think there are issues here that we need to take a step back and think about. Right now, thanks to the lack of documentation, we can probably assume there are approximately zero users of the xslt_process parameter feature. Once we document it that'll no longer be true. So right now would be the time to reflect on whether this is a specification we actually like or believe is usable; it'll be too late to change it later. There are two specific points bothering me now that I see how it works: 1. name = value pretty much sucks, especially with the 100% lack of any quoting convention for either equals or comma. I concur with the thoughts upthread that turning this into a variadic function would be a more sensible solution. 2. I'm not sure whether we ought to auto-single-quote the values. If we don't, how hard is it for users to properly quote nonconstant parameter values? (Will quote_literal work, or are the quoting rules different for libxslt?) If we do, are we giving up functionality someone cares about? 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] including backend ID in relpath of temp rels - updated patch
Robert Haas robertmh...@gmail.com writes: On Thu, Aug 5, 2010 at 7:05 PM, Tom Lane t...@sss.pgh.pa.us wrote: One thing that I find rather distressing about this is the 25% bloat in sizeof(SharedInvalidationMessage). Couldn't we avoid that? Is it really necessary to *ever* send an SI message for a backend-local rel? It can be dropped or unlinked by another backend, so, yes. Really? Surely that should be illegal during normal operation. We might be doing such during crash recovery, but we don't need to broadcast sinval messages then. It might be sufficient to consider that there are local and global smgr inval messages, where the former never get out of the generating backend, so a bool is enough in the message struct. had was that if we could count on the backend ID to fit into an int16 we could fit it in to what's currently padding space. That would require rather dramatically lowering the maximum number of backends (currently INT_MAX/4), but it's a little hard to imagine that we can really support more than 32,767 simultaneous backends anyway. Yeah, that occurred to me too. A further thought is that the id field could probably be reduced to 1 byte, leaving 3 for backendid, which would certainly be plenty. However representing that in a portable struct declaration would be a bit painful I fear. 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] Initial review of xslt with no limits patch
2010/8/6 Tom Lane t...@sss.pgh.pa.us: Andrew Dunstan and...@dunslane.net writes: On 08/06/2010 12:15 PM, Tom Lane wrote: Some examination of http://www.xmlsoft.org/XSLT/tutorial/libxslttutorial.html suggests that the parameter values need to be single-quoted, and indeed when I change the last part of your example to 'n1=''v1'',n2=''v2'',n3=''v3'',n4=''v4'',n5=''v5'''::text); Which would look a whole lot nicer with dollar quoting ;-) No doubt. But one would assume that constant parameters aren't going to be the normal use-case, and dollar quoting isn't helpful for nonconstant text. I think there are issues here that we need to take a step back and think about. Right now, thanks to the lack of documentation, we can probably assume there are approximately zero users of the xslt_process parameter feature. Once we document it that'll no longer be true. So right now would be the time to reflect on whether this is a specification we actually like or believe is usable; it'll be too late to change it later. I know about one important user from Czech Republic There are two specific points bothering me now that I see how it works: 1. name = value pretty much sucks, especially with the 100% lack of any quoting convention for either equals or comma. I concur with the thoughts upthread that turning this into a variadic function would be a more sensible solution. I'll propose a new kind of functions (only position parameter's function). My idea is simple - for functions with this mark the mixed and named notation is blocked. But these functions can have a parameter names - and these names can be passed to function. So there is possible have a xslt_process function with current behave - third argument has not label, and new variadic version like xslt_process(..,.., param_name1 = 'v1', param_name2 = 'v2', param_name3 = 'v3', ...) an implementation of this functionality can be very simple, and we can use this for xslt_process in 9.1 Regards Pavel Stehule 2. I'm not sure whether we ought to auto-single-quote the values. If we don't, how hard is it for users to properly quote nonconstant parameter values? (Will quote_literal work, or are the quoting rules different for libxslt?) If we do, are we giving up functionality someone cares about? 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] including backend ID in relpath of temp rels - updated patch
On Fri, Aug 6, 2010 at 1:14 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Thu, Aug 5, 2010 at 7:05 PM, Tom Lane t...@sss.pgh.pa.us wrote: One thing that I find rather distressing about this is the 25% bloat in sizeof(SharedInvalidationMessage). Couldn't we avoid that? Is it really necessary to *ever* send an SI message for a backend-local rel? It can be dropped or unlinked by another backend, so, yes. Really? Surely that should be illegal during normal operation. We might be doing such during crash recovery, but we don't need to broadcast sinval messages then. autovacuum.c does it when we start to worry about XID wraparound, but you can also do it from any normal backend. Just DROP TABLE pg_temp_2.foo or whatever and away you go. It might be sufficient to consider that there are local and global smgr inval messages, where the former never get out of the generating backend, so a bool is enough in the message struct. It would be nice to be able to do it that way, but I don't believe it's the case, per the above. had was that if we could count on the backend ID to fit into an int16 we could fit it in to what's currently padding space. That would require rather dramatically lowering the maximum number of backends (currently INT_MAX/4), but it's a little hard to imagine that we can really support more than 32,767 simultaneous backends anyway. Yeah, that occurred to me too. A further thought is that the id field could probably be reduced to 1 byte, leaving 3 for backendid, which would certainly be plenty. However representing that in a portable struct declaration would be a bit painful I fear. Well, presumably we'd just represent it as a 1-byte field followed by a 2-byte field, and do a bit of math. But I don't really see the point. The whole architecture of a shared invalidation queue is fundamentally non-scalable because it's a broadcast medium. If we wanted to efficiently support even thousands of backends (let alone tens or hundreds of thousands) I assume we would need to rearchitect this completely with more fine-grained queues, and have backends subscribe to the queues pertaining to the objects they want to access before touching them. Or maybe something else entirely. But I don't think broadcasting to 30,000 backends is going to work for the same reason that plugging 30,000 machines into an Ethernet *hub* doesn't work. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] Cost of AtEOXact_Buffers in --enable-cassert
Hi, I do test (and even run) some stuff running with cassert enabled. For many things the reliability and performance is ok enough and its useful, especially if you have your own c functions and such. Imho thats useful as it makes catching some bugs more likely... The most prohibitively expensive part is the AtEOXact_Buffers check of running through all buffers and checking their pin count. And it makes $app's regression tests take thrice their time... Would somebody object agains putting those in an extra define so that those can be disabled in pg_config_manual? Or even disable it by default entirely... Andres -- 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] Initial review of xslt with no limits patch
Pavel Stehule pavel.steh...@gmail.com writes: 2010/8/6 Tom Lane t...@sss.pgh.pa.us: I think there are issues here that we need to take a step back and think about. Â Right now, thanks to the lack of documentation, we can probably assume there are approximately zero users of the xslt_process parameter feature. Â Once we document it that'll no longer be true. Â So right now would be the time to reflect on whether this is a specification we actually like or believe is usable; it'll be too late to change it later. I know about one important user from Czech Republic Well, if there actually is anybody who's figured it out, we could easily have a backwards-compatible mode. Provide one variadic function that acts as follows: even number of variadic array elements - they're names/values one variadic array element - parse it the old way otherwise - error I wouldn't even bother with fixing the MAXPARAMS limitation for the old way code, just make it work exactly as before. I'll propose a new kind of functions (only position parameter's function). My idea is simple - for functions with this mark the mixed and named notation is blocked. We don't need random new function behaviors for this. Anyway your proposal doesn't work at all for non-constant parameter names. 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] including backend ID in relpath of temp rels - updated patch
Robert Haas robertmh...@gmail.com writes: On Fri, Aug 6, 2010 at 1:14 PM, Tom Lane t...@sss.pgh.pa.us wrote: Really? Surely that should be illegal during normal operation. We might be doing such during crash recovery, but we don't need to broadcast sinval messages then. autovacuum.c does it when we start to worry about XID wraparound, but you can also do it from any normal backend. Just DROP TABLE pg_temp_2.foo or whatever and away you go. Mph. I'm still not convinced that the sinval message needs to carry backendid. But maybe the first-cut solution should just be to squeeze the id into the padding area. You should be able to get up to 65535 allowed backends, not 32k --- or perhaps use different message type IDs for local and global backendid, so that all 65536 bitpatterns are allowed for a non-global backendid. Well, presumably we'd just represent it as a 1-byte field followed by a 2-byte field, and do a bit of math. But I don't really see the point. The whole architecture of a shared invalidation queue is fundamentally non-scalable because it's a broadcast medium. Sure, it tops out somewhere, but 32K is way too close to configurations we know work well enough in the field (I've seen multiple reports of people using a couple thousand backends). In any case, sinval readers don't block each other in the current implementation, so I'm really dubious that there's any inherent scalability limitation there. I'll hold still for 64K but I think it might be better to go for 2^24. 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] Initial review of xslt with no limits patch
2010/8/6 Robert Haas robertmh...@gmail.com: On Fri, Aug 6, 2010 at 1:46 PM, Pavel Stehule pavel.steh...@gmail.com wrote: I'll propose a new kind of functions (only position parameter's function). My idea is simple - for functions with this mark the mixed and named notation is blocked. But these functions can have a parameter names - and these names can be passed to function. So there is possible have a xslt_process function with current behave - third argument has not label, and new variadic version like xslt_process(..,.., param_name1 = 'v1', param_name2 = 'v2', param_name3 = 'v3', ...) an implementation of this functionality can be very simple, and we can use this for xslt_process in 9.1 Why wouldn't we just pass two text arrays to this function and be done with it? Custom syntax is all well and good when you're writing these calls by hand, but it's not hard to imagine someone wanting to pass in values stored somewhere else. yes, it is possible too. And maybe is better then current xslt_process. But it isn't too much readable and robust. You have to calculate position of name and position of value. This is same in other languages. You can use a dynamic parameters in PHP or Perl via two arrays, but nobody use it. People use a hash syntax (param1=val, param2=val). This proposal isn't only for xslt_process function. Why hstore has a custom parser? It can use only a pair of arrays too. For Tom: proposed syntax can be used generally - everywhere when you are working with collection. It can be used for hash (hstore) constructor for example. For me is more readable code like select hstore(name := 'Tomas', surname := 'Novak') than select hstore('name=\'Tomas\', surname=\'Novak\'') The main advance of this feature is simplicity of custom functions. Its must not have a custom parser. So possible an using is hstore, xslt_process Regards Pavel Stehule -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] including backend ID in relpath of temp rels - updated patch
On Fri, Aug 6, 2010 at 12:50 PM, Robert Haas robertmh...@gmail.com wrote: Just DROP TABLE pg_temp_2.foo or whatever and away you go. wow! that's true... and certainly a bug... we shouldn't allow any session to drop other session's temp tables, or is there a reason for this misbehavior? -- Jaime Casanova www.2ndQuadrant.com Soporte y capacitación de PostgreSQL -- 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] Initial review of xslt with no limits patch
Robert Haas robertmh...@gmail.com writes: Why wouldn't we just pass two text arrays to this function and be done with it? That would work too, although I think it might be a bit harder to use than one alternating-name-and-value array, at least in some scenarios. You'd have to be careful that you got the values in the same order in both arrays, which'd be easy to botch. There might be other use-cases where two separate arrays are easier to use, but I'm not seeing one offhand. 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] including backend ID in relpath of temp rels - updated patch
Jaime Casanova ja...@2ndquadrant.com writes: On Fri, Aug 6, 2010 at 12:50 PM, Robert Haas robertmh...@gmail.com wrote: Just DROP TABLE pg_temp_2.foo or whatever and away you go. wow! that's true... and certainly a bug... No, it's not a bug. You'll find only superusers can do it. we shouldn't allow any session to drop other session's temp tables, or is there a reason for this misbehavior? It is intentional, though I'd be willing to give it up if we had more bulletproof crash-cleanup of temp tables --- which is one of the things this patch is supposed to result in. 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] Cost of AtEOXact_Buffers in --enable-cassert
Andres Freund and...@anarazel.de writes: The most prohibitively expensive part is the AtEOXact_Buffers check of running through all buffers and checking their pin count. And it makes $app's regression tests take thrice their time... Would somebody object agains putting those in an extra define so that those can be disabled in pg_config_manual? Or even disable it by default entirely... Not a chance for the latter; this is an important sanity check that catches real coding mistakes with some frequency. I'd be willing to consider a half assert mode that turns off some of the most expensive checks, but AtEOXact_Buffers is hardly the only thing that ought to be in that list. The CLOBBER_FREED_MEMORY and memory context checking stuff is pretty durn expensive too. 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] including backend ID in relpath of temp rels - updated patch
On Fri, Aug 6, 2010 at 2:07 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Fri, Aug 6, 2010 at 1:14 PM, Tom Lane t...@sss.pgh.pa.us wrote: Really? Surely that should be illegal during normal operation. We might be doing such during crash recovery, but we don't need to broadcast sinval messages then. autovacuum.c does it when we start to worry about XID wraparound, but you can also do it from any normal backend. Just DROP TABLE pg_temp_2.foo or whatever and away you go. Mph. I'm still not convinced that the sinval message needs to carry backendid. Hey, if you have an idea... I'd love to get rid of it, too, but I don't see how to do it. But maybe the first-cut solution should just be to squeeze the id into the padding area. You should be able to get up to 65535 allowed backends, not 32k --- or perhaps use different message type IDs for local and global backendid, so that all 65536 bitpatterns are allowed for a non-global backendid. Well, presumably we'd just represent it as a 1-byte field followed by a 2-byte field, and do a bit of math. But I don't really see the point. The whole architecture of a shared invalidation queue is fundamentally non-scalable because it's a broadcast medium. Sure, it tops out somewhere, but 32K is way too close to configurations we know work well enough in the field (I've seen multiple reports of people using a couple thousand backends). In any case, sinval readers don't block each other in the current implementation, so I'm really dubious that there's any inherent scalability limitation there. I'll hold still for 64K but I think it might be better to go for 2^24. Well, I wouldn't expect anyone to use an exclusive lock for readers without a good reason, but you still have n backends that each have to read, presumably, about O(n) messages, so eventually that's going to start to pinch. Do you think it's worth worrying about the reduction in the number of possible SI message types? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] Surprising dead_tuple_count from pgstattuple
This is an expansion of the question I posed in this thread: http://postgresql.1045698.n5.nabble.com/Need-help-understanding-vacuum-verbose-output-tp2265895p2266912.html I am framing the question here in relation to pgstattuple. Running 8.4.4 on Centos. I have a table T with 5,063,463 rows. It was just restored from a backup, and there is no other activity in this database. I ran a vacuum. pg_stat_user_tables.n_dead_tup (which is really pg_stat_get_dead_tuples('T'::regclass::oid)) says 0 pgstattuple says dead_tuple_count=0, free_space=1,355,152 1. I delete 10,000 rows. pg_stat_user_tables.n_live_tup - 5053463 pg_stat_user_tables.n_dead_tup - 1 pgstattuple.dead_tuple_count - 1 pgstattuple.free_space - 1355152 So far, so good. pgstattuple is counting the dead tuples, and not including those tuples in the free space count. 2. I delete 15,000 more rows. pg_stat_user_tables.n_live_tup - 5038463 pg_stat_user_tables.n_dead_tup - 25000 pgstattuple.dead_tuple_count - 15000 ?? pgstattuple.free_space - 1996904 ?? pgstattuple now appears to count the earlier 10K deleted tuples as no longer dead, but free space. 3. I delete 50,000 more rows. pg_stat_user_tables.n_live_tup - 4988463 pg_stat_user_tables.n_dead_tup - 75000 pgstattuple.dead_tuple_count - 50022 ?? pgstattuple.free_space - 2966628 ?? Same thing, pgstattuple appears to see only the most recent delete transaction (but off by 22), and count the prior ones as free. 4. vacuum verbose vacuum verbose t; INFO: vacuuming public.t INFO: scanned index t_pkey to remove 75000 row versions DETAIL: CPU 0.01s/0.38u sec elapsed 0.40 sec. INFO: t: removed 75000 row versions in 637 pages DETAIL: CPU 0.00s/0.00u sec elapsed 0.00 sec. INFO: index t_pkey now contains 4988463 row versions in 13886 pages DETAIL: 75000 index row versions were removed. 204 index pages have been deleted, 0 are currently reusable. CPU 0.00s/0.00u sec elapsed 0.00 sec. INFO: t: found 50022 removable, 3696 nonremovable row versions in 668 out of 51958 pages DETAIL: 0 dead row versions cannot be removed yet. There were 0 unused item pointers. 0 pages are entirely empty. CPU 0.01s/0.39u sec elapsed 0.40 sec. VACUUM Time: 482.771 ms It seems relevant that vacuum reports the same incorrect number -- 50022 -- as part of its output. That makes me think that pgstattuple may be using similar logic to get its dead tuple count. I wonder if the key to this is that pgstattuple uses HeapTupleSatisfiesVisibility() to test for deadness. If so, why would this call return apparently false positives? I know that pgstattuple is meant to be used for debugging only. I have found pgstatindex to be very helpful in identifying bloat in my indexes. Per Tom in the other thread, I now understand that the found 50022 removable, 3696 nonremovable line is referring to the subset of pages that it scanned looking for dead tuples. I keep coming back to this, though -- 50,022 seems to be just wrong, or perhaps simply misleading -- i.e. way too low. It's present in the output of vacuum, and the output of pgstattuple. I'd like to understand what meaning this number has, and, ideally, how I can use to to detect things like bloat or fragmentation. Thanks! -- View this message in context: http://postgresql.1045698.n5.nabble.com/Surprising-dead-tuple-count-from-pgstattuple-tp2266955p2266955.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] Initial review of xslt with no limits patch
Pavel Stehule pavel.steh...@gmail.com writes: For Tom: proposed syntax can be used generally - everywhere when you are working with collection. It can be used for hash (hstore) constructor for example. For me is more readable code like select hstore(name := 'Tomas', surname := 'Novak') You've tried to sell us on that before, with few takers. This proposed use-case impresses me even less than the previous ones, because callers of xslt_process seem quite likely to need to work with non-constant parameter names. In any case, given what we have at the moment for function overload resolution rules, I think it's a fundamentally bad idea to introduce a wild card function type that would necessarily conflict with practically every other possible function declaration. So regardless of what use-cases you propose, I'm going to vote against that. 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] including backend ID in relpath of temp rels - updated patch
On Fri, Aug 6, 2010 at 2:16 PM, Tom Lane t...@sss.pgh.pa.us wrote: Jaime Casanova ja...@2ndquadrant.com writes: On Fri, Aug 6, 2010 at 12:50 PM, Robert Haas robertmh...@gmail.com wrote: Just DROP TABLE pg_temp_2.foo or whatever and away you go. wow! that's true... and certainly a bug... No, it's not a bug. You'll find only superusers can do it. we shouldn't allow any session to drop other session's temp tables, or is there a reason for this misbehavior? It is intentional, though I'd be willing to give it up if we had more bulletproof crash-cleanup of temp tables --- which is one of the things this patch is supposed to result in. This patch only directly addresses the issue of cleaning up the storage, so there are still the catalog entries to worry about. But it doesn't seem impossible to think about building on this approach to eventually handle that part of the problem better, too. I haven't thought too much about what that would look like, but I think it could be done. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] including backend ID in relpath of temp rels - updated patch
Robert Haas robertmh...@gmail.com writes: On Fri, Aug 6, 2010 at 2:07 PM, Tom Lane t...@sss.pgh.pa.us wrote: Sure, it tops out somewhere, but 32K is way too close to configurations we know work well enough in the field (I've seen multiple reports of people using a couple thousand backends). Well, I wouldn't expect anyone to use an exclusive lock for readers without a good reason, but you still have n backends that each have to read, presumably, about O(n) messages, so eventually that's going to start to pinch. Sure, but I don't see much to be gained from multiple queues either. There are few (order of zero, in fact) cases where sinval messages are transmitted that aren't of potential interest to all backends. Maybe you could do something useful with a very large number of dynamically-defined queues (like one per relation) ... but managing that would probably swamp any savings. Do you think it's worth worrying about the reduction in the number of possible SI message types? IIRC the number of message types is the number of catalog caches plus half a dozen or so. We're a long way from exhausting even a 1-byte ID field; and we could play more games if we had to, since there would be a padding byte free in the message types that refer to a catalog cache. IOW, 1-byte id doesn't bother me. 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] including backend ID in relpath of temp rels - updated patch
Robert Haas robertmh...@gmail.com writes: This patch only directly addresses the issue of cleaning up the storage, so there are still the catalog entries to worry about. But it doesn't seem impossible to think about building on this approach to eventually handle that part of the problem better, too. I haven't thought too much about what that would look like, but I think it could be done. Perhaps run through pg_class after restart and flush anything marked relistemp? Although the ideal solution, probably, would be for temp tables to not have persistent catalog entries in the first place. 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] including backend ID in relpath of temp rels - updated patch
On Fri, Aug 06, 2010 at 03:00:35PM -0400, Tom Lane wrote: Robert Haas robertmh...@gmail.com writes: This patch only directly addresses the issue of cleaning up the storage, so there are still the catalog entries to worry about. But it doesn't seem impossible to think about building on this approach to eventually handle that part of the problem better, too. I haven't thought too much about what that would look like, but I think it could be done. Perhaps run through pg_class after restart and flush anything marked relistemp? Although the ideal solution, probably, would be for temp tables to not have persistent catalog entries in the first place. For the upcoming global temp tables, which are visible in other sessions, how would this work? Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] including backend ID in relpath of temp rels - updated patch
David Fetter da...@fetter.org writes: On Fri, Aug 06, 2010 at 03:00:35PM -0400, Tom Lane wrote: Perhaps run through pg_class after restart and flush anything marked relistemp? Although the ideal solution, probably, would be for temp tables to not have persistent catalog entries in the first place. For the upcoming global temp tables, which are visible in other sessions, how would this work? Well, that's a totally different matter. Those would certainly have persistent entries, at least for the master version. I don't think anyone's really figured out what the best implementation looks like; but maybe there would be per-backend child versions that could act just like the current kind of temp table (and again would ideally not have any persistent catalog entries). 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] Initial review of xslt with no limits patch
2010/8/6 Tom Lane t...@sss.pgh.pa.us: Pavel Stehule pavel.steh...@gmail.com writes: For Tom: proposed syntax can be used generally - everywhere when you are working with collection. It can be used for hash (hstore) constructor for example. For me is more readable code like select hstore(name := 'Tomas', surname := 'Novak') You've tried to sell us on that before, with few takers. This proposed use-case impresses me even less than the previous ones, because callers of xslt_process seem quite likely to need to work with non-constant parameter names. In any case, given what we have at the moment for function overload resolution rules, I think it's a fundamentally bad idea to introduce a wild card function type that would necessarily conflict with practically every other possible function declaration. So regardless of what use-cases you propose, I'm going to vote against that. It must not be a function. Just I missing any tool that helps with complex structured data. This proposed kind functions has one advantage - there isn't necessary any change in parser. Yes, I can use a pair of arrays, I can use a one array with seq name, value, I can use a custom parser. But nothing from these offers a comfort or readability for example a Perl's hash tables. so if we have a integrated hash tables, then we can to write some like CREATE FUNCTION xslt_process(..,.., text{}) select xslt_process(..,.., { par1 = val1, par2 = val3, .. } ) any simple method can significantly help for us, who write a lot of complex stored procedures. It can be a big help. I am only dissatisfied because it is Perlism - maybe I don't understand SQL well, but my personal opinion about the most natural syntax for this situation is some like SQL/XML - xmlattributes or named notation. SQL isn't too much consistent too - it uses two semantically similar syntax foo(name=value, ..) versus foo(value AS name, ..) Next my idea was mix of named parameters and marked variadic parameter: CREATE FUNCTION xslt_process(..,.., VARIADIC LABELED text[]) and then we can call SELECT xslt_process(..,.., par1 := var, ..) This doesn't introduce a heavy new syntax - just use old in some specific context. It is only my feeling, so this way is more SQL natural than using a some hash data type. Maybe you don't think, so stored procedures must not to have a this functionality. Everywhere where you can to wrap a c library for SQL you have to meet this task - often you have to pass a set of flags, collection of values. With default parameter values situation is better then before, but its still not ideal - for example SOAP interface or dblink to Oracle SELECT exec(db, 'SELECT * FROM foo where c = :name', name = 'Pavel') so this is my motivation - the possibility to write generic custom functions. Sure - this is redundant to existing functionality. I can write: SELECT exec(db, 'SELECT * FROM ...', ARRAY['name'], ARRAY['Pavel']) -- Robert'syntax or your syntax: SELECT exec(db, 'SELECT * FROM ...', ARRAY['name','Pavel']) or like current xml2 syntax: SELECT exec(db, 'SELECT * FROM ...', 'name=Pavel) But these versions are too simple if you understand me. It doesn't do life of SQL stored procedure's programmer simper. Regards Pavel p.s. sorry for offtopic p.s. for me isn't important notation. Just I would to like more things with custom functions withou parser modification. 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] including backend ID in relpath of temp rels - updated patch
On Fri, Aug 6, 2010 at 3:00 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: This patch only directly addresses the issue of cleaning up the storage, so there are still the catalog entries to worry about. But it doesn't seem impossible to think about building on this approach to eventually handle that part of the problem better, too. I haven't thought too much about what that would look like, but I think it could be done. Perhaps run through pg_class after restart and flush anything marked relistemp? The trouble is that you have to bind to a database before you can run through pg_class, and the postmaster doesn't. Of course, if it could attach to a database and then detach again, this might be feasible, although perhaps still a bit overly complex for the postmaster, but in any event it doesn't. We seem to already have a mechanism in place where a backend that creates a temprel nukes any pre-existing temprel schema for its own backend, but of course that's not fool-proof. Although the ideal solution, probably, would be for temp tables to not have persistent catalog entries in the first place. I've been thinking about that, but it's a bit challenging to imagine how it could work. It's not just the pg_class entries you have to think about, but also pg_attrdef, pg_attribute, pg_constraint, pg_description, pg_index, pg_rewrite, and pg_trigger. An even stickier problem is that we have lots of places in the backend code that refer to objects by OID. We'd either need to change all of that code (which seems like a non-starter) or somehow guarantee that the OIDs assigned to any given backend's private objects would be different from those assigned to any public object (which I also don't see how to do). -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] including backend ID in relpath of temp rels - updated patch
On Fri, Aug 6, 2010 at 2:43 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Fri, Aug 6, 2010 at 2:07 PM, Tom Lane t...@sss.pgh.pa.us wrote: Sure, it tops out somewhere, but 32K is way too close to configurations we know work well enough in the field (I've seen multiple reports of people using a couple thousand backends). Well, I wouldn't expect anyone to use an exclusive lock for readers without a good reason, but you still have n backends that each have to read, presumably, about O(n) messages, so eventually that's going to start to pinch. Sure, but I don't see much to be gained from multiple queues either. There are few (order of zero, in fact) cases where sinval messages are transmitted that aren't of potential interest to all backends. Maybe you could do something useful with a very large number of dynamically-defined queues (like one per relation) ... but managing that would probably swamp any savings. Well, what I was thinking is that if you could guarantee that a certain backend COULDN'T have a particular relfilenode open at the smgr level, for example, then it needn't read the invalidation messages for that relfilenode. Precisely how to slice that up is another matter. For the present case, for instance, you could creating one queue per backend. In the normal course of events, each backend would subscribe only to its own queue, but if one backend wanted to drop a temporary relation belonging to some other backend, it would temporarily subscribe to that backend's queue, do whatever it needed to do, and then flush all the smgr references before unsubscribing from the queue. That's a bit silly because we doubtless wouldn't invent such a complicated mechanism just for this case, but I think it illustrates the kind of thing that one could do. Or if you wanted to optimize for the case of a large number of databases running in a single cluster, perhaps you'd want one queue per database plus a shared queue for the shared catalogs. I don't know. This is just pie in the sky. Do you think it's worth worrying about the reduction in the number of possible SI message types? IIRC the number of message types is the number of catalog caches plus half a dozen or so. We're a long way from exhausting even a 1-byte ID field; and we could play more games if we had to, since there would be a padding byte free in the message types that refer to a catalog cache. IOW, 1-byte id doesn't bother me. OK. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] [ADMIN] postgres 9.0 crash when bringing up hot standby
On 06/08/10 17:31, Fujii Masao wrote: On Fri, Aug 6, 2010 at 10:10 PM, Alanoly Andrewsalano...@invera.com wrote: I’m testing “hot standby” using “streaming WAL records”. On trying to bring (dbx) where _alloc_initial_pthread(??) at 0x949567c __pth_init(??) at 0x9493ba4 uload(??, ??, ??, ??, ??, ??, ??, ??) at 0x9fff0001954 load_64.load(??, ??, ??) at 0x904686c loadAndInit() at 0x947bd7c dlopen(??, ??) at 0x911cc4c internal_load_library(libname = /apps/pg_9.0_b4/lib/postgresql/libpqwalreceiver.so), line 234 in dfmgr.c load_file(filename = libpqwalreceiver, restricted = '\0'), line 156 in dfmgr.c WalReceiverMain(), line 248 in walreceiver.c AuxiliaryProcessMain(argc = 2, argv = 0x0fffa8b8), line 428 in bootstrap.c StartChildProcess(type = WalReceiverProcess), line 4405 in postmaster.c sigusr1_handler(postgres_signal_arg = 30), line 4227 in postmaster.c __fd_select(??, ??, ??, ??, ??) at 0x911805c postmaster.select(__fds = 5, __readlist = 0x0fffd0a8, __writelist = (nil), __exceptlist = (nil), __timeout = 0x00c0), line 229 in time.h unnamed block in ServerLoop(), line 1391 in postmaster.c unnamed block in ServerLoop(), line 1391 in postmaster.c ServerLoop(), line 1391 in postmaster.c PostmasterMain(argc = 1, argv = 0x0001102aa4b0), line 1092 in postmaster.c main(argc = 1, argv = 0x0001102aa4b0), line 188 in main.c Any pointers on how to resolve the issue will be much appreciated. So, loading libpqwalreceiver library crashes. It looks like it might be pthread-related. Perhaps something wrong with our makefiles, causing libpqwalreceiver to be built with wrong flags? Does contrib/dblink work? If you look at the build log, what is the command line used to compile libpqwalreceiver, and what is the command line used to build other libraries, like contrib/dblink? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.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: Review: Re: [PATCH] Re: [HACKERS] Adding xpath_exists function
On fre, 2010-08-06 at 09:04 +0100, Mike Fowler wrote: If the patch is to be committed, does it make sense for me to refine it such that it uses the new xpath internal function you extracted in the xmlexists patch? Yes, you can probably shrink this patch down to about 20 lines. -- 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] Initial review of xslt with no limits patch
On Aug 6, 2010, at 11:13 AM, Tom Lane wrote: That would work too, although I think it might be a bit harder to use than one alternating-name-and-value array, at least in some scenarios. You'd have to be careful that you got the values in the same order in both arrays, which'd be easy to botch. There might be other use-cases where two separate arrays are easier to use, but I'm not seeing one offhand. Stuff like this makes me wish PostgreSQL had an ordered pair data type. Then you'd just have a function with `variadic ordered pair` as the signature. I don't suppose anyone has implemented a data type like this… 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] Cost of AtEOXact_Buffers in --enable-cassert
On Friday 06 August 2010 20:23:15 Tom Lane wrote: Andres Freund and...@anarazel.de writes: The most prohibitively expensive part is the AtEOXact_Buffers check of running through all buffers and checking their pin count. And it makes $app's regression tests take thrice their time... Would somebody object agains putting those in an extra define so that those can be disabled in pg_config_manual? Or even disable it by default entirely... Not a chance for the latter; this is an important sanity check that catches real coding mistakes with some frequency. Ok. I'd be willing to consider a half assert mode that turns off some of the most expensive checks, but AtEOXact_Buffers is hardly the only thing that ought to be in that list. The CLOBBER_FREED_MEMORY and memory context checking stuff is pretty durn expensive too. I personally have seen that catching way more bugs than the AtEOXact_Buffers check, but that might be because I have found mostly bugs in random c functions, not in pg stuff ;-) I will wait a bit and wait for more suggestions about expensive checks and/or other comments and will provide a patch for such a mode. Andres -- 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] including backend ID in relpath of temp rels - updated patch
Excerpts from Robert Haas's message of vie ago 06 15:32:21 -0400 2010: Perhaps run through pg_class after restart and flush anything marked relistemp? The trouble is that you have to bind to a database before you can run through pg_class, and the postmaster doesn't. Of course, if it could attach to a database and then detach again, this might be feasible, although perhaps still a bit overly complex for the postmaster, but in any event it doesn't. A simpler idea seems to run a process specifically to connect to the database to scan pg_class there, and then die. It sounds a tad expensive though. I've been thinking about that, but it's a bit challenging to imagine how it could work. It's not just the pg_class entries you have to think about, but also pg_attrdef, pg_attribute, pg_constraint, pg_description, pg_index, pg_rewrite, and pg_trigger. An even stickier problem is that we have lots of places in the backend code that refer to objects by OID. We'd either need to change all of that code (which seems like a non-starter) or somehow guarantee that the OIDs assigned to any given backend's private objects would be different from those assigned to any public object (which I also don't see how to do). Maybe we could reserve one of the 32 bits of OID to indicate private-ness. -- Álvaro Herrera alvhe...@commandprompt.com 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] Initial review of xslt with no limits patch
On fre, 2010-08-06 at 13:01 -0400, Tom Lane wrote: 2. I'm not sure whether we ought to auto-single-quote the values. If we don't, how hard is it for users to properly quote nonconstant parameter values? (Will quote_literal work, or are the quoting rules different for libxslt?) If we do, are we giving up functionality someone cares about? Not every parameter is a string. Compare xsltproc: --param PARAMNAME PARAMVALUE Pass a parameter of name PARAMNAME and value PARAMVALUE to the stylesheet. You may pass multiple name/value pairs up to a maximum of 32. If the value being passed is a string, you can use --stringparam instead, to avoid additional quote characters that appear in string expressions. Note: the XPath expression must be UTF-8 encoded. --stringparam PARAMNAME PARAMVALUE Pass a parameter of name PARAMNAME and value PARAMVALUE where PARAMVALUE is a string rather than a node identifier. Note: The string must be UTF-8 encoded. -- 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] Initial review of xslt with no limits patch
On fre, 2010-08-06 at 21:31 +0200, Pavel Stehule wrote: It must not be a function. Just I missing any tool that helps with complex structured data. This proposed kind functions has one advantage - there isn't necessary any change in parser. Yes, I can use a pair of arrays, I can use a one array with seq name, value, I can use a custom parser. But nothing from these offers a comfort or readability for example a Perl's hash tables. Maybe you should just use PL/XSLT. :-) -- 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] Initial review of xslt with no limits patch
2010/8/6 David E. Wheeler da...@kineticode.com: On Aug 6, 2010, at 11:13 AM, Tom Lane wrote: That would work too, although I think it might be a bit harder to use than one alternating-name-and-value array, at least in some scenarios. You'd have to be careful that you got the values in the same order in both arrays, which'd be easy to botch. There might be other use-cases where two separate arrays are easier to use, but I'm not seeing one offhand. Stuff like this makes me wish PostgreSQL had an ordered pair data type. Then you'd just have a function with `variadic ordered pair` as the signature. yes it is one a possibility and probably best. The nice of this variant can be two forms like current variadic does - foo(.., a := 10, b := 10) or foo(.., variadic ARRAY[(a,10),(b,10)]) I don't suppose anyone has implemented a data type like this… 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] Initial review of xslt with no limits patch
2010/8/6 Peter Eisentraut pete...@gmx.net: On fre, 2010-08-06 at 21:31 +0200, Pavel Stehule wrote: It must not be a function. Just I missing any tool that helps with complex structured data. This proposed kind functions has one advantage - there isn't necessary any change in parser. Yes, I can use a pair of arrays, I can use a one array with seq name, value, I can use a custom parser. But nothing from these offers a comfort or readability for example a Perl's hash tables. Maybe you should just use PL/XSLT. :-) :) -- 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] Initial review of xslt with no limits patch
yes it is one a possibility and probably best. The nice of this variant can be two forms like current variadic does - foo(.., a := 10, b := 10) or foo(.., variadic ARRAY[(a,10),(b,10)]) pardon foo(..., VARIADIC ARRAY[('a', 10), ('b' 10)]) regards Pavel -- 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] review: xml_is_well_formed
On fre, 2010-08-06 at 07:31 -0400, Robert Haas wrote: What about making the function sensitive to the XML OPTION, such that: test=# SET xmloption TO DOCUMENT; SET text=# SELECT xml_is_well_formed('foo'); xml_is_well_formed f (1 row) That will make using this function a huge hassle, won't it? Functions that do different things depending on GUC settings are usually troublesome. Having three functions would be more sensible if we need all three behaviors, but I don't see why we do. Upthread you opined that this function should essentially indicate whether a cast to type xml would succeed, and observing the xmloption is an essential part of that. I had assumed the original patch actually did 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] review: xml_is_well_formed
On fre, 2010-08-06 at 14:43 +0100, Mike Fowler wrote: Or perhaps it could return a string instead of a boolean: content, document, or NULL if it's neither. I like the sound of that. In fact this helps workaround the IS DOCUMENT and IS CONTENT limitations such that you can you can select only content, only documents or both is you use IS NOT NULL. Unless anyone sees a reason that this function needs to remain a boolean function, I'll rework the patch over the weekend. What is the actual use case for this function? Is the above behavior actually useful? One reason to stick with boolean is backward compatibility. -- 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] Initial review of xslt with no limits patch
On Aug 6, 2010, at 1:49 PM, Pavel Stehule wrote: yes it is one a possibility and probably best. The nice of this variant can be two forms like current variadic does - foo(.., a := 10, b := 10) or foo(.., variadic ARRAY[(a,10),(b,10)]) I started fiddling and got as far as this: CREATE TYPE pair AS ( key text, val text ); CREATE OR REPLACE FUNCTION pair(anyelement, anyelement) RETURNS pair LANGUAGE SQL AS $$ SELECT ROW($1, $2)::pair; $$; CREATE OR REPLACE FUNCTION pair(text, text) RETURNS pair LANGUAGE SQL AS $$ SELECT ROW($1, $2)::pair; $$; CREATE OPERATOR ~ ( LEFTARG = anyelement, RIGHTARG = anyelement, PROCEDURE = pair ); CREATE OPERATOR ~ ( LEFTARG = text, RIGHTARG = text, PROCEDURE = pair ); CREATE OR REPLACE FUNCTION foo(variadic pair[]) RETURNS SETOF text LANGUAGE SQL AS $$ --SELECT unnest($1)::text SELECT $1[1].key UNION SELECT $1[1].val UNION SELECT $1[2].key UNION SELECT $1[2].val; $$; SELECT foo('this' ~ 'that', 1 ~ 4); Not bad, I think. I kind of like it. It reminds me how much I hate the % hstore construction operator, though (the new name for =). 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] Initial review of xslt with no limits patch
On Fri, Aug 6, 2010 at 1:46 PM, Pavel Stehule pavel.steh...@gmail.com wrote: I'll propose a new kind of functions (only position parameter's function). My idea is simple - for functions with this mark the mixed and named notation is blocked. But these functions can have a parameter names - and these names can be passed to function. So there is possible have a xslt_process function with current behave - third argument has not label, and new variadic version like xslt_process(..,.., param_name1 = 'v1', param_name2 = 'v2', param_name3 = 'v3', ...) an implementation of this functionality can be very simple, and we can use this for xslt_process in 9.1 Why wouldn't we just pass two text arrays to this function and be done with it? Custom syntax is all well and good when you're writing these calls by hand, but it's not hard to imagine someone wanting to pass in values stored somewhere else. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] Functional dependencies and GROUP BY
On mån, 2010-07-26 at 10:46 -0600, Alex Hunsaker wrote: On Sat, Jul 24, 2010 at 06:23, Peter Eisentraut pete...@gmx.net wrote: Another open question I thought of was whether we should put the dependency record on the pg_index row, or the pg_constraint row, or perhaps the pg_class row. Right now, it is using pg_index, because that was easiest to code up, but I suspect that once we have not-null constraints in pg_constraint, it will be more consistent to make all dependencies go against pg_constraint rather than a mix of several catalogs. I think for primary keys pg_index is OK. However for the not-null case we have to use pg_constraint... So given that we end up having to code that anyways, it seems like it will end up being cleaner/consistent to always use the pg_constraint row(s). So +1 for using pg_constraint instead of pg_index from me. Next version. Changed dependencies to pg_constraint, removed handling of unique constraints for now, and made some enhancements so that views track dependencies on constraints even in subqueries. Should be close to final now. :-) diff --git a/doc/src/sgml/queries.sgml b/doc/src/sgml/queries.sgml index 416e599..a103229 100644 --- a/doc/src/sgml/queries.sgml +++ b/doc/src/sgml/queries.sgml @@ -886,10 +886,7 @@ SELECT product_id, p.name, (sum(s.units) * p.price) AS sales In this example, the columns literalproduct_id/literal, literalp.name/literal, and literalp.price/literal must be in the literalGROUP BY/ clause since they are referenced in -the query select list. (Depending on how the products -table is set up, name and price might be fully dependent on the -product ID, so the additional groupings could theoretically be -unnecessary, though this is not implemented.) The column +the query select list (but see below). The column literals.units/ does not have to be in the literalGROUP BY/ list since it is only used in an aggregate expression (literalsum(...)/literal), which represents the sales @@ -898,6 +895,18 @@ SELECT product_id, p.name, (sum(s.units) * p.price) AS sales /para para +If the products table is set up so that, +say, literalproduct_id/literal is the primary key, then it +would be enough to group by literalproduct_id/literal in the +above example, since name and price would +be firsttermfunctionally +dependent/firsttermindextermprimaryfunctional +dependency/primary/indexterm on the product ID, and so there +would be no ambiguity about which name and price value to return +for each product ID group. + /para + + para In strict SQL, literalGROUP BY/ can only group by columns of the source table but productnamePostgreSQL/productname extends this to also allow literalGROUP BY/ to group by columns in the diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml index 74021e8..8436d85 100644 --- a/doc/src/sgml/ref/select.sgml +++ b/doc/src/sgml/ref/select.sgml @@ -520,9 +520,12 @@ GROUP BY replaceable class=parameterexpression/replaceable [, ...] produces a single value computed across all the selected rows). When literalGROUP BY/literal is present, it is not valid for the commandSELECT/command list expressions to refer to -ungrouped columns except within aggregate functions, since there -would be more than one possible value to return for an ungrouped -column. +ungrouped columns except within aggregate functions or if the +ungrouped column is functionally dependent on the grouped columns, +since there would otherwise be more than one possible value to +return for an ungrouped column. A functional dependency exists if +the grouped columns (or a subset thereof) are the primary key of +the table containing the ungrouped column. /para /refsect2 diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c index 67c90a2..455fc6e 100644 --- a/src/backend/commands/view.c +++ b/src/backend/commands/view.c @@ -16,7 +16,9 @@ #include access/heapam.h #include access/xact.h +#include catalog/dependency.h #include catalog/namespace.h +#include catalog/pg_constraint.h #include commands/defrem.h #include commands/tablecmds.h #include commands/view.h @@ -390,6 +392,28 @@ UpdateRangeTableOfViewParse(Oid viewOid, Query *viewParse) } /* + * Walker to collect the constraintDeps fields of all Query nodes in a + * query tree into a single list. + */ +static bool +collectConstraintDeps_walker(Node *node, void *context) +{ + if (node == NULL) + return false; + else if (IsA(node, Query)) + { + Query *query = (Query *) node; + List **listp = (List **) context; + + *listp = list_concat_unique_oid(*listp, query-constraintDeps); + + return query_tree_walker(query, collectConstraintDeps_walker, context, 0); + } + else + return expression_tree_walker(node, collectConstraintDeps_walker, context); +} + +/* * DefineView * Execute a CREATE VIEW
Re: [HACKERS] patch for contrib/isn
On ons, 2010-08-04 at 19:32 +0200, Jan Otto wrote: patch against HEAD is attached and validated against a lot of previously wrong and correct hyphenated isbn. I think this module could use a regression test. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Update hstore % Doc
Hackers, I noticed that the hstore docs still document the = operator instead of %. This patch changes that. It also updates the first examples to us full SQL statements, because otherwise the use of = without surrounding single quotes was confusing. Best, David hstoredoc.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] Initial review of xslt with no limits patch
2010/8/6 David E. Wheeler da...@kineticode.com: On Aug 6, 2010, at 1:49 PM, Pavel Stehule wrote: yes it is one a possibility and probably best. The nice of this variant can be two forms like current variadic does - foo(.., a := 10, b := 10) or foo(.., variadic ARRAY[(a,10),(b,10)]) I started fiddling and got as far as this: CREATE TYPE pair AS ( key text, val text ); CREATE OR REPLACE FUNCTION pair(anyelement, anyelement) RETURNS pair LANGUAGE SQL AS $$ SELECT ROW($1, $2)::pair; $$; CREATE OR REPLACE FUNCTION pair(text, text) RETURNS pair LANGUAGE SQL AS $$ SELECT ROW($1, $2)::pair; $$; CREATE OPERATOR ~ ( LEFTARG = anyelement, RIGHTARG = anyelement, PROCEDURE = pair ); CREATE OPERATOR ~ ( LEFTARG = text, RIGHTARG = text, PROCEDURE = pair ); CREATE OR REPLACE FUNCTION foo(variadic pair[]) RETURNS SETOF text LANGUAGE SQL AS $$ -- SELECT unnest($1)::text SELECT $1[1].key UNION SELECT $1[1].val UNION SELECT $1[2].key UNION SELECT $1[2].val; $$; SELECT foo('this' ~ 'that', 1 ~ 4); Not bad, I think. I kind of like it. It reminds me how much I hate the % hstore construction operator, though (the new name for =). so there is only small step to proposed feature SELECT foo(this := 'that', 1 := 4) there is only one difference (but you cannot implement it now) * notation for key is same like for sql identifier - why: I would to clearly identify key and value. When I use a custom operator - like you did, it depends on implementation what is key, what is value. When you use a SQL identifier's notation for key, you can't to do a error Regards Pavel 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] MERGE Specification
On tor, 2010-08-05 at 16:35 +0100, Simon Riggs wrote: * DELETE is an extension to the standard, though supported by Oracle, DB2 and SQLServer and damn useful - SQL:2011 -- 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] Initial review of xslt with no limits patch
On Aug 6, 2010, at 2:12 PM, Pavel Stehule wrote: SELECT foo('this' ~ 'that', 1 ~ 4); Not bad, I think. I kind of like it. It reminds me how much I hate the % hstore construction operator, though (the new name for =). so there is only small step to proposed feature SELECT foo(this := 'that', 1 := 4) there is only one difference (but you cannot implement it now) * notation for key is same like for sql identifier - why: I would to clearly identify key and value. When I use a custom operator - like you did, it depends on implementation what is key, what is value. When you use a SQL identifier's notation for key, you can't to do a error Sorry, not following you here… 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] gincostestimate
=?UTF-8?B?SmFuIFVyYmHFhHNraQ==?= wulc...@wulczer.org writes: [ review of gincostestimate-0.19 ] I went through this patch, re-synced with current HEAD, and did some minor editorializing; a new version is attached. (Caution: I have not tested this beyond verifying that it still compiles.) Codewise I have one question: the patch changes a loop in ginvacuumcleanup from for (blkno = GIN_ROOT_BLKNO + 1; blkno npages; blkno++) to for (blkno = GIN_ROOT_BLKNO; blkno npages; blkno++) why should it now go through all blocks? I think this is correct. Before, vacuum had nothing useful to do on the root page so it just skipped it. Now, it needs to count the root page in the appropriate way in the stats it's gathering. The previous coding maybe could have used a comment, but this version is unsurprising. The patch has lots of statements like if ( GinPageIsLeaf(page) ), that is with extra space between the outer parenthesis and the condition, which AFAIK is not the project style. I guess pgindent fixes that, so it's no big deal. There are lines with elog(NOTICE) that are #if 0, they probably should either become elog(DEBUGX) or get removed. I fixed the latter and cleaned up some of the formatting violations, though not all. I dunno if anyone feels like running pgindent on the patch at the moment. I think there are two big problems left before this patch can be applied: 1. The use of rd_amcache is very questionable. There's no provision for updates executed by one session to get reflected into stats already cached in another session. You could fix that by forcing relcache flushes whenever you update the stats, as btree does: /* send out relcache inval for metapage change */ if (!InRecovery) CacheInvalidateRelcache(rel); However I think that's probably a Really Bad Idea, because it would result in extremely frequent relcache flushes, and those are expensive. (The reason this mechanism is good for btree is it only needs to update its cache after a root page split, which is infrequent.) My advice is to drop the use of rd_amcache completely, and just have the code read the metapage every time gincostestimate runs. If you don't like that then you're going to need to think hard about how often to update the cache and what can drive that operation at the right times. BTW, another problem that would need to be fixed if you keep this code is that ginUpdateStatInPlace wants to force the new values into rd_amcache, which (a) is pretty useless and (b) risks a PANIC on palloc failure, because it's called from a critical section. 2. Some of the calculations in gincostestimate seem completely bogus. In particular, this bit: /* calc partial match: we don't need a search but an index scan */ *indexStartupCost += partialEntriesInQuals * numEntryPages / numEntries; is surely wrong, because the number being added to indexStartupCost is a pure ratio not scaled by any cost unit. I don't understand what this number is supposed to be, so it's not clear what cost variable ought to be included. This equation doesn't seem amazingly sane either: /* cost to scan data pages for each matched entry */ pagesFetched = ceil((searchEntriesInQuals + partialEntriesInQuals) * numDataPages / numEntries); This has pagesFetched *decreasing* as numEntries increases, which cannot be right can it? Also, right after that step, there's a bit of code that re-estimates pagesFetched from selectivity and uses the larger value. Fine, but why are you only applying that idea here and not to the entry-pages calculation done a few lines earlier? regards, tom lane binfclKHw9sIz.bin Description: gincostestimate-0.20.gz -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] review: xml_is_well_formed
On Fri, Aug 6, 2010 at 4:52 PM, Peter Eisentraut pete...@gmx.net wrote: On fre, 2010-08-06 at 07:31 -0400, Robert Haas wrote: What about making the function sensitive to the XML OPTION, such that: test=# SET xmloption TO DOCUMENT; SET text=# SELECT xml_is_well_formed('foo'); xml_is_well_formed f (1 row) That will make using this function a huge hassle, won't it? Functions that do different things depending on GUC settings are usually troublesome. Having three functions would be more sensible if we need all three behaviors, but I don't see why we do. Upthread you opined that this function should essentially indicate whether a cast to type xml would succeed, and observing the xmloption is an essential part of that. I had assumed the original patch actually did that. Oh. I didn't realize the casting behavior was already dependent on the GUC. Yuck. Maybe we should following the GUC after all, then. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] Initial review of xslt with no limits patch
Peter Eisentraut pete...@gmx.net writes: On fre, 2010-08-06 at 13:01 -0400, Tom Lane wrote: 2. I'm not sure whether we ought to auto-single-quote the values. If we don't, how hard is it for users to properly quote nonconstant parameter values? (Will quote_literal work, or are the quoting rules different for libxslt?) If we do, are we giving up functionality someone cares about? Not every parameter is a string. So I gather, but what else is there, and do we actually want to expose the other alternatives in xslt_process()? If we don't auto-quote, we need to provide some sort of quote_xslt() function that will apply the appropriate quoting/escaping to deal with an arbitrary string value. 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] Initial review of xslt with no limits patch
David E. Wheeler da...@kineticode.com writes: On Aug 6, 2010, at 2:12 PM, Pavel Stehule wrote: so there is only small step to proposed feature SELECT foo(this := 'that', 1 := 4) Sorry, not following you here Pavel doesn't understand no ;-) 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] Update hstore % Doc
David E. Wheeler david.whee...@pgexperts.com writes: I noticed that the hstore docs still document the = operator instead of %. This patch changes that. It looks to me like you are changing the examples of the I/O representation ... which did NOT change. 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] Update hstore % Doc
On Aug 6, 2010, at 3:16 PM, Tom Lane wrote: I noticed that the hstore docs still document the = operator instead of %. This patch changes that. It looks to me like you are changing the examples of the I/O representation ... which did NOT change. Hrm? The first few examples at the top? I find them confusing because there are no single quotes around them, so they look like the use of the deprecated = operator (especially the first two). Just look at: http://developer.postgresql.org/pgdocs/postgres/hstore.html Maybe that's standard in the docs, but it seems weird to me. It's a minor point, though. We definitely need to document the `text % text` constructor rather than the deprecated `text = text` constructor. And I hate to say it, but % is awful. Sorry, I know I'm probably opening a can of worms here, and I did finally push % after all the arguments, but coming back to it fresh it just looks bizarre to me. Maybe that ship has sailed, though, and I'm just being difficult. 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] GROUPING SETS revisited
On Thu, Aug 05, 2010 at 04:46:51PM +0200, Pavel Stehule wrote: I am sending a updated version. I've been looking at the changes to gram.y, and noted the comment under func_expr where you added CUBE and ROLLUP definitions. It says that CUBE can't be a reserved keyword because it's already used in the cube contrib module. But then the changes to kwlist.h include this: + PG_KEYWORD(cube, CUBE, RESERVED_KEYWORD) ... + PG_KEYWORD(rollup, ROLLUP, RESERVED_KEYWORD) ...and CUBE and ROLLUP are added in gram.y under the reserved_keyword list. I realize things like CURRENT_TIME, that also have special entries in the func_expr grammar, are also reserved keywords, but this all seems at odds with the comment. What am I missing? Is the comment simply pointing out that the designation of CUBE and ROLLUP as reserved keywords will have to change at some point, but it hasn't been implemented yet (or no one has figured out how to do it)? -- Joshua Tolley / eggyknap End Point Corporation http://www.endpoint.com signature.asc Description: Digital signature
Re: [HACKERS] Update hstore % Doc
David E. Wheeler david.whee...@pgexperts.com writes: On Aug 6, 2010, at 3:16 PM, Tom Lane wrote: It looks to me like you are changing the examples of the I/O representation ... which did NOT change. Hrm? The first few examples at the top? I find them confusing because there are no single quotes around them, so they look like the use of the deprecated = operator (especially the first two). Just look at: Yeah, but there's a sentence in front of them that says specifically that these are examples of the text representation, not pieces of SQL. And I hate to say it, but % is awful. Yeah, I know, but you can't have =. Unless you can persuade the SQL committee to back off their syntax choice for parameters. (:= would have been a lot better ...) 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] Update hstore % Doc
David E. Wheeler david.whee...@pgexperts.com writes: We definitely need to document the `text % text` constructor BTW, there isn't any % constructor anymore --- we agreed to provide only the hstore(text, text) constructor. 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] Initial review of xslt with no limits patch
On Fri, Aug 06, 2010 at 11:48:58PM +0300, Peter Eisentraut wrote: On fre, 2010-08-06 at 21:31 +0200, Pavel Stehule wrote: It must not be a function. Just I missing any tool that helps with complex structured data. This proposed kind functions has one advantage - there isn't necessary any change in parser. Yes, I can use a pair of arrays, I can use a one array with seq name, value, I can use a custom parser. But nothing from these offers a comfort or readability for example a Perl's hash tables. Maybe you should just use PL/XSLT. :-) When's that going into the tree? Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Update hstore % Doc
On Aug 6, 2010, at 3:38 PM, Tom Lane wrote: We definitely need to document the `text % text` constructor BTW, there isn't any % constructor anymore --- we agreed to provide only the hstore(text, text) constructor. Oh, I must've been looking at an older checkout, then. Never mind. 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] Update hstore % Doc
On Fri, Aug 6, 2010 at 6:49 PM, David E. Wheeler david.whee...@pgexperts.com wrote: On Aug 6, 2010, at 3:38 PM, Tom Lane wrote: We definitely need to document the `text % text` constructor BTW, there isn't any % constructor anymore --- we agreed to provide only the hstore(text, text) constructor. Oh, I must've been looking at an older checkout, then. Never mind. And also - it was never the constructor. It was briefly the slice operator. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] [JDBC] Trouble with COPY IN
On Wed, 28 Jul 2010, James William Pye wrote: Not directly regarding your patch, but while the discussion is in the general area. I think it would be wise to throw an error when non-empty CopyData messages are received after CopyData(EOF). Chances are that the user is making a mistake and should be notified of it. As this is also the direction that Tom Lane indicated we should go, here is a patch which errors out after receiving any more copy data past the EOF marker. This also fixes the protocol problem I previously brought up because the act of checking to see if there is any more data does ensure that if there isn't any more data in the current buffer, that we wait for the client to provide CopyDone/Fail. Kris Jurka*** a/src/backend/commands/copy.c --- b/src/backend/commands/copy.c *** *** 2058,2069 CopyFrom(CopyState cstate) int16 fld_count; ListCell *cur; ! if (!CopyGetInt16(cstate, fld_count) || ! fld_count == -1) { done = true; break; } if (fld_count != attr_count) ereport(ERROR, --- 2058,2090 int16 fld_count; ListCell *cur; ! if (!CopyGetInt16(cstate, fld_count)) { done = true; break; } + + if (fld_count == -1) + { + /* +* Reached EOF. In protocol version 3, we must wait for +* the protocol end of copy (CopyDone/Fail). If we +* receive any more copy data after EOF, complain. +*/ + if (cstate-copy_dest == COPY_NEW_FE) + { + int8 unused; + if (CopyGetData(cstate, unused, sizeof(unused), sizeof(unused))) + { + ereport(ERROR, + (errcode(ERRCODE_BAD_COPY_FILE_FORMAT), + errmsg(received copy data after EOF marker))); + } else { + done = true; + break; + } + } + } if (fld_count != attr_count) ereport(ERROR, -- 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 (for 9.1) string functions
2010/7/26 Robert Haas robertmh...@gmail.com: Come to think of it, have we checked that the behavior of LEFT, RIGHT, REVERSE, etc. is the same on other DBs, especially as far as nulls, empty strings, too-large or negative subscripts, etc is concerned? Is CONCAT('foo', NULL) = 'foo' really the behavior that everyone else implements here? I made a discussion page in wiki for the compatibility issue. http://wiki.postgresql.org/wiki/String_Functions_and_Operators_Compatibility Please fill empty cells and fix wrong descriptions. * concat() is not compatible between MySQL and Oracle/DB2. Which do we buy? * How do other databases behave in left() and right() with negative lengths? * Are there any databases that has similar features with format() or sprintf() ? And why does CONCAT() take a variadic ANY argument? Shouldn't that be variadic TEXT? I think we have no other choice but to use VARIADIC any for variadic functions. We have all combinations of argument types for || operator, (text, text), (text, any), (any, text), but we cannot use such codes for variadic functions -- they have no limits of argument numbers. And in the case, the functions should be STABLE because they convert arguments to text in it with typout functions that might be STABLE. IMHO, I'd repeat, syntax for format() is a bad choice because it cannot concatenate multiple arguments without separator, though RAISE also uses it. %s format in sprintf() or {n} syntax in C#'s String.Format() seems to be a better design. -- 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] [JDBC] Trouble with COPY IN
On Aug 6, 2010, at 4:31 PM, Kris Jurka wrote: binary-copy-end-v2.patch I think there's a snag in the patch: postgres=# COPY data FROM '/Users/jwp/DATA.bcopy' WITH BINARY; ERROR: row field count is -1, expected 1 CONTEXT: COPY data, line 4 Probably a quick/small fix away, I imagine. But, I was able to trigger the new ERROR with py-postgresql: import postgresql as pg db=pg.open('localhost/postgres') q=db.prepare('copy data FROM STDIN WITH BINARY') from itertools import chain import sys db.pq.tracer = sys.stderr.write q.load_rows(chain(open('/Users/jwp/DATA.bcopy', 'rb'), (b'EXTRA',))) ↑ B(25): b'B\x00\x00\x00\x18\x00py:0x1268b30\x00\x00\x00\x00\x00\x00\x00' ↑ E(10): b'E\x00\x00\x00\t\x00\x00\x00\x00\x01' ↑ S(5): b'S\x00\x00\x00\x04' ↓ b'2'(0): b'' ↓ b'G'(5): b'\x01\x00\x01\x00\x01' ↑__(7): b'PGCOPY\n' ↑__(3): b'\xff\r\n' ↑__(41): b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x04\x00\x00\x00\x01\x00\x01\x00\x00\x00\x04\x00\x00\x00\x02\x00\x01\x00\x00\x00\x04\x00\x00\x00\x03\xff\xff' ↑__(5): b'EXTRA' ↑ c(5): b'c\x00\x00\x00\x04' ↑ S(5): b'S\x00\x00\x00\x04' ↓ b'E'(95): b'SERROR\x00C22P04\x00Mreceived copy data after EOF marker\x00WCOPY data, line 4\x00Fcopy.c\x00L2081\x00RCopyFrom\x00\x00' ↓ b'Z'(1): b'I' Traceback (most recent call last): File stdin, line 1, in module snip File /Library/Frameworks/Python.framework/Versions/3.1/lib/python3.1/site-packages/postgresql/driver/pq3.py, line 462, in raise_server_error raise server_error postgresql.exceptions.BadCopyError: received copy data after EOF marker CODE: 22P04 LOCATION: File 'copy.c', line 2081, in CopyFrom from SERVER CONTEXT: COPY data, line 4 STATEMENT: [prepared] sql_parameter_types: [] statement_id: py:0x1268b30 string: copy data FROM STDIN WITH BINARY CONNECTION: [idle] client_address: ::1/128 client_port: 63922 version: PostgreSQL 9.1devel on x86_64-apple-darwin10.4.0, compiled by GCC i686-apple-darwin10-gcc-4.2.1 (GCC) 4.2.1 (Apple Inc. build 5664), 64-bit CONNECTOR: [Host] pq://jwp:*...@localhost:5432/postgres category: None DRIVER: postgresql.driver.pq3.Driver -- 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] Surprising dead_tuple_count from pgstattuple
2010/8/7 Gordon Shannon gordo...@gmail.com: 1. I delete 10,000 rows. pgstattuple.dead_tuple_count - 1 2. I delete 15,000 more rows. pgstattuple.dead_tuple_count - 15000 ?? pgstattuple now appears to count the earlier 10K deleted tuples as no longer dead, but free space. I think it's expected behavior that comes from HOT page reclaim. The second DELETE not only deleted rows but also removed physical tuples that were deleted in 1. Missing dead rows were pruned by HOT. -- 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] Surprising dead_tuple_count from pgstattuple
On Fri, Aug 6, 2010 at 9:11 PM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: 2010/8/7 Gordon Shannon gordo...@gmail.com: 1. I delete 10,000 rows. pgstattuple.dead_tuple_count - 1 2. I delete 15,000 more rows. pgstattuple.dead_tuple_count - 15000 ?? pgstattuple now appears to count the earlier 10K deleted tuples as no longer dead, but free space. I think it's expected behavior that comes from HOT page reclaim. The second DELETE not only deleted rows but also removed physical tuples that were deleted in 1. Missing dead rows were pruned by HOT. What would have triggered a HOT prune at any point in this operation? And why would it have reclaimed all of the deleted rows? My thought would be is autovacuum running in the background in between these commands?. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] Functional dependencies and GROUP BY
Peter Eisentraut pete...@gmx.net writes: Next version. Changed dependencies to pg_constraint, removed handling of unique constraints for now, and made some enhancements so that views track dependencies on constraints even in subqueries. Should be close to final now. :-) I've committed this with some revisions, notably: The view.c changes were fundamentally wrong. The right place to extract dependencies from a parsetree is in dependency.c, specifically find_expr_references_walker. The way you were doing it meant that dependencies on constraints would only be noticed for views, not for other cases such as stored plans. I rewrote the catalog search to look only at pg_constraint, not using pg_index at all. I think this will be easier to extend to the case of looking for UNIQUE + NOT NULL, whenever we get around to doing that. I also moved the search into catalog/pg_constraint.c, because it didn't seem to belong in parse_agg (as hinted by the large number of #include additions you had to make to put it there). I put in a bit of caching logic to prevent repeating the search for multiple Vars of the same relation. Tests or no tests, I can't believe that's going to be cheap enough that we want to repeat it over and over... 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] refactoring comment.c
(2010/08/07 0:02), Robert Haas wrote: At PGCon, we discussed the possibility that a minimal SE-PostgreSQL implementation would need little more than a hook in ExecCheckRTPerms() [which we've since added] and a security label facility [for which KaiGai has submitted a patch]. I actually sat down to write the security label patch myself while we were in Ottawa, but quickly ran into difficulties: while the hook we have now can't do anything useful with objects other than relations, it's pretty clear from previous discussions on this topic that the demand for labels on other kinds of objects is not going to go away. Rather than adding additional syntax to every object type in the system (some of which don't even have ALTER commands at present), I suggested basing the syntax on the existing COMMENT syntax. After some discussion[1], we seem to have settled on the following: SECURITY LABEL [ FORprovider ] ONobject class object name IS 'label'; At present, there are some difficulties with generalizing this syntax to other object types. As I found out when I initially set out to write this patch, it'd basically require duplicating all of comment.c, which is an unpleasant prospect, because that file is big and crufty; it has a large amount of internal duplication. Furthermore, the existing locking mechanism that we're using for comments is known to be inadequate[2]. Dropping a comment while someone else is in the midst of commenting on it leaves orphaned comments lying around in pg_(sh)description that could later be inherited by a new object. That's a minor nuisance for comments and would be nice to fix, but is obviously a far larger problem for security labels, where even a small chance of randomly mislabeling an object is no good. So I wrote a patch. The attached patch factors out all of the code in comment.c that is responsible for translating parser representations into a new file parser/parse_object.c, leaving just the comment-specific stuff in commands/comment.c. It also adds appropriate locking, so that concurrent COMMENT/DROP scenarios don't leave behind leftovers. It's a fairly large patch, but the changes are extremely localized: comment.c gets a lot smaller, and parse_object.c gets bigger by a slightly smaller amount. Any comments? (ha ha ha...) [1] http://archives.postgresql.org/pgsql-hackers/2010-07/msg01328.php [2] http://archives.postgresql.org/pgsql-hackers/2010-07/msg00351.php Thanks for your efforts. I believe the get_object_address() enables to implement security label features on various kind of database objects. I tried to look at the patch. Most part is fine, but I found out two issues. On the object_exists(), when we verify existence of a large object, it needs to scan pg_largeobject_metadata, instead of pg_largeobject. When we implement pg_largeobject_metadata catalog, we decided to set LargeObjectRelationId on object.classId due to the backend compatibility. |/* | * For object types that have a relevant syscache, we use it; for | * everything else, we'll have to do an index-scan. This switch | * sets either the cache to be used for the syscache lookup, or the | * index to be used for the index scan. | */ |switch (address.classId) |{ |case RelationRelationId: |cache = RELOID; |break; | : |case LargeObjectRelationId: |indexoid = LargeObjectMetadataOidIndexId; |break; | : | } | |/* Found a syscache? */ |if (cache != -1) |return SearchSysCacheExists1(cache, ObjectIdGetDatum(address.objectId)); | |/* No syscache, so examine the table directly. */ |Assert(OidIsValid(indexoid)); |ScanKeyInit(skey[0], ObjectIdAttributeNumber, BTEqualStrategyNumber, |F_OIDEQ, ObjectIdGetDatum(address.objectId)); |rel = heap_open(address.classId, AccessShareLock); ^^^ - It tries to open pg_largeobject |sd = systable_beginscan(rel, indexoid, true, SnapshotNow, 1, skey); |found = HeapTupleIsValid(systable_getnext(sd)); |systable_endscan(sd); |heap_close(rel, AccessShareLock); |return found; | } Although no caller invokes get_object_address() with lockmode = NoLock, isn't it necessary to skip locking if NoLock was given. |/* | * If we're dealing with a relation or attribute, then the relation is | * already locked. If we're dealing with any other type of object, we need | * to lock it and then verify that it still exists. | */ |if (address.classId != RelationRelationId) |{ |if (IsSharedRelation(address.classId)) |LockSharedObject(address.classId, address.objectId, 0, lockmode); |else |LockDatabaseObject(address.classId, address.objectId, 0, lockmode); |/* Did it go away while we were waiting for the lock? */ |if (!object_exists(address)) |elog(ERROR, cache lookup failed