Re: [HACKERS] [PATCH] Negative Transition Aggregate Functions (WIP)

2013-12-15 Thread Tom Lane
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


Re: [HACKERS] [PATCH] Negative Transition Aggregate Functions (WIP)

2013-12-15 Thread David Rowley
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)

2013-12-15 Thread David Rowley
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)

2013-12-15 Thread Tom Lane
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)

2013-12-15 Thread Ants Aasma
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: make_timestamp function

2013-12-15 Thread Pavel Stehule
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] SSL: better default ciphersuite

2013-12-15 Thread James Cloos
> "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] [bug fix] pg_ctl always uses the same event source

2013-12-15 Thread Amit Kapila
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: Custom Scan APIs (Re: [HACKERS] Custom Plan node)

2013-12-15 Thread Shigeru Hanada
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] logical changeset generation v6.8

2013-12-15 Thread Robert Haas
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: [HACKERS] Extension Templates S03E11

2013-12-15 Thread Jeff Davis
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] [PATCH] Negative Transition Aggregate Functions (WIP)

2013-12-15 Thread David Rowley
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] [PATCH] Negative Transition Aggregate Functions (WIP)

2013-12-15 Thread David Fetter
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