Re: [HACKERS] final patch - plpgsql: for-in-array
On Wed, Nov 24, 2010 at 1:06 AM, Tom Lane t...@sss.pgh.pa.us wrote: =?ISO-8859-1?Q?C=E9dric_Villemain?= cedric.villemain.deb...@gmail.com writes: I think you (Robert) misunderstood dramatically what Pavel try to do. Pavel did an excellent optimization work for a specific point. This specific point looks crucial for me in the current behavior of PostgreSQL[1]. AFAIS Pavel didn't want to implement a genious syntax, but an optimization feature. As near as I can tell, Pavel is bullheadedly insisting on adding new syntax, not on the optimization aspect of it. I already pointed out how he could get 100% of the performance benefit using the existing syntax, but he doesn't appear to be willing to pursue that route. Right, that was my impression, too. But, I think this may be partly a case of people talking past each other. My impression of this conversation was a repetition of this sequence: A: This syntax is bad. B: But it's way faster! ...which makes no sense. However, what I now think is going on here is that there are really two separate things that are wished for here - a more compact syntax, and a performance improvement. And taken separately, I agree with both of those desires. PL/pgsql is an incredibly clunky language syntactically, and it's also slow. A patch that improves either one of those things has value, whether or not it also does the other one. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] final patch - plpgsql: for-in-array
Robert Haas robertmh...@gmail.com writes: Right, that was my impression, too. But, I think this may be partly a case of people talking past each other. My impression of this conversation was a repetition of this sequence: A: This syntax is bad. B: But it's way faster! ...which makes no sense. However, what I now think is going on here is that there are really two separate things that are wished for here - a more compact syntax, and a performance improvement. And taken separately, I agree with both of those desires. PL/pgsql is an incredibly clunky language syntactically, and it's also slow. A patch that improves either one of those things has value, whether or not it also does the other one. I understand the desire for nicer syntax, in the abstract. I'm just unimpressed by this particular change, mainly because I'm afraid that it will make syntax-error behaviors worse and foreclose future options for other changes to FOR. If it were necessary to change the syntax to get the performance benefit, I might think that on balance we should do so; but it isn't. 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] final patch - plpgsql: for-in-array
Hello 2010/11/24 Tom Lane t...@sss.pgh.pa.us: Robert Haas robertmh...@gmail.com writes: Right, that was my impression, too. But, I think this may be partly a case of people talking past each other. My impression of this conversation was a repetition of this sequence: A: This syntax is bad. B: But it's way faster! ...which makes no sense. However, what I now think is going on here is that there are really two separate things that are wished for here - a more compact syntax, and a performance improvement. And taken separately, I agree with both of those desires. PL/pgsql is an incredibly clunky language syntactically, and it's also slow. A patch that improves either one of those things has value, whether or not it also does the other one. I understand the desire for nicer syntax, in the abstract. I'm just unimpressed by this particular change, mainly because I'm afraid that it will make syntax-error behaviors worse and foreclose future options for other changes to FOR. If it were necessary to change the syntax to get the performance benefit, I might think that on balance we should do so; but it isn't. I am for any readable syntax. It must not be FOR-IN-ARRAY. I understand to problem with syntax-error checking. But I am not sure if some different loop with control variable can be less ugliness in language. Cannot we rewrite a parsing for-clause be more robust? I agree with you, so there can be a other request in future. And if I remember well, there was only few changes in other statements (on parser level) and significant changes in FOR. probably some hypothetical statement should be (my opinion) FOR var [, vars] UNKNOWN SYNTAX LOOP ... END LOOP; PL/SQL uses a enhanced FOR FOR var IN collection.first .. collection.last LOOP END LOOP; From my view a introduction of new keyword should be a higher risk so I don't would to do. Regards Pavel Stehule 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] final patch - plpgsql: for-in-array
On Wed, Nov 24, 2010 at 10:33 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Right, that was my impression, too. But, I think this may be partly a case of people talking past each other. My impression of this conversation was a repetition of this sequence: A: This syntax is bad. B: But it's way faster! ...which makes no sense. However, what I now think is going on here is that there are really two separate things that are wished for here - a more compact syntax, and a performance improvement. And taken separately, I agree with both of those desires. PL/pgsql is an incredibly clunky language syntactically, and it's also slow. A patch that improves either one of those things has value, whether or not it also does the other one. I understand the desire for nicer syntax, in the abstract. I'm just unimpressed by this particular change, mainly because I'm afraid that it will make syntax-error behaviors worse and foreclose future options for other changes to FOR. If it were necessary to change the syntax to get the performance benefit, I might think that on balance we should do so; but it isn't. It'd be nicer syntax if there were some way to have the keyword not adjacent to the arbitrary expression. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] final patch - plpgsql: for-in-array
On Mon, Nov 22, 2010 at 11:55 PM, Pavel Stehule pavel.steh...@gmail.com wrote: ok, I can only recapitulate so this feature was proposed cca two months ago, and minimally Tom and maybe you did agreement - with request on syntax - do you remember? I am little bit tired so this agreement was changed when I spent my time with this. I went back and reread the thread I believe you're speaking about. The first post is here: http://archives.postgresql.org/pgsql-hackers/2010-09/msg01945.php I cannot find one single email on that thread where Tom or I or anyone else endorsed the syntax you've proposed here; indeed, it and some other suggestions were roundly criticized. You responded to that by saying that the arguments against it were all wrong, but no one other than you ever appeared to believe that. There are a few emails on that thread where other people agreed that it would be nice, in theory, to have some syntax for this, but there is not one single email that I see saying that any syntax you proposed was a good one. If you read that thread and concluded that there was consensus to implement this syntax, you did not read it very carefully. If we had ELEMENT as a reserved keyword (which apparently it is in some versions of the SQL standard), maybe FOR ELEMENT wunk IN wunkarray... would be sufficiently unambiguous. But it's not even an unreserved keyword right now, and I have a hard time thinking it would be worth reserving it just for this. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] final patch - plpgsql: for-in-array
2010/11/23 Robert Haas robertmh...@gmail.com: On Mon, Nov 22, 2010 at 11:55 PM, Pavel Stehule pavel.steh...@gmail.com wrote: ok, I can only recapitulate so this feature was proposed cca two months ago, and minimally Tom and maybe you did agreement - with request on syntax - do you remember? I am little bit tired so this agreement was changed when I spent my time with this. I went back and reread the thread I believe you're speaking about. The first post is here: http://archives.postgresql.org/pgsql-hackers/2010-09/msg01945.php Here perhaps ? (or before) http://archives.postgresql.org/pgsql-hackers/2010-09/msg01983.php I cannot find one single email on that thread where Tom or I or anyone else endorsed the syntax you've proposed here; Nah, but you didn't disagree on the main idea, you just said : 'like Tom I agree that syntax must be uptaded to something beter' , more or less indeed, it and some other suggestions were roundly criticized. You responded to that by saying that the arguments against it were all wrong, but no one other than you ever appeared to believe that. There are a few emails on that thread where other people agreed that it would be nice, in theory, to have some syntax for this, but there is not one single email that I see saying that any syntax you proposed was a good one. If you read that thread and concluded that there was consensus to implement this syntax, you did not read it very carefully. I think you (Robert) misunderstood dramatically what Pavel try to do. Pavel did an excellent optimization work for a specific point. This specific point looks crucial for me in the current behavior of PostgreSQL[1]. AFAIS Pavel didn't want to implement a genious syntax, but an optimization feature. I don't care about syntax, I care with Tom explanation on that. but no more. I care with the idea that this patch is just a quick way to cut the iceberg. It is. and ? And we might do it better with more deep analysis and refactoring more stuff, I agree... Still this patch is interesting enought from perf point of view to not trash it that quickly, IMO. If we had ELEMENT as a reserved keyword (which apparently it is in some versions of the SQL standard), maybe FOR ELEMENT wunk IN wunkarray... would be sufficiently unambiguous. But it's not even an unreserved keyword right now, and I have a hard time thinking it would be worth reserving it just for this. I am not aware of SQL spec precisely about that. David, did your recent post about UNNEST stuff looks relevant in this thread ? I mean can we elaborate something from your suggestion to improve the situation of the current patch (and vice-versa) ? [1] data compression in the array allow to insert billions of data for a small size print. (I know it is not pure design, it is just pure end-user very effective solution) -- Cédric Villemain 2ndQuadrant http://2ndQuadrant.fr/ PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] final patch - plpgsql: for-in-array
On Tue, Nov 23, 2010 at 8:56 PM, Cédric Villemain cedric.villemain.deb...@gmail.com wrote: 2010/11/23 Robert Haas robertmh...@gmail.com: On Mon, Nov 22, 2010 at 11:55 PM, Pavel Stehule pavel.steh...@gmail.com wrote: ok, I can only recapitulate so this feature was proposed cca two months ago, and minimally Tom and maybe you did agreement - with request on syntax - do you remember? I am little bit tired so this agreement was changed when I spent my time with this. I went back and reread the thread I believe you're speaking about. The first post is here: http://archives.postgresql.org/pgsql-hackers/2010-09/msg01945.php Here perhaps ? (or before) http://archives.postgresql.org/pgsql-hackers/2010-09/msg01983.php Dang. You're right. I stand corrected. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] final patch - plpgsql: for-in-array
=?ISO-8859-1?Q?C=E9dric_Villemain?= cedric.villemain.deb...@gmail.com writes: I think you (Robert) misunderstood dramatically what Pavel try to do. Pavel did an excellent optimization work for a specific point. This specific point looks crucial for me in the current behavior of PostgreSQL[1]. AFAIS Pavel didn't want to implement a genious syntax, but an optimization feature. As near as I can tell, Pavel is bullheadedly insisting on adding new syntax, not on the optimization aspect of it. I already pointed out how he could get 100% of the performance benefit using the existing syntax, but he doesn't appear to be willing to pursue that route. 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] final patch - plpgsql: for-in-array
2010/11/24 Robert Haas robertmh...@gmail.com: On Tue, Nov 23, 2010 at 8:56 PM, Cédric Villemain cedric.villemain.deb...@gmail.com wrote: 2010/11/23 Robert Haas robertmh...@gmail.com: On Mon, Nov 22, 2010 at 11:55 PM, Pavel Stehule pavel.steh...@gmail.com wrote: ok, I can only recapitulate so this feature was proposed cca two months ago, and minimally Tom and maybe you did agreement - with request on syntax - do you remember? I am little bit tired so this agreement was changed when I spent my time with this. I went back and reread the thread I believe you're speaking about. The first post is here: http://archives.postgresql.org/pgsql-hackers/2010-09/msg01945.php Here perhaps ? (or before) http://archives.postgresql.org/pgsql-hackers/2010-09/msg01983.php Dang. You're right. I stand corrected. Sorry, I though so you and Tom hasn't a problem with syntax FOR-IN-ARRAY (what is a Kevin Grittner's proposal). So problematic is just my original proposal FOR-IN-expr, but proposed feature isn't rejected. My proposal isn't really genial - is true so first my motivation was to replace a pattern array_lower(var,1)..array_upper(var,1). It's relative simple in ADA, statement FOR is defined over range type, and relative impossible in PL/pgSQL, where range type doesn't exists. Some special construct in PL/pgSQL can to solve iteration over array significantly better and simpler then any other solution - this really must not be syntax FOR-IN-ARRAY - and with any next test and next code checking I am more sure: why: * there is clean indicia so developer wants to process all items in array * there isn't random access to array * is possibility for a reuse varlena types stored in array without a temporal copy I am sorry, so I didn't speaking about these facts ear -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] final patch - plpgsql: for-in-array
sorry, there was a broken message 2010/11/24 Pavel Stehule pavel.steh...@gmail.com: 2010/11/24 Robert Haas robertmh...@gmail.com: On Tue, Nov 23, 2010 at 8:56 PM, Cédric Villemain cedric.villemain.deb...@gmail.com wrote: 2010/11/23 Robert Haas robertmh...@gmail.com: On Mon, Nov 22, 2010 at 11:55 PM, Pavel Stehule pavel.steh...@gmail.com wrote: ok, I can only recapitulate so this feature was proposed cca two months ago, and minimally Tom and maybe you did agreement - with request on syntax - do you remember? I am little bit tired so this agreement was changed when I spent my time with this. I went back and reread the thread I believe you're speaking about. The first post is here: http://archives.postgresql.org/pgsql-hackers/2010-09/msg01945.php Here perhaps ? (or before) http://archives.postgresql.org/pgsql-hackers/2010-09/msg01983.php Dang. You're right. I stand corrected. Sorry, I though so you and Tom hasn't a problem with syntax FOR-IN-ARRAY (what is a Kevin Grittner's proposal). So problematic is just my original proposal FOR-IN-expr, but proposed feature isn't rejected. Sorry, I though so you and Tom hasn't a problem with syntax FOR-IN-ARRAY (what is a Kevin Grittner's proposal). I though so problematic is just my original proposal FOR-IN-expr, but proposed feature isn't a problem. My proposal isn't really genial - is true so first my motivation was to replace unwished pattern array_lower(var,1)..array_upper(var,1). It's relative simple in ADA, where statement FOR is defined over range type, and relative impossible in PL/pgSQL, where range type doesn't exists yet. Some special construct in PL/pgSQL can to solve iteration over array significantly better and simpler then any other solution - there must not be used the syntax FOR-IN-ARRAY - with any next test and next code checking I am more sure: why?: * there is clean indicia so developer wants to process all items in array, or almost all * there isn't random access to array!! * is possibility for a reuse varlena's value stored in array without a temporal copy - with maybe some trick!! * there is a very low overhead I am sorry, so I didn't speaking about these advices early. I though about other possible syntax - what do you think about FOR var OVER expr LOOP ... END LOOP ? OVER is keyword now Regards Pavel -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] final patch - plpgsql: for-in-array
Hi, with the FOR e IN SELECT UNNEST(a) construct there is an issue again related to the unresting of composite type arrays: BEGIN; CREATE TYPE truple AS (i integer, a text, b text); DO $SQL$ DECLARE start_time timestamp; t truple; ta truple[] := ARRAY( select ROW(s.i, 'A' || (s.i)::text, 'B' || (s.i)::text )::truple from generate_series(1, 1) as s(i) ); i integer := 1; BEGIN start_time := clock_timestamp(); FOR t IN SELECT UNNEST(ta) LOOP raise info 't is %', t; i := i + 1; END LOOP; RAISE INFO 'looped in %', clock_timestamp() - start_time; END; $SQL$; ROLLBACK; fails with ERROR: invalid input syntax for integer: (1,A1,B1) CONTEXT: PL/pgSQL function inline_code_block line 8 at FOR over SELECT rows So to UNNEST such an array one has to SELECT * FROM UNNEST(a) to be able loop there like: BEGIN; CREATE TYPE truple AS (i integer, a text, b text); DO $SQL$ DECLARE start_time timestamp; t truple; ta truple[] := ARRAY( select ROW(s.i, 'A' || (s.i)::text, 'B' || (s.i)::text )::truple from generate_series(1, 1) as s(i) ); i integer := 1; BEGIN start_time := clock_timestamp(); FOR t IN SELECT * FROM UNNEST(ta) LOOP raise info 't is %', t; i := i + 1; END LOOP; RAISE INFO 'looped in %', clock_timestamp() - start_time; END; $SQL$; ROLLBACK; Is it a bug or a feature? And if the second, then any work on optimizing FOR e IN SELECT UNNEST(a) should probably include FOR e IN SELECT * FROM UNNEST(a) statement optimizations. Also, would the suggested FOR-IN-ARRAY construct loop in such a composite type arrays? Best regards, -- Valenine Gogichashvili On Thu, Nov 18, 2010 at 8:16 PM, Tom Lane t...@sss.pgh.pa.us wrote: Pavel Stehule pavel.steh...@gmail.com writes: 2010/11/18 Tom Lane t...@sss.pgh.pa.us: The problem here is that FOR is a syntactic choke point: it's already overloaded with several different sub-syntaxes that are quite difficult to separate. Adding another one makes that worse, with the consequences that we might misinterpret the user's intent, leading either to misleading/unhelpful error messages or unexpected runtime behavior. yes, this argument is correct - but we can rearange a parser rules related to FOR statement. It can be solved. No, it can't. The more things that can possibly follow FOR, the less likely that you correctly guess which one the user had in mind when faced with something that's not quite syntactically correct. Or maybe it *is* syntactically correct, only not according to the variant that the user thought he was invoking. We've seen bug reports of this sort connected with FOR already; in fact I'm pretty sure you've responded to a few yourself. Adding more variants *will* make it worse. We need a decent return on investment for anything we add here, and this proposal just doesn't offer enough benefit. 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] final patch - plpgsql: for-in-array
On Wed, Nov 17, 2010 at 7:08 PM, Jaime Casanova ja...@2ndquadrant.com wrote: On Thu, Sep 30, 2010 at 7:46 AM, Pavel Stehule pavel.steh...@gmail.com wrote: Hello this patch implement a new iteration construct - iteration over an array. The sense of this new iteration is: * a simple and cleaner syntax i will start the review of this one... so, what is the concensus for this patch? return with feedback? reject the patch on the grounds that we should go fix unnest() if it slow? something else? the patch itself works as advertised (in functionality) but i haven't make much performance tests to see if we actually win something -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: 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] final patch - plpgsql: for-in-array
On Mon, Nov 22, 2010 at 6:21 AM, Valentine Gogichashvili val...@gmail.com wrote: Hi, with the FOR e IN SELECT UNNEST(a) construct there is an issue again related to the unresting of composite type arrays: [ example ] Is it a bug or a feature? It looks like the problem in this example is that PL/pgsql tries to assign the result of unest(ta) to t.i rather than to t as a whole. This is pretty ridiculously stupid in this case, but it would make sense if the subselect where of the form SELECT a, b, c FROM ... I haven't looked at the code to see whether there's a way to make this case smarter (it would be nice - I've been bitten by similar problems myself) but it's only tangentially related to the patch under discussion. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] final patch - plpgsql: for-in-array
On Mon, Nov 22, 2010 at 8:39 AM, Jaime Casanova ja...@2ndquadrant.com wrote: On Wed, Nov 17, 2010 at 7:08 PM, Jaime Casanova ja...@2ndquadrant.com wrote: On Thu, Sep 30, 2010 at 7:46 AM, Pavel Stehule pavel.steh...@gmail.com wrote: Hello this patch implement a new iteration construct - iteration over an array. The sense of this new iteration is: * a simple and cleaner syntax i will start the review of this one... so, what is the concensus for this patch? return with feedback? reject the patch on the grounds that we should go fix unnest() if it slow? something else? I think it should be marked rejected. I don't think Tom is likely to ever be in favor of a syntax change here, and while I hesitate to deal in absolutes, I don't think I will be either, and certainly not without a lot more work on improving the performance of the existing constructs. In particular, this seems like something that really ought to be pursued: http://archives.postgresql.org/pgsql-hackers/2010-11/msg01177.php -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] final patch - plpgsql: for-in-array
Hello I spent last two days a searching how to solve this problem better. Probably I removed a issue with toasting. But I found other issue, that wasn't discussed before. This issue is only seq access to items via array_seek function. I though about some variable that stores a last accessed field and last used subscript - but there isn't a good place where this info can be stored (maybe in ArrayType). Other issues are a arrays with a null bitmap. It is difficult to say if this cache can have a positive effect - maybe yes - for large arrays. Problem is in possible a increase of price for random access to array. And there are not any hint, that helps with specification about access to array. I don't think so searching inside expr. plan is a good idea. Is way to have more code, more complexity. If we will do it, then more important are accelerations of expression var = var + 1; var = var || var; or some else. So, please, I know, so you and Tom are busy, but try to spend a few time about this problem before you are definitely reject this idea. With my last patch (removing a toasting issue) the classic access to array should be usable. But still any direct access to array from PL executor is 20% faster - depends on array type. Maybe it is useless discus - and all can be solved with possible support SRF inside simple expression, but I don't know when it will be possible. Regards Pavel Stehule * * CAUTION: if you change the header for ordinary arrays you will also * need to change the headers for oidvector and int2vector! */ I think it should be marked rejected. I don't think Tom is likely to ever be in favor of a syntax change here, and while I hesitate to deal in absolutes, I don't think I will be either, and certainly not without a lot more work on improving the performance of the existing constructs. In particular, this seems like something that really ought to be pursued: http://archives.postgresql.org/pgsql-hackers/2010-11/msg01177.php -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] final patch - plpgsql: for-in-array
On Mon, Nov 22, 2010 at 3:36 PM, Pavel Stehule pavel.steh...@gmail.com wrote: So, please, I know, so you and Tom are busy, but try to spend a few time about this problem before you are definitely reject this idea. If I were to spend more time on this problem, what exactly would I spend that time doing and how would it help? If I were interested in spending time I'd probably spend it pursuing the suggestions Tom already made, and that's what I think you should do. But I'm not going to do that, because the purpose of the CommitFest is not for me to write new patches from scratch that do something vaguely similar to what a patch you wrote was trying to do. It's for all of us to review and commit the patches that have already been written. You aren't helping with that process, so your complaint that we aren't spending enough time on your patches would be unfair even if were true, and it isn't. The problem with your patch is that it has a design weakness, not that it got short shift. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] final patch - plpgsql: for-in-array
2010/11/22 Robert Haas robertmh...@gmail.com: On Mon, Nov 22, 2010 at 3:36 PM, Pavel Stehule pavel.steh...@gmail.com wrote: So, please, I know, so you and Tom are busy, but try to spend a few time about this problem before you are definitely reject this idea. If I were to spend more time on this problem, what exactly would I spend that time doing and how would it help? If I were interested in spending time I'd probably spend it pursuing the suggestions Tom already made, and that's what I think you should do. But I'm not going to do that, because the purpose of the CommitFest is not for me to write new patches from scratch that do something vaguely similar to what a patch you wrote was trying to do. It's for all of us to review and commit the patches that have already been written. You aren't helping with that process, so your complaint that we aren't spending enough time on your patches would be unfair even if were true, and it isn't. The problem with your patch is that it has a design weakness, not that it got short shift. ok, I can only recapitulate so this feature was proposed cca two months ago, and minimally Tom and maybe you did agreement - with request on syntax - do you remember? I am little bit tired so this agreement was changed when I spent my time with this. Pavel -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] final patch - plpgsql: for-in-array
On Tue, Nov 23, 2010 at 05:55:28AM +0100, Pavel Stehule wrote: ok, I can only recapitulate so this feature was proposed cca two months ago, and minimally Tom and maybe you did agreement - with request on syntax - do you remember? I am little bit tired so this agreement was changed when I spent my time with this. What you can actually do that's productive is stop all current development and concentrate on reviewing patches. If the language gap is an issue, please feel free to mail me your reviews in Czech, and I will get them translated. 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] final patch - plpgsql: for-in-array
2010/11/23 David Fetter da...@fetter.org: On Tue, Nov 23, 2010 at 05:55:28AM +0100, Pavel Stehule wrote: ok, I can only recapitulate so this feature was proposed cca two months ago, and minimally Tom and maybe you did agreement - with request on syntax - do you remember? I am little bit tired so this agreement was changed when I spent my time with this. What you can actually do that's productive is stop all current development and concentrate on reviewing patches. If the language gap is an issue, please feel free to mail me your reviews in Czech, and I will get them translated. it was a last mail to this topic minimally for some months. Regards Pavel Stehule 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] final patch - plpgsql: for-in-array
I checked my tests and the most important is a remove a repeated detoast.
postgres=# CREATE OR REPLACE FUNCTION public.filter01(text[], text, integer)
RETURNS text[]
LANGUAGE plpgsql
AS $function$
DECLARE
s text[] := '{}';
l int := 0; i int;
v text; loc text[] = $1;
BEGIN
FOR i IN array_lower(loc,1)..array_upper(loc,1)
LOOP
EXIT WHEN l = $3;
IF loc[i] LIKE $2 THEN
s := s || loc[i];
l := l + 1;
END IF;
END LOOP;
RETURN s;
END;$function$;
This code is very slow when array is large - tested on n=1000. With
one small modification can be 20x faster
DECLARE
s text[] := '{}';
l int := 0; i int;
v text; loc text[] = $1 || '{}'::text[]; -- does just detoast and
docomprimation
BEGIN
the final version of test can be:
so result:
Don't access to large unmodified array inside cycle, when data comes
from table (for iteration over A[1000] of text(10)). A speadup is from
451 sec to 15 sec. This rule can be interesting for PostGIS people,
because it can be valid for other long varlena values. But still this
is 2x slower than special statement.
Regards
Pavel Stehule
samples %symbol name
332 22.1333 exec_eval_expr
311 20.7333 plpgsql_param_fetch
267 17.8000 exec_eval_datum
220 14.6667 exec_stmts
916.0667 setup_param_list
825.4667 exec_eval_cleanup.clone.10
714.7333 __i686.get_pc_thunk.bx
483.2000 exec_simple_cast_value
432.8667 exec_eval_boolean
samples %symbol name
4636 37.5994 array_seek.clone.0
961 7.7940 pglz_decompress
901 7.3074 list_member_ptr
443 3.5929 MemoryContextAllocZero
384 3.1144 AllocSetAlloc
381 3.0900 ExecEvalParamExtern
334 2.7088 GetSnapshotData
255 2.0681 AllocSetFree
254 2.0600 LWLockRelease
249 2.0195 ExecMakeFunctionResultNoSets
249 2.0195 UTF8_MatchText
234 1.8978 LWLockAcquire
195 1.5815 AllocSetReset
167 1.3544 AllocSetCheck
163 1.3220 pfree
151 1.2247 ExecEvalArrayRef
149 1.2084 RevalidateCachedPlan
138 1.1192 bms_is_member
126 1.0219 CopySnapshot
--
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] final patch - plpgsql: for-in-array
2010/11/18 Pavel Stehule pavel.steh...@gmail.com: 2010/11/18 Tom Lane t...@sss.pgh.pa.us: Merlin Moncure mmonc...@gmail.com writes: On Wed, Nov 17, 2010 at 7:08 PM, Jaime Casanova ja...@2ndquadrant.com wrote: i will start the review of this one... but before that sorry for suggesting this a bit later but about using UNNEST as part of the sintax? Does for-in-array do what unnset does? Yes, which begs the question of why bother at all. AFAICS this patch simply allows you to replace for x in select unnest(array_value) loop with for x in unnest array_value loop (plus or minus a parenthesis or so). I do not think we need to add a bunch of code and create even more syntactic ambiguity (FOR loops are already on the hairy edge of unparsability) to save people from writing select. this patch is semantically equal to SELECT unnest(..), but it is evaluated as simple expression and does directly array unpacking and iteration, - so it means this fragment is significantly faster. Did you implement a method to be able to walk the array and detoast only the current needed data ? (I wonder because I have something like that in that garage : select array_filter(foo,'like','%bar%',10); where 10 is the limit and can be avoided, foo is the array, like is callback function, '%bar%' the parameter for the callback function for filtering results.) It will make my toy in the garage a fast race car (and probably doable in (plpg)SQL instead of C) ... -- Cédric Villemain 2ndQuadrant http://2ndQuadrant.fr/ PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] final patch - plpgsql: for-in-array
2010/11/18 Cédric Villemain cedric.villemain.deb...@gmail.com:
2010/11/18 Pavel Stehule pavel.steh...@gmail.com:
2010/11/18 Tom Lane t...@sss.pgh.pa.us:
Merlin Moncure mmonc...@gmail.com writes:
On Wed, Nov 17, 2010 at 7:08 PM, Jaime Casanova ja...@2ndquadrant.com
wrote:
i will start the review of this one... but before that sorry for
suggesting this a bit later but about using UNNEST as part of the
sintax?
Does for-in-array do what unnset does?
Yes, which begs the question of why bother at all. AFAICS this patch
simply allows you to replace
for x in select unnest(array_value) loop
with
for x in unnest array_value loop
(plus or minus a parenthesis or so). I do not think we need to add a
bunch of code and create even more syntactic ambiguity (FOR loops are
already on the hairy edge of unparsability) to save people from writing
select.
this patch is semantically equal to SELECT unnest(..), but it is
evaluated as simple expression and does directly array unpacking and
iteration, - so it means this fragment is significantly faster.
Did you implement a method to be able to walk the array and detoast
only the current needed data ?
not only - iteration over array can help with readability but a
general work with SRF (set returning functions is more harder and
slower) - so special loop statement can to safe a some toast op / when
you use a large array and access via index, or can to safe a some work
with memory, because there isn't necessary convert array to set of
tuples. Please, recheck these tests.
test:
CREATE OR REPLACE FUNCTION rndstr() RETURNS text AS $$select
array_to_string(array(select substring('ABCDEFGHIJKLMNOPQ' FROM
(random()*16)::int FOR 1) from generate_series(1,10)),'')$$ LANGUAGE
sql;
create or replace function rndarray(int) returns text[] as $$select
array(select rndstr() from generate_series(1,$1)) $$ language sql;
create table t10(x text[]);
insert into t10 select rndarray(10) from generate_series(1,1);
create table t100(x text[]);
insert into t100 select rndarray(100) from generate_series(1,1);
create table t1000(x text[]);
insert into t1000 select rndarray(1000) from generate_series(1,1);
CREATE OR REPLACE FUNCTION public.filter(text[], text, integer)
RETURNS text[]
LANGUAGE plpgsql
AS $function$
DECLARE
s text[] := '{}';
l int := 0;
v text;
BEGIN
FOR v IN ARRAY $1
LOOP
EXIT WHEN l = $3;
IF v LIKE $2 THEN
s := s || v;
l := l + 1;
END IF;
END LOOP;
RETURN s;
END;$function$;
postgres=# select avg(array_upper(filter(x,'%AA%', 10),1)) from t10;
avg
1.1596079803990200
(1 row)
Time: 393.649 ms
postgres=# select avg(array_upper(filter(x,'%AA%', 10),1)) from t100;
avg
3.497689245536
(1 row)
Time: 2804.502 ms
postgres=# select avg(array_upper(filter(x,'%AA%', 10),1)) from t1000;
avg
-
10.
(1 row)
Time: 9729.994 ms
CREATE OR REPLACE FUNCTION public.filter01(text[], text, integer)
RETURNS text[]
LANGUAGE plpgsql
AS $function$
DECLARE
s text[] := '{}';
l int := 0;
v text;
BEGIN
FOR v IN SELECT UNNEST($1)
LOOP
EXIT WHEN l = $3;
IF v LIKE $2 THEN
s := s || v;
l := l + 1;
END IF;
END LOOP;
RETURN s;
END;$function$;
postgres=# select avg(array_upper(filter01(x,'%AA%', 10),1)) from t10;
avg
1.1596079803990200
(1 row)
Time: 795.383 ms
postgres=# select avg(array_upper(filter01(x,'%AA%', 10),1)) from t100;
avg
3.497689245536
(1 row)
Time: 3848.258 ms
postgres=# select avg(array_upper(filter01(x,'%AA%', 10),1)) from t1000;
avg
-
10.
(1 row)
Time: 12366.093 ms
The iteration via specialized FOR IN ARRAY is about 25-30% faster than
FOR IN SELECT UNNEST
postgres=# CREATE OR REPLACE FUNCTION public.filter02(text[], text, integer)
RETURNS text[]
LANGUAGE plpgsql
AS $function$
DECLARE
s text[] := '{}';
l int := 0; i int;
v text;
BEGIN
FOR i IN array_lower($1,1)..array_upper($1,1)
LOOP
EXIT WHEN l = $3;
IF $1[i] LIKE $2 THEN
s := s || $1[i];
l := l + 1;
END IF;
END LOOP;
RETURN s;
END;$function$
;
postgres=# select avg(array_upper(filter02(x,'%AA%', 10),1)) from t10;
avg
1.1596079803990200
(1 row)
Time: 414.960 ms
postgres=# select avg(array_upper(filter02(x,'%AA%', 10),1)) from t100;
avg
3.497689245536
(1 row)
Time: 3460.970 ms
there FOR IN ARRAY is faster about 30% then access per index
for T1000 I had to cancel over 1 minute
(I wonder because I have something like that in that garage : select
array_filter(foo,'like','%bar%',10); where 10 is the limit and can be
avoided, foo is the array, like is callback function, '%bar%' the
parameter for the callback function for filtering results.)
It will make my toy in the garage a fast race
Re: [HACKERS] final patch - plpgsql: for-in-array
On Thu, Nov 18, 2010 at 12:47 AM, Tom Lane t...@sss.pgh.pa.us wrote: Merlin Moncure mmonc...@gmail.com writes: On Wed, Nov 17, 2010 at 7:08 PM, Jaime Casanova ja...@2ndquadrant.com wrote: i will start the review of this one... but before that sorry for suggesting this a bit later but about using UNNEST as part of the sintax? Does for-in-array do what unnset does? Yes, which begs the question of why bother at all. AFAICS this patch simply allows you to replace for x in select unnest(array_value) loop with for x in unnest array_value loop (plus or minus a parenthesis or so). I do not think we need to add a bunch of code and create even more syntactic ambiguity (FOR loops are already on the hairy edge of unparsability) to save people from writing select. Pavel's performance argument is imnsho valid. arrays at present are the best way to pass data around functions and any optimizations here are very welcome. Given that, is there any way to mitigate your concerns on the syntax side? merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] final patch - plpgsql: for-in-array
2010/11/18 Pavel Stehule pavel.steh...@gmail.com:
2010/11/18 Cédric Villemain cedric.villemain.deb...@gmail.com:
2010/11/18 Pavel Stehule pavel.steh...@gmail.com:
2010/11/18 Tom Lane t...@sss.pgh.pa.us:
Merlin Moncure mmonc...@gmail.com writes:
On Wed, Nov 17, 2010 at 7:08 PM, Jaime Casanova ja...@2ndquadrant.com
wrote:
i will start the review of this one... but before that sorry for
suggesting this a bit later but about using UNNEST as part of the
sintax?
Does for-in-array do what unnset does?
Yes, which begs the question of why bother at all. AFAICS this patch
simply allows you to replace
for x in select unnest(array_value) loop
with
for x in unnest array_value loop
(plus or minus a parenthesis or so). I do not think we need to add a
bunch of code and create even more syntactic ambiguity (FOR loops are
already on the hairy edge of unparsability) to save people from writing
select.
this patch is semantically equal to SELECT unnest(..), but it is
evaluated as simple expression and does directly array unpacking and
iteration, - so it means this fragment is significantly faster.
Did you implement a method to be able to walk the array and detoast
only the current needed data ?
not only - iteration over array can help with readability but a
general work with SRF (set returning functions is more harder and
slower) - so special loop statement can to safe a some toast op / when
you use a large array and access via index, or can to safe a some work
with memory, because there isn't necessary convert array to set of
tuples. Please, recheck these tests.
test:
CREATE OR REPLACE FUNCTION rndstr() RETURNS text AS $$select
array_to_string(array(select substring('ABCDEFGHIJKLMNOPQ' FROM
(random()*16)::int FOR 1) from generate_series(1,10)),'')$$ LANGUAGE
sql;
create or replace function rndarray(int) returns text[] as $$select
array(select rndstr() from generate_series(1,$1)) $$ language sql;
create table t10(x text[]);
insert into t10 select rndarray(10) from generate_series(1,1);
create table t100(x text[]);
insert into t100 select rndarray(100) from generate_series(1,1);
create table t1000(x text[]);
insert into t1000 select rndarray(1000) from generate_series(1,1);
CREATE OR REPLACE FUNCTION public.filter(text[], text, integer)
RETURNS text[]
LANGUAGE plpgsql
AS $function$
DECLARE
s text[] := '{}';
l int := 0;
v text;
BEGIN
FOR v IN ARRAY $1
LOOP
EXIT WHEN l = $3;
IF v LIKE $2 THEN
s := s || v;
l := l + 1;
END IF;
END LOOP;
RETURN s;
END;$function$;
postgres=# select avg(array_upper(filter(x,'%AA%', 10),1)) from t10;
avg
1.1596079803990200
(1 row)
Time: 393.649 ms
postgres=# select avg(array_upper(filter(x,'%AA%', 10),1)) from t100;
avg
3.497689245536
(1 row)
Time: 2804.502 ms
postgres=# select avg(array_upper(filter(x,'%AA%', 10),1)) from t1000;
avg
-
10.
(1 row)
Time: 9729.994 ms
CREATE OR REPLACE FUNCTION public.filter01(text[], text, integer)
RETURNS text[]
LANGUAGE plpgsql
AS $function$
DECLARE
s text[] := '{}';
l int := 0;
v text;
BEGIN
FOR v IN SELECT UNNEST($1)
LOOP
EXIT WHEN l = $3;
IF v LIKE $2 THEN
s := s || v;
l := l + 1;
END IF;
END LOOP;
RETURN s;
END;$function$;
postgres=# select avg(array_upper(filter01(x,'%AA%', 10),1)) from t10;
avg
1.1596079803990200
(1 row)
Time: 795.383 ms
postgres=# select avg(array_upper(filter01(x,'%AA%', 10),1)) from t100;
avg
3.497689245536
(1 row)
Time: 3848.258 ms
postgres=# select avg(array_upper(filter01(x,'%AA%', 10),1)) from t1000;
avg
-
10.
(1 row)
Time: 12366.093 ms
The iteration via specialized FOR IN ARRAY is about 25-30% faster than
FOR IN SELECT UNNEST
postgres=# CREATE OR REPLACE FUNCTION public.filter02(text[], text, integer)
RETURNS text[]
LANGUAGE plpgsql
AS $function$
DECLARE
s text[] := '{}';
l int := 0; i int;
v text;
BEGIN
FOR i IN array_lower($1,1)..array_upper($1,1)
LOOP
EXIT WHEN l = $3;
IF $1[i] LIKE $2 THEN
s := s || $1[i];
l := l + 1;
END IF;
END LOOP;
RETURN s;
END;$function$
;
postgres=# select avg(array_upper(filter02(x,'%AA%', 10),1)) from t10;
avg
1.1596079803990200
(1 row)
Time: 414.960 ms
postgres=# select avg(array_upper(filter02(x,'%AA%', 10),1)) from t100;
avg
3.497689245536
(1 row)
Time: 3460.970 ms
there FOR IN ARRAY is faster about 30% then access per index
for T1000 I had to cancel over 1 minute
I can't test until this week-end. But I will.
(I wonder because I have something like that in that garage : select
array_filter(foo,'like','%bar%',10); where 10 is the
Re: [HACKERS] final patch - plpgsql: for-in-array
On Thu, Nov 18, 2010 at 10:24 AM, Merlin Moncure mmonc...@gmail.com wrote: On Thu, Nov 18, 2010 at 12:47 AM, Tom Lane t...@sss.pgh.pa.us wrote: Merlin Moncure mmonc...@gmail.com writes: On Wed, Nov 17, 2010 at 7:08 PM, Jaime Casanova ja...@2ndquadrant.com wrote: i will start the review of this one... but before that sorry for suggesting this a bit later but about using UNNEST as part of the sintax? Does for-in-array do what unnset does? Yes, which begs the question of why bother at all. AFAICS this patch simply allows you to replace for x in select unnest(array_value) loop with for x in unnest array_value loop (plus or minus a parenthesis or so). I do not think we need to add a bunch of code and create even more syntactic ambiguity (FOR loops are already on the hairy edge of unparsability) to save people from writing select. Pavel's performance argument is imnsho valid. arrays at present are the best way to pass data around functions and any optimizations here are very welcome. Given that, is there any way to mitigate your concerns on the syntax side? Can we get the performance benefit any other way? I hate to think that it will still be slow for people using the already-supported syntax. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] final patch - plpgsql: for-in-array
2010/11/18 Robert Haas robertmh...@gmail.com: On Thu, Nov 18, 2010 at 10:24 AM, Merlin Moncure mmonc...@gmail.com wrote: On Thu, Nov 18, 2010 at 12:47 AM, Tom Lane t...@sss.pgh.pa.us wrote: Merlin Moncure mmonc...@gmail.com writes: On Wed, Nov 17, 2010 at 7:08 PM, Jaime Casanova ja...@2ndquadrant.com wrote: i will start the review of this one... but before that sorry for suggesting this a bit later but about using UNNEST as part of the sintax? Does for-in-array do what unnset does? Yes, which begs the question of why bother at all. AFAICS this patch simply allows you to replace for x in select unnest(array_value) loop with for x in unnest array_value loop (plus or minus a parenthesis or so). I do not think we need to add a bunch of code and create even more syntactic ambiguity (FOR loops are already on the hairy edge of unparsability) to save people from writing select. Pavel's performance argument is imnsho valid. arrays at present are the best way to pass data around functions and any optimizations here are very welcome. Given that, is there any way to mitigate your concerns on the syntax side? Can we get the performance benefit any other way? I hate to think that it will still be slow for people using the already-supported syntax. If you are able to make unnest() outputting 1st row without detoasting last field. I think if we have : #define DatumGetTextPSlice(X,m,n) ((text *) PG_DETOAST_DATUM_SLICE(X,m,n)) but for array, most is done Pavel, am I correct ? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Cédric Villemain 2ndQuadrant http://2ndQuadrant.fr/ PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] final patch - plpgsql: for-in-array
2010/11/18 Robert Haas robertmh...@gmail.com: On Thu, Nov 18, 2010 at 10:24 AM, Merlin Moncure mmonc...@gmail.com wrote: On Thu, Nov 18, 2010 at 12:47 AM, Tom Lane t...@sss.pgh.pa.us wrote: Merlin Moncure mmonc...@gmail.com writes: On Wed, Nov 17, 2010 at 7:08 PM, Jaime Casanova ja...@2ndquadrant.com wrote: i will start the review of this one... but before that sorry for suggesting this a bit later but about using UNNEST as part of the sintax? Does for-in-array do what unnset does? Yes, which begs the question of why bother at all. AFAICS this patch simply allows you to replace for x in select unnest(array_value) loop with for x in unnest array_value loop (plus or minus a parenthesis or so). I do not think we need to add a bunch of code and create even more syntactic ambiguity (FOR loops are already on the hairy edge of unparsability) to save people from writing select. Pavel's performance argument is imnsho valid. arrays at present are the best way to pass data around functions and any optimizations here are very welcome. Given that, is there any way to mitigate your concerns on the syntax side? Can we get the performance benefit any other way? I hate to think that it will still be slow for people using the already-supported syntax. I afraid so other ways are more difficult with deeper impacts on plpgsql executor. what is a slow: a) repeated detoasting - access with subscripts - maybe detoasted values can be cached? b) evaluation of SRF expression - maybe call of SRF function can be simple expression, c) faster evaluation ro query The most important is @a. Only a few people uses a FOR IN SELECT UNNEST form now. Probably not optimizable on PLpgSQL level is form FOR IN SELECT * FROM UNNEST. FOR IN ARRAY doesn't impacts on expression executing - this patch is just simple and isolated. Pavel -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] final patch - plpgsql: for-in-array
2010/11/18 Cédric Villemain cedric.villemain.deb...@gmail.com: 2010/11/18 Robert Haas robertmh...@gmail.com: On Thu, Nov 18, 2010 at 10:24 AM, Merlin Moncure mmonc...@gmail.com wrote: On Thu, Nov 18, 2010 at 12:47 AM, Tom Lane t...@sss.pgh.pa.us wrote: Merlin Moncure mmonc...@gmail.com writes: On Wed, Nov 17, 2010 at 7:08 PM, Jaime Casanova ja...@2ndquadrant.com wrote: i will start the review of this one... but before that sorry for suggesting this a bit later but about using UNNEST as part of the sintax? Does for-in-array do what unnset does? Yes, which begs the question of why bother at all. AFAICS this patch simply allows you to replace for x in select unnest(array_value) loop with for x in unnest array_value loop (plus or minus a parenthesis or so). I do not think we need to add a bunch of code and create even more syntactic ambiguity (FOR loops are already on the hairy edge of unparsability) to save people from writing select. Pavel's performance argument is imnsho valid. arrays at present are the best way to pass data around functions and any optimizations here are very welcome. Given that, is there any way to mitigate your concerns on the syntax side? Can we get the performance benefit any other way? I hate to think that it will still be slow for people using the already-supported syntax. If you are able to make unnest() outputting 1st row without detoasting last field. I think if we have : #define DatumGetTextPSlice(X,m,n) ((text *) PG_DETOAST_DATUM_SLICE(X,m,n)) but for array, most is done Pavel, am I correct ? yes, it can help, but still if you iterate over complete array, you have to do n - detoast ops. Pavel -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Cédric Villemain 2ndQuadrant http://2ndQuadrant.fr/ PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] final patch - plpgsql: for-in-array
On 11/18/2010 10:33 AM, Robert Haas wrote: On Thu, Nov 18, 2010 at 10:24 AM, Merlin Moncuremmonc...@gmail.com wrote: Pavel's performance argument is imnsho valid. arrays at present are the best way to pass data around functions and any optimizations here are very welcome. Given that, is there any way to mitigate your concerns on the syntax side? Can we get the performance benefit any other way? I hate to think that it will still be slow for people using the already-supported syntax. It's not disastrously slower. AFAICT from a very quick glance over the patch, he's getting the speedup by bypassing the normal mechanism for evaluating for x in select So we'd have to special-case that to trap calls to unnest in the general form. That would be pretty ugly. Or else make unnest and SPI faster. But that's a much bigger project. Syntactic sugar is not entirely to be despised, anyway. 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] final patch - plpgsql: for-in-array
2010/11/18 Pavel Stehule pavel.steh...@gmail.com: 2010/11/18 Cédric Villemain cedric.villemain.deb...@gmail.com: 2010/11/18 Robert Haas robertmh...@gmail.com: On Thu, Nov 18, 2010 at 10:24 AM, Merlin Moncure mmonc...@gmail.com wrote: On Thu, Nov 18, 2010 at 12:47 AM, Tom Lane t...@sss.pgh.pa.us wrote: Merlin Moncure mmonc...@gmail.com writes: On Wed, Nov 17, 2010 at 7:08 PM, Jaime Casanova ja...@2ndquadrant.com wrote: i will start the review of this one... but before that sorry for suggesting this a bit later but about using UNNEST as part of the sintax? Does for-in-array do what unnset does? Yes, which begs the question of why bother at all. AFAICS this patch simply allows you to replace for x in select unnest(array_value) loop with for x in unnest array_value loop (plus or minus a parenthesis or so). I do not think we need to add a bunch of code and create even more syntactic ambiguity (FOR loops are already on the hairy edge of unparsability) to save people from writing select. Pavel's performance argument is imnsho valid. arrays at present are the best way to pass data around functions and any optimizations here are very welcome. Given that, is there any way to mitigate your concerns on the syntax side? Can we get the performance benefit any other way? I hate to think that it will still be slow for people using the already-supported syntax. If you are able to make unnest() outputting 1st row without detoasting last field. I think if we have : #define DatumGetTextPSlice(X,m,n) ((text *) PG_DETOAST_DATUM_SLICE(X,m,n)) but for array, most is done Pavel, am I correct ? yes, it can help, but still if you iterate over complete array, you have to do n - detoast ops. sure. Pavel -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Cédric Villemain 2ndQuadrant http://2ndQuadrant.fr/ PostgreSQL : Expertise, Formation et Support -- Cédric Villemain 2ndQuadrant http://2ndQuadrant.fr/ PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] final patch - plpgsql: for-in-array
Merlin Moncure mmonc...@gmail.com writes: On Thu, Nov 18, 2010 at 12:47 AM, Tom Lane t...@sss.pgh.pa.us wrote: Yes, which begs the question of why bother at all. Pavel's performance argument is imnsho valid. Well, that argument is unsupported by any evidence, so far as I've seen. More to the point, if there is indeed an interesting performance win here, we could get the same win by internally optimizing the existing syntax. That would provide the benefit to existing code not just new code; and it would avoid foreclosing our future options for extending FOR in not-so-redundant ways. 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] final patch - plpgsql: for-in-array
2010/11/18 Tom Lane t...@sss.pgh.pa.us: Merlin Moncure mmonc...@gmail.com writes: On Thu, Nov 18, 2010 at 12:47 AM, Tom Lane t...@sss.pgh.pa.us wrote: Yes, which begs the question of why bother at all. Pavel's performance argument is imnsho valid. Well, that argument is unsupported by any evidence, so far as I've seen. More to the point, if there is indeed an interesting performance win here, we could get the same win by internally optimizing the existing syntax. That would provide the benefit to existing code not just new code; and it would avoid foreclosing our future options for extending FOR in not-so-redundant ways. sorry, but I don't agree. I don't think, so there are some big space for optimizing - and if then it means much more code complexity for current expr executor. Next - FOR IN ARRAY takes fields from array on request - and it is possible, because a unpacking of array is controlled by statement - it's impossible do same when unpacking is inside other functions with same effectivity. Regards Pavel Stehule 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] final patch - plpgsql: for-in-array
Pavel Stehule pavel.steh...@gmail.com writes: 2010/11/18 Tom Lane t...@sss.pgh.pa.us: More to the point, if there is indeed an interesting performance win here, we could get the same win by internally optimizing the existing syntax. sorry, but I don't agree. I don't think, so there are some big space for optimizing - and if then it means much more code complexity for current expr executor. Next - FOR IN ARRAY takes fields from array on request - and it is possible, because a unpacking of array is controlled by statement - it's impossible do same when unpacking is inside other functions with same effectivity. Just because you haven't thought about it doesn't mean it's impossible or impractical. The first implementation I was thinking of would involve looking at the SELECT querytree after parsing to see if it's SELECT UNNEST(something) --- that is, empty jointree and so on, single tlist item that is an invocation of the unnest() function. If it is, you pull out the argument expression of unnest() and go from there, with exactly the same execution behavior as in the specialized-syntax patch. This is perfectly safe if you identify the array_unnest function by OID: since it's a built-in function you know exactly what it's supposed to do. But having said that, it's still not apparent to me that array_unnest itself is markedly slower than what you could hope to do in plpgsql. I think the real issue here is that plpgsql's simple-expression code can't be used with set-returning expressions, which means that we have to go through the vastly more expensive SPI code path. But that restriction is largely based on fear of re-using expression trees, which is something we fixed a few weeks ago. I think that it would now be interesting to look at whether FOR x IN SELECT simple-expression could use the simple-expression code even when the expression returns set. That approach might bring a useful speedup not just for unnest, but for many other use-cases as well. 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] final patch - plpgsql: for-in-array
Andrew Dunstan and...@dunslane.net writes: Syntactic sugar is not entirely to be despised, anyway. If it were harmless syntactic sugar I wouldn't be objecting so loudly. The problem here is that FOR is a syntactic choke point: it's already overloaded with several different sub-syntaxes that are quite difficult to separate. Adding another one makes that worse, with the consequences that we might misinterpret the user's intent, leading either to misleading/unhelpful error messages or unexpected runtime behavior. If you consult the archives you can find numerous past instances of complaints from people who hit such problems with the existing set of FOR sub-syntaxes, so this isn't hypothetical. I'm also quite afraid of having a conflict with other future extensions of FOR, which we might want to introduce either on our own hook or for compatibility with something Oracle might add to PL/SQL in future. This might not be the worst place in the entire system to be introducing inessential syntactic sugar, but it's certainly one of the top ten. I would *much* rather we get the performance benefit by internal optimization, and forego inventing syntax. 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] final patch - plpgsql: for-in-array
2010/11/18 Tom Lane t...@sss.pgh.pa.us: Pavel Stehule pavel.steh...@gmail.com writes: 2010/11/18 Tom Lane t...@sss.pgh.pa.us: More to the point, if there is indeed an interesting performance win here, we could get the same win by internally optimizing the existing syntax. sorry, but I don't agree. I don't think, so there are some big space for optimizing - and if then it means much more code complexity for current expr executor. Next - FOR IN ARRAY takes fields from array on request - and it is possible, because a unpacking of array is controlled by statement - it's impossible do same when unpacking is inside other functions with same effectivity. Just because you haven't thought about it doesn't mean it's impossible or impractical. The first implementation I was thinking of would involve looking at the SELECT querytree after parsing to see if it's SELECT UNNEST(something) --- that is, empty jointree and so on, single tlist item that is an invocation of the unnest() function. If it is, you pull out the argument expression of unnest() and go from there, with exactly the same execution behavior as in the specialized-syntax patch. This is perfectly safe if you identify the array_unnest function by OID: since it's a built-in function you know exactly what it's supposed to do. this additional control will do slow down for any expression - more - somebody can use a form: SELECT FROM unnest(), and this form will be slower. But having said that, it's still not apparent to me that array_unnest itself is markedly slower than what you could hope to do in plpgsql. I think the real issue here is that plpgsql's simple-expression code can't be used with set-returning expressions, which means that we have to go through the vastly more expensive SPI code path. But that restriction is largely based on fear of re-using expression trees, which is something we fixed a few weeks ago. I think that it would now be interesting to look at whether FOR x IN SELECT simple-expression could use the simple-expression code even when the expression returns set. That approach might bring a useful speedup not just for unnest, but for many other use-cases as well. any SRF call must not be faster then direct access to array. There is overhead with tuples. I don't understand you. Sorry. I don't think, so your objections are objective. Regards Pavel 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] final patch - plpgsql: for-in-array
On Thu, Nov 18, 2010 at 12:36 PM, Tom Lane t...@sss.pgh.pa.us wrote: I would *much* rather we get the performance benefit by internal optimization, and forego inventing syntax. +1. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] final patch - plpgsql: for-in-array
2010/11/18 Tom Lane t...@sss.pgh.pa.us: Andrew Dunstan and...@dunslane.net writes: Syntactic sugar is not entirely to be despised, anyway. If it were harmless syntactic sugar I wouldn't be objecting so loudly. The problem here is that FOR is a syntactic choke point: it's already overloaded with several different sub-syntaxes that are quite difficult to separate. Adding another one makes that worse, with the consequences that we might misinterpret the user's intent, leading either to misleading/unhelpful error messages or unexpected runtime behavior. If you consult the archives you can find numerous past instances of complaints from people who hit such problems with the existing set of FOR sub-syntaxes, so this isn't hypothetical. yes, this argument is correct - but we can rearange a parser rules related to FOR statement. It can be solved. I'm also quite afraid of having a conflict with other future extensions of FOR, which we might want to introduce either on our own hook or for compatibility with something Oracle might add to PL/SQL in future. we talked about it last time - and I respect it - syntax is FOR IN ARRAY expression This might not be the worst place in the entire system to be introducing inessential syntactic sugar, but it's certainly one of the top ten. I would *much* rather we get the performance benefit by internal optimization, and forego inventing syntax. Regards Pavel 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] final patch - plpgsql: for-in-array
2010/11/18 Robert Haas robertmh...@gmail.com: On Thu, Nov 18, 2010 at 12:36 PM, Tom Lane t...@sss.pgh.pa.us wrote: I would *much* rather we get the performance benefit by internal optimization, and forego inventing syntax. +1. any optimization will be about 10-20% slower than direct access. See my tests: on large arrays isn't significant if you use a simple expression or full query. This is just overhead from building a tuplestore and access to data via cursor. And you cannot to change a SRF functions to returns just array. I would to see any optimization on this level, but I think so it's unreal expecting. Regards Pavel Stehule -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] final patch - plpgsql: for-in-array
On Thu, Nov 18, 2010 at 1:03 PM, Pavel Stehule pavel.steh...@gmail.com wrote: 2010/11/18 Robert Haas robertmh...@gmail.com: On Thu, Nov 18, 2010 at 12:36 PM, Tom Lane t...@sss.pgh.pa.us wrote: I would *much* rather we get the performance benefit by internal optimization, and forego inventing syntax. +1. any optimization will be about 10-20% slower than direct access. See my tests: on large arrays isn't significant if you use a simple expression or full query. This is just overhead from building a tuplestore and access to data via cursor. And you cannot to change a SRF functions to returns just array. I would to see any optimization on this level, but I think so it's unreal expecting. How can you possibly make a general statement like that? What's slow is not the syntax; it's what the syntax is making happen under the hood. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] final patch - plpgsql: for-in-array
2010/11/18 Robert Haas robertmh...@gmail.com: On Thu, Nov 18, 2010 at 1:03 PM, Pavel Stehule pavel.steh...@gmail.com wrote: 2010/11/18 Robert Haas robertmh...@gmail.com: On Thu, Nov 18, 2010 at 12:36 PM, Tom Lane t...@sss.pgh.pa.us wrote: I would *much* rather we get the performance benefit by internal optimization, and forego inventing syntax. +1. any optimization will be about 10-20% slower than direct access. See my tests: on large arrays isn't significant if you use a simple expression or full query. This is just overhead from building a tuplestore and access to data via cursor. And you cannot to change a SRF functions to returns just array. I would to see any optimization on this level, but I think so it's unreal expecting. How can you possibly make a general statement like that? What's slow is not the syntax; it's what the syntax is making happen under the hood. ok, it is based on my tests, but it can be subjective. Probably is possible to work with a tuplestore as result of SRF function. And probably we can read from it without cursor. Maybe we can to teach a SRF functions to store values as scalars not as tuple - tuplestore can do it, but the we have to have a global state and we must to modify buildin functions (not just unnest - if the feature should be general). But newer we can to ensure a working with only necessary data like a special PL statement. unnest returns all fields, but these fields should not be used. There isn't possible to say - stop, I don't need other fields. It's possible just with special PL statement, because it is controlled by PL. So it is reason why I don't believe in optimizations on PL level. Regards Pavel Stehule -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] final patch - plpgsql: for-in-array
Pavel Stehule pavel.steh...@gmail.com writes: what is a slow: a) repeated detoasting - access with subscripts - maybe detoasted values can be cached? b) evaluation of SRF expression - maybe call of SRF function can be simple expression, c) faster evaluation ro query The most important is @a. Really? Becase AFAICS array_unnest only detoasts the source array once, and saves the value between calls. array_unnest doesn't currently have any smarts about fetching slices of an array. I'm not sure how useful that would be in practice, since (1) in most usages you probably run the function to the end and fetch all the values anyway; (2) it's hard to see how to optimize that way if the elements are varlena, which they most likely are in most usages where this could possibly be a win. But if Cedric's use-case is really worth optimizing, I'd sure rather see the smarts for it in the general purpose array_unnest function instead of buried in plpgsql's FOR logic. 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] final patch - plpgsql: for-in-array
Pavel Stehule pavel.steh...@gmail.com writes: 2010/11/18 Tom Lane t...@sss.pgh.pa.us: The problem here is that FOR is a syntactic choke point: it's already overloaded with several different sub-syntaxes that are quite difficult to separate. Â Adding another one makes that worse, with the consequences that we might misinterpret the user's intent, leading either to misleading/unhelpful error messages or unexpected runtime behavior. yes, this argument is correct - but we can rearange a parser rules related to FOR statement. It can be solved. No, it can't. The more things that can possibly follow FOR, the less likely that you correctly guess which one the user had in mind when faced with something that's not quite syntactically correct. Or maybe it *is* syntactically correct, only not according to the variant that the user thought he was invoking. We've seen bug reports of this sort connected with FOR already; in fact I'm pretty sure you've responded to a few yourself. Adding more variants *will* make it worse. We need a decent return on investment for anything we add here, and this proposal just doesn't offer enough benefit. 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] final patch - plpgsql: for-in-array
2010/11/18 Tom Lane t...@sss.pgh.pa.us: Pavel Stehule pavel.steh...@gmail.com writes: what is a slow: a) repeated detoasting - access with subscripts - maybe detoasted values can be cached? b) evaluation of SRF expression - maybe call of SRF function can be simple expression, c) faster evaluation ro query The most important is @a. Really? Becase AFAICS array_unnest only detoasts the source array once, and saves the value between calls. I know. this note was a different -only a few people use FOR IN SELECT UNNEST for iteration over array. So from Robert's question (what is important for current code?) perspective the more significant is access to individual fields via subscripts. For example: for i in 1..1 loop s := s + A[i]; end loop is slow, when high limit of array is some bigger number 1000. But almost all stored procedures used this pattern. I know so some people use a pattern FOR IN SELECT UNNEST, but (for example) I didn't meet that developer in Czech Rep. It isn't usual so people can mix SQL and PL well. It has a practical reasons - using a UNNEST for small arrays is slower. array_unnest doesn't currently have any smarts about fetching slices of an array. I'm not sure how useful that would be in practice, since (1) in most usages you probably run the function to the end and fetch all the values anyway; (2) it's hard to see how to optimize that way if the elements are varlena, which they most likely are in most usages where this could possibly be a win. But if Cedric's use-case is really worth optimizing, I'd sure rather see the smarts for it in the general purpose array_unnest function instead of buried in plpgsql's FOR logic. Probably - example with LIKE filter is really specific. But there can be a tasks, where you can early break a iteration where you find a value higher or less then some constant - it's not too artificial - test IS MEMBER OF Regards Pavel 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] final patch - plpgsql: for-in-array
Pavel Stehule pavel.steh...@gmail.com writes: this note was a different -only a few people use FOR IN SELECT UNNEST for iteration over array. So from Robert's question (what is important for current code?) perspective the more significant is access to individual fields via subscripts. For example: for i in 1..1 loop s := s + A[i]; end loop is slow, when high limit of array is some bigger number 1000. True, but inventing new FOR syntax isn't going to help people who are used to doing 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] final patch - plpgsql: for-in-array
Pavel Stehule pavel.steh...@gmail.com writes: unnest returns all fields, but these fields should not be used. There isn't possible to say - stop, I don't need other fields. It's possible just with special PL statement, because it is controlled by PL. So it is reason why I don't believe in optimizations on PL level. That is complete nonsense. array_unnest doesn't return the whole array contents at once, so it's just as capable of being optimized as any single-purpose implementation. If you exit the loop early, you just don't call it anymore. 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] final patch - plpgsql: for-in-array
On 11/18/2010 02:17 PM, Pavel Stehule wrote: -only a few people use FOR IN SELECT UNNEST for iteration over array. How on earth do you know that? I use it a lot and I was just demonstrating it to a client yesterday, and I'm quite sure he will use it a lot too. I bet I'm far from alone. 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] final patch - plpgsql: for-in-array
2010/11/18 Tom Lane t...@sss.pgh.pa.us: Pavel Stehule pavel.steh...@gmail.com writes: 2010/11/18 Tom Lane t...@sss.pgh.pa.us: The problem here is that FOR is a syntactic choke point: it's already overloaded with several different sub-syntaxes that are quite difficult to separate. Adding another one makes that worse, with the consequences that we might misinterpret the user's intent, leading either to misleading/unhelpful error messages or unexpected runtime behavior. yes, this argument is correct - but we can rearange a parser rules related to FOR statement. It can be solved. No, it can't. The more things that can possibly follow FOR, the less likely that you correctly guess which one the user had in mind when faced with something that's not quite syntactically correct. Or maybe it *is* syntactically correct, only not according to the variant that the user thought he was invoking. We've seen bug reports of this sort connected with FOR already; in fact I'm pretty sure you've responded to a few yourself. Adding more variants *will* make it worse. We need a decent return on investment for anything we add here, and this proposal just doesn't offer enough benefit. yes I reported a allowing a labels on wrong position. But minimally this patch must not to change a current behave. It's your idea to use keyword ARRAY there. Maybe we have just only different view on complexity. My proposal increase complexity in parser, your proposal in executor. Anybody thinking so other variant is worst. I don't speak so we have to have a just FOR IN ARRAY syntax - I though so there was a agreement on last discus. We can use a different syntax - but should be readable. Regards Pavel Stehule 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] final patch - plpgsql: for-in-array
2010/11/18 Tom Lane t...@sss.pgh.pa.us: Pavel Stehule pavel.steh...@gmail.com writes: this note was a different -only a few people use FOR IN SELECT UNNEST for iteration over array. So from Robert's question (what is important for current code?) perspective the more significant is access to individual fields via subscripts. For example: for i in 1..1 loop s := s + A[i]; end loop is slow, when high limit of array is some bigger number 1000. True, but inventing new FOR syntax isn't going to help people who are used to doing that. sure - I don't try it. Any change of this mean significant plpgsql's refactoring and significant increasing the size and complexity of code. More there can be still some overhead, because subscript can be expression. And in almost all cases people dislike to write subscripts. 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] final patch - plpgsql: for-in-array
2010/11/18 Andrew Dunstan and...@dunslane.net: On 11/18/2010 02:17 PM, Pavel Stehule wrote: -only a few people use FOR IN SELECT UNNEST for iteration over array. How on earth do you know that? I use it a lot and I was just demonstrating it to a client yesterday, and I'm quite sure he will use it a lot too. I bet I'm far from alone. how much people are active readers and writers in pg_hackers like you? :) I didn't say so nobody use it. You, me, David. But I really didn't see this pattern here in real applications. Regards Pavel 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] final patch - plpgsql: for-in-array
On 11/18/2010 02:39 PM, Pavel Stehule wrote: 2010/11/18 Andrew Dunstanand...@dunslane.net: On 11/18/2010 02:17 PM, Pavel Stehule wrote: -only a few people use FOR IN SELECT UNNEST for iteration over array. How on earth do you know that? I use it a lot and I was just demonstrating it to a client yesterday, and I'm quite sure he will use it a lot too. I bet I'm far from alone. how much people are active readers and writers in pg_hackers like you? :) I didn't say so nobody use it. You, me, David. But I really didn't see this pattern here in real applications. Lots of people are told to use it on IRC. Trust me, it's getting well known. 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] final patch - plpgsql: for-in-array
2010/11/18 Tom Lane t...@sss.pgh.pa.us: Pavel Stehule pavel.steh...@gmail.com writes: unnest returns all fields, but these fields should not be used. There isn't possible to say - stop, I don't need other fields. It's possible just with special PL statement, because it is controlled by PL. So it is reason why I don't believe in optimizations on PL level. That is complete nonsense. array_unnest doesn't return the whole array contents at once, so it's just as capable of being optimized as any single-purpose implementation. If you exit the loop early, you just don't call it anymore. no it isn't - actually you cannot to limit a returned set when you call SRF function in expression context - if I remember well. We can change it - but this is other complexity. Pavel 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] final patch - plpgsql: for-in-array
2010/11/18 Andrew Dunstan and...@dunslane.net: On 11/18/2010 02:39 PM, Pavel Stehule wrote: 2010/11/18 Andrew Dunstanand...@dunslane.net: On 11/18/2010 02:17 PM, Pavel Stehule wrote: -only a few people use FOR IN SELECT UNNEST for iteration over array. How on earth do you know that? I use it a lot and I was just demonstrating it to a client yesterday, and I'm quite sure he will use it a lot too. I bet I'm far from alone. how much people are active readers and writers in pg_hackers like you? :) I didn't say so nobody use it. You, me, David. But I really didn't see this pattern here in real applications. Lots of people are told to use it on IRC. Trust me, it's getting well known. can be. but people on IRC are not representative. I have about 10 courses of PL/pgSQL per year (about 100 people) - almost all my students newer visited IRC. 30% of my students has a problem to write a bublesort or some little bit complex code. I meet this people. There can be a language barrier or some laziness. Really it is surprisingly how too less people are interesting about coding. This people has own problems, and usually uses a most classic patter that know from programming languages. These peoples are normal and typical. Some courses I have under some big Czech agency - so there are people from banks, industry. Actually only I do a courses of PLpgSQL in Czech language - so I think I know what people use. For example - only a few people know and use a generate_series functions. sorry for offtopic :) Pavel 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] final patch - plpgsql: for-in-array
Excerpts from Pavel Stehule's message of jue nov 18 17:00:04 -0300 2010: 2010/11/18 Andrew Dunstan and...@dunslane.net: I didn't say so nobody use it. You, me, David. But I really didn't see this pattern here in real applications. Lots of people are told to use it on IRC. Trust me, it's getting well known. can be. but people on IRC are not representative. Yeah, that's true. I point out usage of unnest to our customers too, but it's much more common to see people not using it, instead relying on subscripts. People using Postgres show up unexpectedly from under rocks, in the weirdest corners; they rarely consult documentation and even more rarely get into IRC or mailing list to get help. I fail to see how this supports the FOR-IN-array development though. It will just be another unused construct for most people, no? -- Á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] final patch - plpgsql: for-in-array
2010/11/18 Alvaro Herrera alvhe...@commandprompt.com: Excerpts from Pavel Stehule's message of jue nov 18 17:00:04 -0300 2010: 2010/11/18 Andrew Dunstan and...@dunslane.net: I didn't say so nobody use it. You, me, David. But I really didn't see this pattern here in real applications. Lots of people are told to use it on IRC. Trust me, it's getting well known. can be. but people on IRC are not representative. Yeah, that's true. I point out usage of unnest to our customers too, but it's much more common to see people not using it, instead relying on subscripts. People using Postgres show up unexpectedly from under rocks, in the weirdest corners; they rarely consult documentation and even more rarely get into IRC or mailing list to get help. I fail to see how this supports the FOR-IN-array development though. It will just be another unused construct for most people, no? maybe I don't understand well, but patch FOR-IN-ARRAY has a documentation sect2 id=plpgsql-array-iterating + titleLooping Through Array/title +. + para + The syntax is: + synopsis + optional lt;lt;replaceablelabel/replaceablegt;gt; /optional + FOR replaceabletarget/replaceable IN replaceablearray expression/replaceable + replaceablestatements/replaceable + END LOOP optional replaceablelabel/replaceable /optional; + /synopsis +. + The replaceabletarget/replaceable is a record variable, row variable, + or comma-separated list of scalar variables. + The replaceabletarget/replaceable is successively assigned each item + of result of the replaceablearray_expression/replaceable and the loop body + executed for each item. Here is an example: +. + programlisting + CREATE TYPE mypt AS (x int, y int); +. + CREATE FUNCTION iterate_over_points() RETURNS void AS $$ + DECLARE + x int; y int; + a mypt[] = ARRAY[(10,11),(20,21),(30,31)]; + BEGIN + FOR x, y IN ARRAY a + LOOP + RAISE NOTICE 'x = %, y = %', x, y; + END LOOP; + END; + $$ LANGUAGE plpgsql; + /programlisting +. + If the loop is terminated by an literalEXIT/ statement, the last + assigned item value is still accessible after the loop. + /para +/sect2 +. Pavel -- Á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] final patch - plpgsql: for-in-array
Pavel Stehule pavel.steh...@gmail.com writes: 2010/11/18 Alvaro Herrera alvhe...@commandprompt.com: I fail to see how this supports the FOR-IN-array development though. Â It will just be another unused construct for most people, no? maybe I don't understand well, but patch FOR-IN-ARRAY has a documentation UNNEST is documented too. Adding still more features doesn't really improve matters for people who haven't memorized the documentation; it only makes it even harder for them to find out what they should be using. (More features != better) To my mind, the complaint about subscripting being slow suggests that we ought to fix subscripting, not introduce a nonstandard feature that will make certain use-cases faster if people rewrite their code to use it. I think it would probably not be terribly hard to arrange forcible detoasting of an array variable's value the first time it gets subscripted, for instance. Of course that only fixes some use-cases; but it would help, and it helps without requiring people to change their code. 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] final patch - plpgsql: for-in-array
On Thursday 18 November 2010 21:11:32 Alvaro Herrera wrote: Excerpts from Pavel Stehule's message of jue nov 18 17:00:04 -0300 2010: 2010/11/18 Andrew Dunstan and...@dunslane.net: I didn't say so nobody use it. You, me, David. But I really didn't see this pattern here in real applications. Lots of people are told to use it on IRC. Trust me, it's getting well known. can be. but people on IRC are not representative. Yeah, that's true. I point out usage of unnest to our customers too, but it's much more common to see people not using it, instead relying on subscripts. People using Postgres show up unexpectedly from under rocks, in the weirdest corners; they rarely consult documentation and even more rarely get into IRC or mailing list to get help. Well, a good reason for that might be that unnest() is pretty new... Most code I read has been initially written quite a bit earlier. Seeing 8.4 in production is only starting to get common. 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] final patch - plpgsql: for-in-array
On 11/18/2010 06:06 PM, Andres Freund wrote: Well, a good reason for that might be that unnest() is pretty new... Most code I read has been initially written quite a bit earlier. Seeing 8.4 in production is only starting to get common. I guess we must have more adventurous customers than you :-) (Incidentally, there is a (slow but still very useful) userland version of unnest that works with 8.3.) 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] final patch - plpgsql: for-in-array
2010/11/18 Tom Lane t...@sss.pgh.pa.us: Pavel Stehule pavel.steh...@gmail.com writes: what is a slow: a) repeated detoasting - access with subscripts - maybe detoasted values can be cached? b) evaluation of SRF expression - maybe call of SRF function can be simple expression, c) faster evaluation ro query The most important is @a. Really? Becase AFAICS array_unnest only detoasts the source array once, and saves the value between calls. array_unnest doesn't currently have any smarts about fetching slices of an array. I'm not sure how useful that would be in practice, since (1) in most usages you probably run the function to the end and fetch all the values anyway; (2) it's hard to see how to optimize that way if the elements are varlena, which they most likely are in most usages where this could possibly be a win. But if Cedric's use-case is really worth optimizing, I'd sure rather see the smarts for it in the general purpose array_unnest function instead of buried in plpgsql's FOR logic. My use case is the following: I have text[] data containing around 50k field. Executing my array_filter (which is probably not as fast as what Pavel did) is the same thing as executing unnest, except that during the array walk, I apply a callback function and increment an internal counter up to the p_limit parameter when callback function success. So that it stops walking the array as soon as p_limit counter is full, or there are no more elements to walk to in the array. 1) At a maximum it is slow like unnest (plus callback overhead), at a minimum it find quickly and the gain is huge. Don't have the exact numbers right here, but (from memory) the durations are between Oms and 45 milliseconds (20M rows, of 50k field text array, not very long text 100 char, 15k calls per second, depending on items to walk in the array, linear). WIth just unnest, a minimum is 45 ms per query. 2) DETOAST_SLICE I don't know the internals here I have a concrete usage of what Pavel did. And I agree that this is fast and easy way to handle the real issues behind :/ 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 -- Cédric Villemain 2ndQuadrant http://2ndQuadrant.fr/ PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] final patch - plpgsql: for-in-array
2010/11/18 Tom Lane t...@sss.pgh.pa.us: Pavel Stehule pavel.steh...@gmail.com writes: 2010/11/18 Alvaro Herrera alvhe...@commandprompt.com: I fail to see how this supports the FOR-IN-array development though. It will just be another unused construct for most people, no? maybe I don't understand well, but patch FOR-IN-ARRAY has a documentation UNNEST is documented too. Adding still more features doesn't really improve matters for people who haven't memorized the documentation; it only makes it even harder for them to find out what they should be using. (More features != better) yes, but less user feature doesn't mean less code. Mainly in little bit specific environment like plpgsql. To my mind, the complaint about subscripting being slow suggests that we ought to fix subscripting, not introduce a nonstandard feature that will make certain use-cases faster if people rewrite their code to use it. I think it would probably not be terribly hard to arrange forcible detoasting of an array variable's value the first time it gets subscripted, for instance. Of course that only fixes some use-cases; but it would help, and it helps without requiring people to change their code. This is just one half of problem and isn't simple. Second half is array_seek - So any access with subscripts means seq reading of array's data. Please, look on this part. I am thinking, so this is more important, than anything what we discussed before. For fast access there is necessary to call a deconstruct_array function. Then you can access to subscripts quickly. Actually we have not a control for access to items in array, when subscript is used in expression (inside PL). So it is very difficult to accelerate speed in area - probably it means a subscript expr should be evaluated individually. A deconstruct_area is relative expensive function, so you have to have a information about a using of array. Without it, and for smaller arrays, the optimization can be bad. There isn't any backend infrastructure for this decision now. I did a profiling first example: FOR IN ARRAY samples %symbol name 336 20.6642 exec_eval_expr 269 16.5437 plpgsql_param_fetch 229 14.0836 exec_stmts 225 13.8376 exec_eval_datum 118 7.2571 exec_assign_value 915.5966 exec_eval_cleanup.clone.10 885.4121 setup_param_list 724.4280 __i686.get_pc_thunk.bx 653.9975 exec_eval_boolean 472.8905 exec_simple_cast_value 432.6445 free_var.clone.2 281.7220 exec_cast_value samples %image name symbol name 1064 16.1188 postgres pglz_decompress 410 6.2112 postgres AllocSetAlloc 353 5.3477 postgres MemoryContextAllocZero 293 4.4387 postgres GetSnapshotData 290 4.3933 postgres AllocSetFree 281 4.2569 postgres ExecEvalParamExtern 223 3.3783 postgres ExecMakeFunctionResultNoSets 220 3.3328 postgres AllocSetReset 212 3.2116 postgres UTF8_MatchText 210 3.1813 postgres LWLockAcquire 195 2.9541 postgres AllocSetCheck 195 2.9541 postgres LWLockRelease 172 2.6057 postgres pfree 163 2.4693 postgres CopySnapshot 162 2.4542 postgres list_member_ptr 144 2.1815 postgres RevalidateCachedPlan 133 2.0148 postgres PushActiveSnapshot 121 1.8331 postgres PopActiveSnapshot 121 1.8331 postgres bms_is_member 118 1.7876 postgres MemoryContextAlloc 108 1.6361 postgres textlike 105 1.5907 postgres AcquireExecutorLocks 791.1968 postgres TransactionIdPrecedes 761.1513 postgres pgstat_end_function_usage 751.1362 postgres pgstat_init_function_usage 721.0907 postgres check_list_invariants sample01 - FOR i IN array_lowe()..array_upper() for t1000 Profiling through timer interrupt samples %symbol name 1039 29.4084 exec_stmts 723 20.4642 exec_eval_expr 587 16.6148 exec_eval_datum 408 11.5483 plpgsql_param_fetch 176 4.9816 exec_eval_cleanup.clone.10 167 4.7269 setup_param_list 159 4.5004 exec_eval_boolean 128 3.6230 __i686.get_pc_thunk.bx 661.8681 exec_simple_cast_value samples %image name symbol name 312604 84.1141 postgres pglz_decompress 4800 1.2916 postgres hash_search_with_hash_value 4799 1.2913 postgres array_seek.clone.0 2935 0.7897 postgres LWLockAcquire 2399 0.6455 postgres
Re: [HACKERS] final patch - plpgsql: for-in-array
On Thu, Sep 30, 2010 at 7:46 AM, Pavel Stehule pavel.steh...@gmail.com wrote: Hello this patch implement a new iteration construct - iteration over an array. The sense of this new iteration is: * a simple and cleaner syntax i will start the review of this one... but before that sorry for suggesting this a bit later but about using UNNEST as part of the sintax? FOR var IN UNNEST array_expr LOOP END LOOP i like this because: 1) is cleaner when array_expr is ARRAY[1,2,3] 2) is not legal now to use the unnest() function without a SELECT in the context of a FOR loop (or am i missing something?) 3) the unnest() function does the same so seems intuitive what a FOR-IN-UNNEST do what i don't know if is this syntax could co-exist with the unnest() function? -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: 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] final patch - plpgsql: for-in-array
On Wed, Nov 17, 2010 at 7:08 PM, Jaime Casanova ja...@2ndquadrant.com wrote: On Thu, Sep 30, 2010 at 7:46 AM, Pavel Stehule pavel.steh...@gmail.com wrote: Hello this patch implement a new iteration construct - iteration over an array. The sense of this new iteration is: * a simple and cleaner syntax i will start the review of this one... but before that sorry for suggesting this a bit later but about using UNNEST as part of the sintax? Does for-in-array do what unnset does? unnest() flattens the whole array into scalars irregardless of dimensions: select unnest(array[array[1,2],array[3,4]]); unnest 1 2 3 4 If yes, then +1 (unless there is some other problem) otherwise -1. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] final patch - plpgsql: for-in-array
Merlin Moncure mmonc...@gmail.com writes: On Wed, Nov 17, 2010 at 7:08 PM, Jaime Casanova ja...@2ndquadrant.com wrote: i will start the review of this one... but before that sorry for suggesting this a bit later but about using UNNEST as part of the sintax? Does for-in-array do what unnset does? Yes, which begs the question of why bother at all. AFAICS this patch simply allows you to replace for x in select unnest(array_value) loop with for x in unnest array_value loop (plus or minus a parenthesis or so). I do not think we need to add a bunch of code and create even more syntactic ambiguity (FOR loops are already on the hairy edge of unparsability) to save people from writing select. 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] final patch - plpgsql: for-in-array
2010/11/18 Tom Lane t...@sss.pgh.pa.us: Merlin Moncure mmonc...@gmail.com writes: On Wed, Nov 17, 2010 at 7:08 PM, Jaime Casanova ja...@2ndquadrant.com wrote: i will start the review of this one... but before that sorry for suggesting this a bit later but about using UNNEST as part of the sintax? Does for-in-array do what unnset does? Yes, which begs the question of why bother at all. AFAICS this patch simply allows you to replace for x in select unnest(array_value) loop with for x in unnest array_value loop (plus or minus a parenthesis or so). I do not think we need to add a bunch of code and create even more syntactic ambiguity (FOR loops are already on the hairy edge of unparsability) to save people from writing select. this patch is semantically equal to SELECT unnest(..), but it is evaluated as simple expression and does directly array unpacking and iteration, - so it means this fragment is significantly faster. Regards Pavel Stehule regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] final patch - plpgsql: for-in-array
Hello this patch implement a new iteration construct - iteration over an array. The sense of this new iteration is: * a simple and cleaner syntax * a faster execution - this bring down a number of detoast operations create or replace function subscripts(anyarray, int) returns int[] as $$ select array(select generate_subscripts($1,$2)); $$ language sql; create or replace function fora_test() returns int as $$ declare x int; s int = 0; a int[] := array[1,2,3,4,5,6,7,8,9,10]; begin for x in array subscripts(a, 1) loop s := s + a[x]; end loop; return s; end; $$ language plpgsql; create or replace function fora_test() returns int as $$ declare x int; s int = 0; begin for x in array array[1,2,3,4,5,6,7,8,9,10] loop s := s + x; end loop; return s; end; $$ language plpgsql; create or replace function fora_test() returns int as $$ declare x int; y int; a fora_point[] := array[(1,2),(3,4),(5,6)]; begin for x, y in array a loop raise notice 'point=%,%', x, y; end loop; return 0; end; $$ language plpgsql; Regards Pavel Stehule for-in-array 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
