public inbox for [email protected]help / color / mirror / Atom feed
Re: Add proper planner support for ORDER BY / DISTINCT aggregates 5+ messages / 5 participants [nested] [flat]
* Re: Add proper planner support for ORDER BY / DISTINCT aggregates @ 2023-01-18 09:49 David Rowley <[email protected]> 0 siblings, 1 reply; 5+ messages in thread From: David Rowley @ 2023-01-18 09:49 UTC (permalink / raw) To: Richard Guo <[email protected]>; +Cc: Tom Lane <[email protected]>; Dean Rasheed <[email protected]>; Ronan Dunklau <[email protected]>; Justin Pryzby <[email protected]>; Pavel Luzanov <[email protected]>; [email protected]; Ranier Vilela <[email protected]> On Wed, 18 Jan 2023 at 22:37, Richard Guo <[email protected]> wrote: > I'm still confused that when the same scenario happens with ORDER BY in > an aggregate function, like in query 1, the result is different in that > we would get an unsorted output. > > I wonder if we should avoid this inconsistent behavior. It certainly seems pretty strange that aggregates with an ORDER BY behave differently from the query's ORDER BY. I'd have expected that to be the same. I've not looked to see why there's a difference, but suspect that we thought about how we want it to work for the query's ORDER BY and when ORDER BY aggregates were added, that behaviour was not considered. Likely finding the code or location where that code should be would help us understand if something was just forgotten in the aggregate's case. It's probably another question as to if we should be adjusting this behaviour now. David ^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Add proper planner support for ORDER BY / DISTINCT aggregates @ 2023-01-18 10:52 Dean Rasheed <[email protected]> parent: David Rowley <[email protected]> 0 siblings, 1 reply; 5+ messages in thread From: Dean Rasheed @ 2023-01-18 10:52 UTC (permalink / raw) To: David Rowley <[email protected]>; +Cc: Richard Guo <[email protected]>; Tom Lane <[email protected]>; Ronan Dunklau <[email protected]>; Justin Pryzby <[email protected]>; Pavel Luzanov <[email protected]>; [email protected]; Ranier Vilela <[email protected]> On Wed, 18 Jan 2023 at 09:49, David Rowley <[email protected]> wrote: > > On Wed, 18 Jan 2023 at 22:37, Richard Guo <[email protected]> wrote: > > I'm still confused that when the same scenario happens with ORDER BY in > > an aggregate function, like in query 1, the result is different in that > > we would get an unsorted output. > > > > I wonder if we should avoid this inconsistent behavior. > > It certainly seems pretty strange that aggregates with an ORDER BY > behave differently from the query's ORDER BY. I'd have expected that > to be the same. I've not looked to see why there's a difference, but > suspect that we thought about how we want it to work for the query's > ORDER BY and when ORDER BY aggregates were added, that behaviour was > not considered. > I think the behaviour of an ORDER BY in the query can also be pretty surprising. For example, consider: SELECT ARRAY[random(), random(), random()] FROM generate_series(1, 3); array ------------------------------------------------------------- {0.2335800863701647,0.14688842754711273,0.2975659224823368} {0.10616525384762876,0.8371175798972244,0.2936178886154661} {0.21679841321788262,0.5254761982948826,0.7789412240118161} (3 rows) which produces 9 different random values, as expected, and compare that to: SELECT ARRAY[random(), random(), random()] FROM generate_series(1, 3) ORDER BY random(); array --------------------------------------------------------------- {0.01952216253949679,0.01952216253949679,0.01952216253949679} {0.6735145595500629,0.6735145595500629,0.6735145595500629} {0.9406665780147616,0.9406665780147616,0.9406665780147616} (3 rows) which now only has 3 distinct random values. It's pretty counterintuitive that adding an ORDER BY clause changes the contents of the rows returned, not just their order. The trouble is, if we tried to fix that, we'd risk changing some other behaviour that users may have come to rely on. Regards, Dean ^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Add proper planner support for ORDER BY / DISTINCT aggregates @ 2023-01-18 15:46 Tom Lane <[email protected]> parent: Dean Rasheed <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Tom Lane @ 2023-01-18 15:46 UTC (permalink / raw) To: Dean Rasheed <[email protected]>; +Cc: David Rowley <[email protected]>; Richard Guo <[email protected]>; Ronan Dunklau <[email protected]>; Justin Pryzby <[email protected]>; Pavel Luzanov <[email protected]>; [email protected]; Ranier Vilela <[email protected]> Dean Rasheed <[email protected]> writes: > I think the behaviour of an ORDER BY in the query can also be pretty > surprising. Indeed. The fundamental question is this: in > SELECT ARRAY[random(), random(), random()] > FROM generate_series(1, 3) > ORDER BY random(); are those four occurrences of random() supposed to refer to the same value, or not? This only matters for volatile functions of course; with stable or immutable functions, textually-equal subexpressions should have the same value in any given row. It is very clear what we are supposed to do for SELECT random() FROM ... ORDER BY 1; which sadly isn't legal SQL anymore. It gets fuzzy as soon as we have SELECT random() FROM ... ORDER BY random(); You could make an argument either way for those being the same value or not, but historically we've concluded that it's more useful to deem them the same value. Then the behavior you show is not such a surprising extension, although it could be argued that such matches should only extend to identical top-level targetlist entries. > The trouble is, if we tried to fix that, we'd risk changing some other > behaviour that users may have come to rely on. Yeah. I'm hesitant to try to adjust semantics here; we're much more likely to get complaints than kudos. regards, tom lane ^ permalink raw reply [nested|flat] 5+ messages in thread
* [PATCH v2 3/4] expand binaryheap api @ 2023-07-20 16:52 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Nathan Bossart @ 2023-07-20 16:52 UTC (permalink / raw) --- src/common/binaryheap.c | 28 ++++++++++++++++++++++++++++ src/include/lib/binaryheap.h | 3 +++ 2 files changed, 31 insertions(+) diff --git a/src/common/binaryheap.c b/src/common/binaryheap.c index 7a590eec4a..526e56bbd0 100644 --- a/src/common/binaryheap.c +++ b/src/common/binaryheap.c @@ -211,6 +211,34 @@ binaryheap_remove_first(binaryheap *heap) return result; } +/* + * binaryheap_remove_node + * + * Removes the given node from the heap. O(log n) worst case. + */ +void +binaryheap_remove_node(binaryheap *heap, int n) +{ + int cmp; + + Assert(!binaryheap_empty(heap) && heap->bh_has_heap_property); + Assert(n >= 0 && n < heap->bh_size); + + /* compare last node to the one that is being removed */ + cmp = heap->bh_compare(heap->bh_nodes[--heap->bh_size], + heap->bh_nodes[n], + heap->bh_arg); + + /* remove the last node, placing it in the vacated entry */ + heap->bh_nodes[n] = heap->bh_nodes[heap->bh_size]; + + /* sift as needed to preserve the heap property */ + if (cmp > 0) + sift_up(heap, n); + else if (cmp < 0) + sift_down(heap, n); +} + /* * binaryheap_replace_first * diff --git a/src/include/lib/binaryheap.h b/src/include/lib/binaryheap.h index 06e3878cf7..9f09aff29f 100644 --- a/src/include/lib/binaryheap.h +++ b/src/include/lib/binaryheap.h @@ -76,8 +76,11 @@ extern void binaryheap_build(binaryheap *heap); extern void binaryheap_add(binaryheap *heap, Datum d); extern Datum binaryheap_first(binaryheap *heap); extern Datum binaryheap_remove_first(binaryheap *heap); +extern void binaryheap_remove_node(binaryheap *heap, int n); extern void binaryheap_replace_first(binaryheap *heap, Datum d); #define binaryheap_empty(h) ((h)->bh_size == 0) +#define binaryheap_size(h) ((h)->bh_size) +#define binaryheap_get_node(h, n) ((h)->bh_nodes[n]) #endif /* BINARYHEAP_H */ -- 2.25.1 --qDbXVdCdHGoSgWSk Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v2-0004-use-priority-queue-for-pg_restore-ready_list.patch" ^ permalink raw reply [nested|flat] 5+ messages in thread
* [PATCH v8 7/7] Allow to print raw parse tree. @ 2023-09-25 05:01 Tatsuo Ishii <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Tatsuo Ishii @ 2023-09-25 05:01 UTC (permalink / raw) --- src/backend/tcop/postgres.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 21b9763183..3e3653816e 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -651,6 +651,10 @@ pg_parse_query(const char *query_string) } #endif + if (Debug_print_parse) + elog_node_display(LOG, "raw parse tree", raw_parsetree_list, + Debug_pretty_print); + TRACE_POSTGRESQL_QUERY_PARSE_DONE(query_string); return raw_parsetree_list; -- 2.25.1 ----Next_Part(Mon_Sep_25_14_26_30_2023_752)---- ^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2023-09-25 05:01 UTC | newest] Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2023-01-18 09:49 Re: Add proper planner support for ORDER BY / DISTINCT aggregates David Rowley <[email protected]> 2023-01-18 10:52 ` Dean Rasheed <[email protected]> 2023-01-18 15:46 ` Tom Lane <[email protected]> 2023-07-20 16:52 [PATCH v2 3/4] expand binaryheap api Nathan Bossart <[email protected]> 2023-09-25 05:01 [PATCH v8 7/7] Allow to print raw parse tree. Tatsuo Ishii <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox