I wrote:
> There's probably some overhead from traversing advance_transition_function
> for each row, which your version wouldn't have done. 15% sounds pretty
> high for that though, since advance_transition_function doesn't have much
> to do when the transition function is non strict and the stat
Andrew Gierth writes:
> "Tom" == Tom Lane writes:
> Tom> The key reason for that was, and remains, not having the
> Tom> behavior hard-wired in nodeAgg; I believe that this design
> Tom> permits things to be accomplished in aggregate implementation
> Tom> functions that would not have been po
> "Tom" == Tom Lane writes:
>> Furthermore, I can't help noticing that the increased complexity
>> has now pretty much negated your original arguments for moving so
>> much of the work out of nodeAgg.c.
Tom> The key reason for that was, and remains, not having the
Tom> behavior hard-wir
Andrew Gierth writes:
> Retesting with your changes shows that the gap is down to 15% but still
> present:
There's probably some overhead from traversing advance_transition_function
for each row, which your version wouldn't have done. 15% sounds pretty
high for that though, since advance_transit
> "Tom" == Tom Lane writes:
>> Initial tests suggest that your version is ~40% slower than ours on
>> some workloads.
Tom> I poked at this a bit with perf and oprofile, and concluded that
Tom> probably the difference comes from ordered_set_startup()
Tom> repeating lookups for each group
On Sun, Jan 5, 2014 at 12:00 PM, Tom Lane wrote:
>
>
> Looking at this example makes me wonder if it wouldn't be worthwhile to
> provide a way to reset and re-use a tuplesort object, instead of redoing
> all the lookup work involved. Or maybe just find a way to cache the
> catalog lookups that ar
Andrew Gierth writes:
> "Tom" == Tom Lane writes:
> Tom> I've committed this after significant editorialization --- most
> Tom> notably, I pushed control of the sort step into the aggregate
> Tom> support functions.
> Initial tests suggest that your version is ~40% slower than ours on
> some
> "Tom" == Tom Lane writes:
Tom> I've committed this after significant editorialization --- most
Tom> notably, I pushed control of the sort step into the aggregate
Tom> support functions.
Initial tests suggest that your version is ~40% slower than ours on
some workloads.
On my system, th
Sent from my iPad
> On 24-Dec-2013, at 2:50, Tom Lane wrote:
>
> Atri Sharma writes:
>> Please find attached the latest patch for WITHIN GROUP. This patch is
>> after fixing the merge conflicts.
>
> I've committed this after significant editorialization --- most notably,
> I pushed control o
Atri Sharma writes:
> Please find attached the latest patch for WITHIN GROUP. This patch is
> after fixing the merge conflicts.
I've committed this after significant editorialization --- most notably,
I pushed control of the sort step into the aggregate support functions.
I didn't like the way no
> "Tom" == Tom Lane writes:
Tom> What I'm now thinking we want to do is:
Tom> 1. Non-hypothetical direct args always contribute to determining
Tom> the agg's collation.
Tom> 2. Hypothetical and aggregated args contribute to the agg's
Tom> collation only if the agg is designed so that t
I wrote:
> Or, really, why don't we just do the same thing I'm advocating for
> the plain-ordered-set case? That is, if there's a single collation
> applying to all the collatable inputs, that's the collation to use
> for the aggregate; otherwise it has no determinate collation, and
> it'll throw
I wrote:
> ... So my reaction to this example is not
> that we should hack the behavior for plain ordered-set aggregates,
> but that we ought to find a rule that allows result-collation
> determination for hypotheticals. We speculated upthread about
> "merge the collations normally, but ignore inp
Andrew Gierth writes:
> The examples I've thought of which would return collatable types are
> all ones that would be implemented as plain ordered set functions even
> if their logic was in some sense hypothetical. For example you could
> envisage a value_prec(x) within group (order by y) that ret
> "Tom" == Tom Lane writes:
Tom> I eventually decided that we were overthinking this problem. At
Tom> least for regular ordered-set aggregates, we can just deem that
Tom> the collation of the aggregate is indeterminate unless all the
Tom> inputs (both direct and aggregated) agree on the
[ still hacking away at this patch ]
Andrew Gierth writes:
> "Tom" == Tom Lane writes:
> Tom> Not wanting to consider the sort args when there's more than one
> Tom> doesn't square with forcing them to be considered when there's
> Tom> just one. It's the same aggregate after all,
> This log
On 11/21/13, 5:04 AM, Atri Sharma wrote:
> Please find attached the latest patch for WITHIN GROUP. This patch is
> after fixing the merge conflicts.
I would like to see more explanations and examples in the documentation.
You introduce this feature with "Ordered set functions compute a single
res
> "Tom" == Tom Lane writes:
Tom> We could alternatively decide that the agg has level 0, but that
Tom> doesn't seem terribly useful, and I think it's not per spec
Tom> either. SQL:2008 section 6.9 seems
Tom> pretty clear that only aggregated arguments should be considered
Tom> when det
Andrew Gierth writes:
> There's also the question of ungrouped vars, maybe. Consider these two
> queries:
> select array(select a+sum(x) from (values (0.3),(0.7)) v(a) group by a)
> from generate_series(1,5) g(x);
> select array(select percentile_disc(a) within group (order by x)
>
> "Tom" == Tom Lane writes:
>> Hmm... yes, you're probably right; but we'd still have to check
>> somewhere for improper nesting, no? since not even the direct args
>> are allowed to contain nested aggregate calls.
Tom> Yeah, I had come to that same conclusion while making the
Tom> chan
Andrew Gierth writes:
> "Tom" == Tom Lane writes:
> Tom> After examining this more closely, ISTM that the direct
> Tom> arguments are supposed to be processed as if they weren't inside
> Tom> an aggregate call at all. That being the case, isn't it flat
> Tom> out wrong for check_agg_argument
> "Tom" == Tom Lane writes:
Tom> After examining this more closely, ISTM that the direct
Tom> arguments are supposed to be processed as if they weren't inside
Tom> an aggregate call at all. That being the case, isn't it flat
Tom> out wrong for check_agg_arguments() to be examining the
T
Andrew Gierth writes:
> "Tom" == Tom Lane writes:
> Tom> I believe that the spec requires that the "direct" arguments of
> Tom> an inverse or hypothetical-set aggregate must not contain any
> Tom> Vars of the current query level.
> False.
After examining this more closely, ISTM that the dire
> "Tom" == Tom Lane writes:
Tom> Not wanting to consider the sort args when there's more than one
Tom> doesn't square with forcing them to be considered when there's
Tom> just one. It's the same aggregate after all,
This logic is only applied in the patch to aggregates that _aren't_
hypo
Andrew Gierth writes:
> "Tom" == Tom Lane writes:
> Tom> Meh. I don't think you can have that and also have the behavior
> Tom> that multiple ORDER BY items aren't constrained to have the same
> Tom> collation; at least not without some rule that amounts to a
> Tom> special case for percenti
> "Tom" == Tom Lane writes:
Tom> 2. For an ordered set function, n must equal aggnfixedargs. We
Tom> treat all n fixed arguments as contributing to the aggregate's
Tom> result collation, but ignore the sort arguments.
>> That doesn't work for getting a sensible collation out of
>> perc
Andrew Gierth writes:
> The patch as submitted answers those questions as follows:
> CREATE AGGREGATE func(integer) WITHIN GROUP (text) ...
You've glossed over a significant amount of complexity, as shown by your
example that prints WITHIN GROUP (*), a syntax that you've not defined
here.
In th
> "Josh" == Josh Berkus writes:
>> Since I don't particularly trust my own judgement on aesthetics, I
>> used the feedback I got from others when deciding what to
>> do. Frankly I think this one needs wider input than just you and
>> me arguing over it.
Josh> Can someone paste examples
Josh Berkus writes:
> Can someone paste examples of the two syntax alternatives we're talking
> about here? I've lost track.
I'll leave it to Andrew to describe/defend exactly what his patch is
doing. The alternative I'm thinking about is that in CREATE AGGREGATE
as well as \da output, the argu
> "Tom" == Tom Lane writes:
Tom> Another thing to think about here is to wonder why the committee chose
Tom> anything as verbose as "agg(...) WITHIN GROUP (ORDER BY ...)" in the
Tom> first place. The words ORDER BY certainly seem pretty unnecessary.
All of the ordered-set functions that
On 12/06/2013 01:30 PM, Andrew Gierth wrote:
> Since I don't particularly trust my own judgement on aesthetics, I used
> the feedback I got from others when deciding what to do. Frankly I think
> this one needs wider input than just you and me arguing over it.
Can someone paste examples of the two
> "Tom" == Tom Lane writes:
>> pg_get_function_arguments()'s interface assumes that the caller is
>> providing the enclosing parens. Changing it would have meant
>> returning a result like 'float8) WITHIN GROUP (float8' which I
>> reckoned would have too much chance of breaking existing c
Andrew Gierth writes:
> "Tom" == Tom Lane writes:
> Tom> Actually, now that I think of it, why not use this syntax for
> Tom> declaration and display purposes:
> Tom> type1, type2 ORDER BY type3, type4
> But unfortunately it looks exactly like the calling sequence for a
> normal aggre
Andrew Gierth wrote
>> "Tom" == Tom Lane <
> tgl@.pa
> > writes:
>
> >> Please don't object that that doesn't look exactly like the syntax
> >> for calling the function, because it doesn't anyway --- remember
> >> you also need ORDER BY in the call.
>
> Tom> Actually, now that I think o
> "Tom" == Tom Lane writes:
>> Please don't object that that doesn't look exactly like the syntax
>> for calling the function, because it doesn't anyway --- remember
>> you also need ORDER BY in the call.
Tom> Actually, now that I think of it, why not use this syntax for
Tom> declaratio
I wrote:
> One possibility is to forget all the parens and say that the display
> looks like
> type1, type2 WITHIN GROUP type3, type4
> Please don't object that that doesn't look exactly like the syntax
> for calling the function, because it doesn't anyway --- remember you
> also need ORDER B
Andrew Gierth writes:
> "Tom" == Tom Lane writes:
> Tom> Regardless of that, though ... what is the reasoning for
> Tom> inventing pg_get_aggregate_arguments() rather than just making
> Tom> pg_get_function_arguments() do the right thing?
> pg_get_function_arguments()'s interface assumes that
> "Tom" == Tom Lane writes:
Tom> Regardless of that, though ... what is the reasoning for
Tom> inventing pg_get_aggregate_arguments() rather than just making
Tom> pg_get_function_arguments() do the right thing?
pg_get_function_arguments()'s interface assumes that the caller is
providing t
I don't especially like the syntax you invented for listing arguments in
CREATE AGGREGATE, especially not the WITHIN GROUP (*) special case.
If we stick with that we're going to need to touch a lot more places than
you have, notably regprocedure. I also fear that this syntax is not
appropriate for
Andrew Gierth writes:
> "Tom" == Tom Lane writes:
> Tom> I believe that the spec requires that the "direct" arguments of
> Tom> an inverse or hypothetical-set aggregate must not contain any
> Tom> Vars of the current query level.
> In any event, going by the docs on the web, Oracle does not f
> "Tom" == Tom Lane writes:
Tom> Further questions about WITHIN GROUP:
Tom> I believe that the spec requires that the "direct" arguments of
Tom> an inverse or hypothetical-set aggregate must not contain any
Tom> Vars of the current query level.
False.
The spec requires that the direct
David Johnston writes:
> The original design references the spec as allowing a column reference if it
> is a group by key:
> Select cume_dist(f1) within group ( order by f1 ) from text_tbl group by f1
> No example was shown where this would be useful...but nonetheless the
> comment and error bot
Tom Lane-2 wrote
> Further questions about WITHIN GROUP:
>
> I believe that the spec requires that the "direct" arguments of an inverse
> or hypothetical-set aggregate must not contain any Vars of the current
> query level. They don't manage to say that in plain English, of course,
> but in the
Further questions about WITHIN GROUP:
I believe that the spec requires that the "direct" arguments of an inverse
or hypothetical-set aggregate must not contain any Vars of the current
query level. They don't manage to say that in plain English, of course,
but in the case the behavior is defined
Andrew Gierth writes:
> "Tom" == Tom Lane writes:
> Tom> But anyway, what I'm thinking right now is that these questions
> Tom> would all go away if the aggregate transfunction were receiving
> Tom> the rows and sticking them into the tuplestore. It could add
> Tom> whatever columns it felt
> "Tom" == Tom Lane writes:
Tom> Well, sure, but I was only suggesting adding it when the
Tom> aggregate asks for it, probably via a new flag column in
Tom> pg_aggregate.
Sure, I was only pointing out the necessity.
Tom> The question you're evading is what additional functionality
Tom>
Andrew Gierth writes:
> "Tom" == Tom Lane writes:
> Tom> Well, okay, but you've not said anything that wouldn't be
> Tom> handled just as well by some logic that adds a fixed
> Tom> integer-constant-zero flag column to the rows going into the
> Tom> tuplesort.
> Adding such a column uncondit
> "Tom" == Tom Lane writes:
Tom> Well, okay, but you've not said anything that wouldn't be
Tom> handled just as well by some logic that adds a fixed
Tom> integer-constant-zero flag column to the rows going into the
Tom> tuplesort.
Adding such a column unconditionally even for non-hypothe
Andrew Gierth writes:
> "Tom" == Tom Lane writes:
> Tom> 1. I really hate the way you've overloaded the transvalue to do
> Tom> something that has approximately nothing to do with transition
> Tom> state (and haven't updated catalogs.sgml to explain that,
> Tom> either). Seems like it'd be c
> "Tom" == Tom Lane writes:
Tom> 1. I really hate the way you've overloaded the transvalue to do
Tom> something that has approximately nothing to do with transition
Tom> state (and haven't updated catalogs.sgml to explain that,
Tom> either). Seems like it'd be cleaner to just hardwire a
Atri Sharma writes:
> Please find attached the latest patch for WITHIN GROUP. This patch is
> after fixing the merge conflicts.
I've started to look at this patch now. I have a couple of immediate
reactions to the catalog changes:
1. I really hate the way you've overloaded the transvalue to do
> "Vik" == Vik Fearing writes:
Vik> I certainly want it. I do not have a copy of the SQL standard,
Vik> but I have full faith in the Andrew Gierth's claim that this is
Vik> part of it.
For reference, this is how I believe it matches up against the spec
(I'm working from the 2008 final):
On 11/21/2013 11:04 AM, Atri Sharma wrote:
> Please find attached the latest patch for WITHIN GROUP. This patch is
> after fixing the merge conflicts.
I have spent quite some time on this and the previous versions. Here is
my review, following the questions on the wiki.
This patch is in context
On Fri, 2013-11-15 at 00:05 +0530, Atri Sharma wrote:
> Please find the latest version of the patch. This version fixes the
> issues pointed out by the reviewer and the divide by zero bug in
> percent_rank function. This version also adds a regression test for
> the divide by zero case in percent_r
On 11/04/2013 08:43 AM, Atri Sharma wrote:
> Please find attached our latest version of the patch. This version
> fixes the issues pointed out by the reviewers.
No, it doesn't. The documentation still contains formatting and
grammatical errors, and the code comments still do not match the their
s
On 10/09/2013 04:19 PM, Pavel Stehule wrote:
>
> I checked a conformance with ANSI SQL - and I didn't find any issue.
>
> I found so following error message is not too friendly (mainly
> because this functionality will be new)
>
> postgres=# select dense_rank(3,3,2) within group (or
On Thu, Oct 10, 2013 at 10:35 PM, Andrew Gierth
wrote:
> The first alternative that springs to mind is:
>
> ERROR: Incorrect number of arguments for hypothetical set function
> DETAIL: Number of hypothetical arguments (3) must equal number of ordered
> columns (2)
I'd suggest trying to collapse
2013/10/11 Andrew Gierth
> > "Pavel" == Pavel Stehule writes:
>
> >> I found so following error message is not too friendly (mainly because
> >> this functionality will be new)
> >>
> >> postgres=# select dense_rank(3,3,2) within group (order by num desc,
> odd)
> >> from test4;
> >> E
> "Pavel" == Pavel Stehule writes:
>> I found so following error message is not too friendly (mainly because
>> this functionality will be new)
>>
>> postgres=# select dense_rank(3,3,2) within group (order by num desc, odd)
>> from test4;
>> ERROR: Incorrect number of arguments for hy
2013/10/9 Pavel Stehule
> Hello
>
> I checked a conformance with ANSI SQL - and I didn't find any issue.
>
> I found so following error message is not too friendly (mainly because
> this functionality will be new)
>
> postgres=# select dense_rank(3,3,2) within group (order by num desc, odd)
> fro
60 matches
Mail list logo
