Re: [HACKERS] [PATCH] Negative Transition Aggregate Functions (WIP)
On Mon, Dec 16, 2013 at 08:39:33PM +1300, David Rowley wrote: > On Mon, Dec 16, 2013 at 6:00 AM, Ants Aasma wrote: > > > On Dec 15, 2013 6:44 PM, "Tom Lane" wrote: > > > David Rowley writes: > > > > I've attached an updated patch which includes some documentation. > > > > I've also added support for negfunc in CREATE AGGREGATE. Hopefully > > that's > > > > an ok name for the option, but if anyone has any better ideas please > > let > > > > them be known. > > > > > > I'd be a bit inclined to build the terminology around "reverse" instead > > of > > > "negative" --- the latter seems a bit too arithmetic-centric. But that's > > > just MHO. > > > > To contribute to the bike shedding, inverse is often used in similar > > contexts. > > > I guess it's not really bike shedding, most of the work I hope is done, so > I might as well try to get the docs polished up and we'd need a consensus > on what we're going to call them before I can get that done. > > I like both of these better than negative transition function and I agree > negative implies arithmetic rather than opposite. > Out of these 2 I do think inverse fits better than reverse, so I guess that > would make it "inverse aggregate transition function". > Would that make the CREATE AGGREGATE option be INVFUNC ? > > Any other ideas or +1's for any of the existing ones? +1 for inverse. Cheers, David. -- David Fetter 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] [PATCH] Negative Transition Aggregate Functions (WIP)
On Mon, Dec 16, 2013 at 6:00 AM, Ants Aasma wrote: > On Dec 15, 2013 6:44 PM, "Tom Lane" wrote: > > David Rowley writes: > > > I've attached an updated patch which includes some documentation. > > > I've also added support for negfunc in CREATE AGGREGATE. Hopefully > that's > > > an ok name for the option, but if anyone has any better ideas please > let > > > them be known. > > > > I'd be a bit inclined to build the terminology around "reverse" instead > of > > "negative" --- the latter seems a bit too arithmetic-centric. But that's > > just MHO. > > To contribute to the bike shedding, inverse is often used in similar > contexts. > I guess it's not really bike shedding, most of the work I hope is done, so I might as well try to get the docs polished up and we'd need a consensus on what we're going to call them before I can get that done. I like both of these better than negative transition function and I agree negative implies arithmetic rather than opposite. Out of these 2 I do think inverse fits better than reverse, so I guess that would make it "inverse aggregate transition function". Would that make the CREATE AGGREGATE option be INVFUNC ? Any other ideas or +1's for any of the existing ones? Regards David Rowley > -- > Ants Aasma >
Re: [HACKERS] Extension Templates S03E11
On Sat, 2013-12-14 at 13:46 -0800, Josh Berkus wrote: > All: > > Can someone summarize the issues with this patch for those of us who > haven't been following it closely? I was just chatting with a couple > other contributors, and at this point none of just know what it > implements, what it doesn't implement, what the plans are for expanding > its feature set (if any), and why Frost doesn't like it. I tried > reading through the thread on -hackers, and came away even more confused. > > Is there maybe a wiki page for it? The patch offers an alternative to dropping files on the filesystem before doing CREATE EXTENSION. Instead, if the extension has no C code, you can put it in the catalog using ordinary SQL access, and execute the same kind of CREATE EXTENSION. Aside from that, it's pretty much identical to existing extensions. Stephen doesn't like the idea that the SQL in an extension is a blob of text. There are weird cases, like if you make local modifications to objects held in an extension, then dump/reload will lose those local modifications. Another issue, which I agree is dubious in many situations, is that the version of an extension is not preserved across dump/reload (this is actually a feature, which was designed with contrib-style extensions in mind, but can be surprising in other circumstances). This isn't necessarily a dead-end, but there are a lot of unsettled issues, and it will take some soul-searching to answer them. Is an "extension" a blob of text with a version, that's maintained in some external repo? Is it the job of postgres to ensure that dump/reload creates the same situation that you started with, including local modifications to objects that are part of an extension? Should everything be an extension, or do we need to invent a new concept for some of the use cases? What role to external tools play in all of this? Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical changeset generation v6.8
On Wed, Dec 11, 2013 at 7:08 PM, Andres Freund wrote: >> I don't think there's much point in including "remapping" in all of >> the error messages. It adds burden for translators and users won't >> know what a remapping file is anyway. > > It helps in locating wich part of the code caused a problem. I utterly > hate to get reports with error messages that I can't correlate to a > sourcefile. Yes, I know that verbose error output exists, but usually > you don't get it that way... That said, I'll try to make the messages > simpler. Well, we could adopt a policy of not making messages originating from different locations in the code the same. However, it looks to me like that's NOT the current policy, because some care has been taken to reuse messages rather than distinguish them. There's no hard and fast rule here, because some cases are distinguished, but my gut feeling is that all of the errors your patch introduces are sufficiently obscure cases that separate messages with separate translations are not warranted. The time when this is likely to fail is when someone borks the permissions on the data directory, and the user probably won't need to care exactly which file we can't write. >> + if (!TransactionIdIsNormal(xmax)) >> + { >> + /* >> +* no xmax is set, can't have any permanent ones, so >> this check is >> +* sufficient >> +*/ >> + } >> + else if (HEAP_XMAX_IS_LOCKED_ONLY(new_tuple->t_data->t_infomask)) >> + { >> + /* only locked, we don't care */ >> + } >> + else if (!TransactionIdPrecedes(xmax, cutoff)) >> + { >> + /* tuple has been deleted recently, log */ >> + do_log_xmax = true; >> + } >> >> Obfuscated. Rewrite without empty blocks. > > I don't understand why having an empty block is less clear than > including a condition in several branches? Especially if the individual > conditions might need to be documented? It's not a coding style we typically use, to my knowledge. Of course, what is actually clearest is a matter of opinion, but my own experience is that such blocks typically end up seeming clearer to me when the individual comments are joined together into a paragraph that explains the logic in full sentences what's going on. >> + /* >> +* Write out buffer everytime we've too many in-memory entries. >> +*/ >> + if (state->rs_num_rewrite_mappings >= 1000 /* arbitrary number */) >> + logical_heap_rewrite_flush_mappings(state); >> >> What happens if the number of items per hash table entry is small but >> the number of entries is large? > > rs_num_rewrite_mappings is the overall number of in-memory mappings, not > the number of per-entry mappings. That means we flush, even if all > entries have only a couple of mappings, as soon as 1000 in memory > entries have been collected. A bit simplistic, but seems okay enough? Possibly, not sure yet. I need to examine that logic in more detail, but I had trouble following it and was hoping the next version would be better-commented. >> I'm still unhappy that we're introducing logical decoding slots but no >> analogue for physical replication. If we had the latter, would it >> share meaningful amounts of structure with this? > > Yes, I think we could mostly reuse it, we'd probably want to add a field > or two more (application_name, sync_prio?). I have been wondering > whether some of the code in replication/logical/logical.c shouldn't be > in replication/slot.c or similar. So far I've opted for leaving it in > its current place since it would have to change a bit for a more general > role. I strongly favor moving the slot-related code to someplace with "slot" in the name, and replication/slot.c seems about right. Even if we don't extend them to cover non-logical replication in this release, we'll probably do it eventually, and it'd be better if that didn't require moving large amounts of code between files. > I still think that the technical parts of properly supporting persistent > slots for physical rep aren't that hard, but that the behavioural > decisions are. I think there are primarily two things for SR that we > want to prevent using slots: > a) removal of still used WAL (i.e. maintain knowledge about a standby's >last required REDO location) Check. > b) make hot_standby_feedback work across disconnections of the walsender >connection (i.e peg xmin, not just for catalogs though) Check; might need to be optional. > c) Make sure we can transport those across cascading >replication. Not sure I follow. > once those are there it's also useful to keep a bit more information > about the state of replicas: > * write, flush, apply lsn Check. > The hard questions that I see are like: > * How do we manage standby registration? Does the admin have to do that > manually? Or does a standby register itself automatically if
Re: Custom Scan APIs (Re: [HACKERS] Custom Plan node)
Hi Kaigai-san, 2013/12/11 Kohei KaiGai : > 2013/12/10 Shigeru Hanada : >> The patches could be applied cleanly, but I saw a compiler warning >> about get_rel_relkind() in foreign.c, but it's minor issue. Please >> just add #include of utils/lsyscache.h there. >> > Fixed, Check. >> I have some more random comments about EXPLAIN. >> >> 1) You use "Operation" as the label of Custom Scan nodes in non-text >> format, but it seems to me rather "provider name". What is the string >> shown there? >> > I tried free-riding on the existing properties, but it does not make sense > indeed, as you pointed out. > I adjusted the explain.c to show "Custom-Provider" property for Custom- > Scan node, as follows. New name seems better, it is what the node express. >> 2) It would be nice if we can see the information about what the >> Custom Scan node replaced in EXPLAIN output (even only in verbose >> mode). I know that we can't show plan tree below custom scan nodes, >> because CS Provider can't obtain other candidates. But even only >> relation names used in the join or the scan would help users to >> understand what is going on in Custom Scan. >> > Even though I agree that it helps users to understand the plan, > it also has a headache to implement because CustomScan node > (and its super class) does not have an information which relations > are underlying. Probably, this functionality needs to show > the underlying relations on ExplainTargetRel() if CustomScan node > represents a scan instead of join. What data source can produce > the list of underlying relations here? > So, if it is not a significant restriction for users, I'd like to work on this > feature later. Agreed. It would be enough that Custom Scan Providers can add arbitrary information, such as "Remote SQL" of postgres_fdw, to EXPLAIN result via core API. Some kind of framework which helps authors of Custom Scan Providers, but it should be considered after the first cut. > The attached patch fixes up a minor warning around get_rel_relkind > and name of the property for custom-provider. Please check it. The patch can be applied onto 2013-12-16 HEAD cleanly, and gives no unexpected error/warinig. I'm sorry to post separately, but I have some comments on document. (1) ctidscan Is session_preload_libraries available to enable the feature, like shared_*** and local_***? According to my trial it works fine like two similar GUCs. (2) postgres_fdw JOIN push--down is a killer application of Custom Scan Provider feature, so I think it's good to mention it in the "Remote Query Optimization" section. Codes for core and contrib seem fine, so I'll mark the patches "Ready for committer" after the document enhancement. Regards, -- Shigeru HANADA -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [bug fix] pg_ctl always uses the same event source
On Thu, Dec 12, 2013 at 4:43 PM, MauMau wrote: > Hi, Amit san, > > From: "Amit Kapila" >>> >>> [elog.c] >>> Writing the default value in this file was redundant, because >>> event_source >>> cannot be NULL. So change >>> >> I think this change might not be safe as write_eventlog() gets called >> from write_stderr() which might get called >> before reading postgresql.conf, so the code as exists in PG might be >> to handle that case or some other similar >> situation where event_source is still not set. Have you confirmed in >> all ways that it is never the case that >> event_source is not set when the control reaches this function. > > > Yes, you are right. I considered this again after replying to your previous > mail, and found out write_stderr() calls write_eventlog(). In that case, > event_source is still NULL before GUC initialization. Few minor things: 1. evtHandle = RegisterEventSource(NULL, *event_source? event_source: DEFAULT_EVENT_SOURCE); In this code, you are trying to access the value (*event_source) and incase it is not initialised, it will not contain the value and could cause problem, why not directly check 'event_source'? 2. minor coding style issue pg_ctl.c evtHandle = RegisterEventSource(NULL, *event_source? event_source: DEFAULT_EVENT_SOURCE); elog.c ! evtHandle = RegisterEventSource(NULL, ! event_source ? event_source : DEFAULT_EVENT_SOURCE); In both above usages, it is better that arguments in second line should start inline with previous lines first argument. You can refer other places, for ex. refer call to ReportEvent in pg_ctl.c just below RegisterEventSource call. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSL: better default ciphersuite
> "MK" == Marko Kreen writes: > "PE" == Peter Eisentraut writes: MK>> Well, we should - the DEFAULT is clearly a client-side default MK>> for compatibility only. No server should ever run with it. PE> Any other opinions on this out there? For reference, see: https://wiki.mozilla.org/Security/Server_Side_TLS for the currently suggested suite for TLS servers. That is: ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES128-GCM-SHA256: ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES256-GCM-SHA384: DHE-RSA-AES128-GCM-SHA256:DHE-DSS-AES128-GCM-SHA256:kEDH+AESGCM: ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA: ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA384: ECDHE-RSA-AES256-SHA:ECDHE-ECDSA-AES256-SHA:DHE-RSA-AES128-SHA256: DHE-RSA-AES128-SHA:DHE-DSS-AES128-SHA256:DHE-RSA-AES256-SHA256: DHE-DSS-AES256-SHA:DHE-RSA-AES256-SHA:AES128-GCM-SHA256: AES256-GCM-SHA384:ECDHE-RSA-RC4-SHA:ECDHE-ECDSA-RC4-SHA: AES128:AES256:RC4-SHA:HIGH: !aNULL:!eNULL:!EXPORT:!DES:!3DES:!MD5:!PSK The page explains why. But for pgsql, I'd leave off the !PSK; pre-shared keys may prove useful for some. And RC4, perhaps, also should be !ed. And if anyone wants Kerberos tls-authentication, one could add KRB5-DES-CBC3-SHA, but that is ssl3-only. Once salsa20-poly1305 lands in openssl, that should be added to the start of the list. -JimC -- James Cloos OpenPGP: 1024D/ED7DAEA6 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: make_timestamp function
Hello 2013/12/13 Jim Nasby > On 12/13/13 1:49 PM, FabrÃzio de Royes Mello wrote: > >> >> On Fri, Dec 13, 2013 at 5:35 PM, Tom Lane > t...@sss.pgh.pa.us>> wrote: >> >> > >> > =?ISO-8859-1?Q?Fabr=EDzio_de_Royes_Mello?= >> > fabriziome...@gmail.com>> writes: >> > > I think the goal of the "make_date/time/timestamp" function series >> is build >> > > a date/time/timestamp from scratch, so the use of 'make_timestamptz' >> is to >> > > build a specific timestamp with timezone and don't convert it. >> > >> > Yeah; we don't really want to incur an extra timezone rotation just to >> get >> > to a timestamptz. However, it's not clear to me if make_timestamptz() >> > needs to have an explicit zone parameter or not. It could just assume >> > that you meant the active timezone. >> > >> >> +1. And if you want a different timezone you can just set the 'timezone' >> GUC. >> > > Why wouldn't we have a version that optionally accepts the timezone? That > mirrors what you can currently do with a cast from text, and having to set > the GUC if you need a different TZ would be a real PITA. > It is not bad idea. What will be format for timezone in this case? Is a doble enough? last version of this patch attached (without overloading in this moment) > -- > Jim C. Nasby, Data Architect j...@nasby.net > 512.569.9461 (cell) http://jim.nasby.net > commit 8b7d40b3677847f572d2281994e4c10ba039f9ff Author: Pavel Stehule Date: Thu Dec 12 18:08:47 2013 +0100 initial implementation make_timestamp diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index a411e3a..3da8a8c 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -6735,6 +6735,76 @@ SELECT SUBSTRING('XY1234Z', 'Y*?([0-9]{1,3})'); + make_timetz + + + + make_timetz(hour int, + min int, + sec double precision) + + + +time with time zone + + Create time with time zone from hour, minute and seconds fields + +make_timetz(8, 15, 23.5) +08:15:23.5+01 + + + + + + make_timestamp + + + + make_timestamp(year int, + month int, + day int, + hour int, + min int, + sec double precision) + + + +timestamp + + Create timestamp from year, month, day, hour, minute and seconds fields + +make_timestamp(1-23, 7, 15, 8, 15, 23.5) +2013-07-15 08:15:23.5 + + + + + + make_timestamptz + + + + make_timestamptz(year int, + month int, + day int, + hour int, + min int, + sec double precision) + + + +timestamp with time zone + + Create timestamp with time zone from year, month, day, hour, minute + and seconds fields + +make_timestamp(1-23, 7, 15, 8, 15, 23.5) +2013-07-15 08:15:23.5+01 + + + + + now now() diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c index fe091da..7ee8729 100644 --- a/src/backend/utils/adt/date.c +++ b/src/backend/utils/adt/date.c @@ -1246,38 +1246,65 @@ timetypmodout(PG_FUNCTION_ARGS) } /* - * make_time - time constructor + * time constructor used for make_time and make_timetz */ -Datum -make_time(PG_FUNCTION_ARGS) +static TimeADT +make_time_internal(int hour, int min, double sec) { - int tm_hour = PG_GETARG_INT32(0); - int tm_min = PG_GETARG_INT32(1); - double sec = PG_GETARG_FLOAT8(2); TimeADT time; /* This should match the checks in DecodeTimeOnly */ - if (tm_hour < 0 || tm_min < 0 || tm_min > MINS_PER_HOUR - 1 || + if (hour < 0 || min < 0 || min > MINS_PER_HOUR - 1 || sec < 0 || sec > SECS_PER_MINUTE || - tm_hour > HOURS_PER_DAY || + hour > HOURS_PER_DAY || /* test for > 24:00:00 */ - (tm_hour == HOURS_PER_DAY && (tm_min > 0 || sec > 0))) + (hour == HOURS_PER_DAY && (min > 0 || sec > 0))) ereport(ERROR, (errcode(ERRCODE_DATETIME_FIELD_OVERFLOW), errmsg("time field value out of range: %d:%02d:%02g", - tm_hour, tm_min, sec))); + hour, min, sec))); /* This should match tm2time */ #ifdef HAVE_INT64_TIMESTAMP - time = (((tm_hour * MINS_PER_HOUR + tm_min) * SECS_PER_MINUTE) + time = (((hour * MINS_PER_HOUR + min) * SECS_PER_MINUTE) * USECS_PER_SEC) + rint(sec * USECS_PER_SEC); #else - time = ((tm_hour * MINS_PER_HOUR + tm_min) * SECS_PER_MINUTE) + sec; + time = ((hour * MINS_PER_HOUR + min) * SECS_PER_MINUTE) + sec; #endif + return time; +} + +/* + * make_time - time constructor + */ +Datum +make_time(PG_FUNCTION_ARGS) +{ + TimeADT time; + + time = make_time_inter
Re: [HACKERS] [PATCH] Negative Transition Aggregate Functions (WIP)
On Dec 15, 2013 6:44 PM, "Tom Lane" wrote: > David Rowley writes: > > I've attached an updated patch which includes some documentation. > > I've also added support for negfunc in CREATE AGGREGATE. Hopefully that's > > an ok name for the option, but if anyone has any better ideas please let > > them be known. > > I'd be a bit inclined to build the terminology around "reverse" instead of > "negative" --- the latter seems a bit too arithmetic-centric. But that's > just MHO. To contribute to the bike shedding, inverse is often used in similar contexts. -- Ants Aasma
Re: [HACKERS] [PATCH] Negative Transition Aggregate Functions (WIP)
David Rowley writes: > I've attached an updated patch which includes some documentation. > I've also added support for negfunc in CREATE AGGREGATE. Hopefully that's > an ok name for the option, but if anyone has any better ideas please let > them be known. I'd be a bit inclined to build the terminology around "reverse" instead of "negative" --- the latter seems a bit too arithmetic-centric. But that's just MHO. 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] [PATCH] Negative Transition Aggregate Functions (WIP)
On Sun, Dec 15, 2013 at 9:29 PM, Tom Lane wrote: > David Rowley writes: > > I guess the answer for the people that complain about slowness could be > > that they create their own aggregate function which implements float4pl > as > > the trans function and float4mi as the negative trans function. They can > > call it SUMFASTBUTWRONG() if they like. Perhaps it would be worth a note > in > > the documents for this patch? > > I think it would be an absolutely perfect documentation example to show > how to set up such an aggregate (and then point out the risks, of course). > > I've attached an updated patch which includes some documentation. I've also added support for negfunc in CREATE AGGREGATE. Hopefully that's an ok name for the option, but if anyone has any better ideas please let them be known. For the checks before the aggregate is created, I put these together quite quickly and I think I'm still missing a check to ensure that the strict property for the trans and negtrans functions are set to the same thing, though I do have a runtime check for this, it's likely bad to wait until then to tell the user about it. > As for numeric, I did start working on this just after I posted the > > original patch and before I saw your comment about it. I did end up > making > > do_numeric_desperse() which was to be the reverse of do_numeric_accum(), > > but I got stuck on the equivalent of when do_numeric_accum() > > does mul_var(&X, &X, &X2, X.dscale * 2); > > Ummm ... why doesn't it work to just use numeric_add and numeric_sub, > exactly parallel to the float case? > > I've not quite got back to this yet and I actually pulled out my initial try at this thinking that we didn't want it because it was affecting the scale. The transition function for SUM numeric does not seem to use numeric_add, it uses numeric_avg_accum as the transition function which lets do_numeric_accum do the hard work and that just does add_var. I changes these add_var's to sub_var's and altered the initial value to flip the sign so that NULL,10 would be -10 instead of 10. I think that's all it needs, and I guess I leave the dscale as is in this situation then? Regards David Rowley > regards, tom lane > negative_transition_funcs_v0.8.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Negative Transition Aggregate Functions (WIP)
On Sun, Dec 15, 2013 at 3:08 PM, Tom Lane wrote: > I wrote: > > It's not so good with two-row windows though: > > Actually, carrying that example a bit further makes the point even more > forcefully: > > Table correct sum of negative-transition > this + next value result > 1e201e201e20 + 1 = 1e20 > 1 1 1e20 - 1e20 + 0 = 0 > 0 0 0 - 1 + 0 = -1 > 0 1 -1 - 0 + 1 = 0 > 1 > > Those last few answers are completely corrupt. > > For sake of the archives I just wanted to reproduce this... I used the following query with the patch which was attached upthread to confirm this: SELECT sum(n::float8) OVER (ORDER BY i ROWS BETWEEN CURRENT ROW AND 1 FOLLOWING) FROM (VALUES(1,1e20),(2,1)) n(i,n); sum 1e+020 0 (2 rows) SUM(1) should equal 1 not 0. But unpatched I get: sum 1e+020 1 (2 rows) This discovery seems like good information to keep around, so I've added a regression test in my local copy of the patch to try to make sure nobody tries to add a negative trans for float or double in the future. Regards David Rowley > regards, tom lane >
Re: [HACKERS] [PATCH] Negative Transition Aggregate Functions (WIP)
David Rowley writes: > I guess the answer for the people that complain about slowness could be > that they create their own aggregate function which implements float4pl as > the trans function and float4mi as the negative trans function. They can > call it SUMFASTBUTWRONG() if they like. Perhaps it would be worth a note in > the documents for this patch? I think it would be an absolutely perfect documentation example to show how to set up such an aggregate (and then point out the risks, of course). > As for numeric, I did start working on this just after I posted the > original patch and before I saw your comment about it. I did end up making > do_numeric_desperse() which was to be the reverse of do_numeric_accum(), > but I got stuck on the equivalent of when do_numeric_accum() > does mul_var(&X, &X, &X2, X.dscale * 2); Ummm ... why doesn't it work to just use numeric_add and numeric_sub, exactly parallel to the float case? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers