remaining patches would need to be
rebased due to my changes.
David
From cbd37463bdaa96afed4c7c739c8e91b770a9f8a7 Mon Sep 17 00:00:00 2001
From: David Rowley
Date: Wed, 1 Feb 2023 19:35:16 +1300
Subject: [PATCH v8] Refactor heapam.c adding heapgettup_initial_block function
Here we adjust h
On Tue, 31 Jan 2023 at 12:18, Melanie Plageman
wrote:
>
> On Fri, Jan 27, 2023 at 10:34 PM David Rowley wrote:
> > 4. I think it might be a good idea to use unlikely() in if
> > (!scan->rs_inited). The idea is to help coax the compiler into moving
> > that code off to
On Wed, 1 Feb 2023 at 15:53, Yugo NAGATA wrote:
> Maybe, you missed to set plan_cache_mode to force_generic_plan.
> "Subplan Removed" doesn't appear when using a custom plan.
I wouldn't say that's 100% true. The planner is only able to prune
using values which are known during planning. Constant
On Wed, 1 Feb 2023 at 03:02, Melanie Plageman wrote:
>
> On Tue, Jan 31, 2023 at 11:46:05PM +1300, David Rowley wrote:
> > Both can be easily fixed, so no need to submit another patch as far as
> > I'm concerned.
>
> I realized I forgot a commit message in the second ver
On Wed, 1 Feb 2023 at 03:02, Melanie Plageman wrote:
>
> On Tue, Jan 31, 2023 at 11:46:05PM +1300, David Rowley wrote:
> > My thoughts were that we might want to put them
> > table_scan_getnextslot() and table_scan_getnextslot_tidrange(). My
> > rationale for that was tha
On Tue, 31 Jan 2023 at 09:57, Melanie Plageman
wrote:
> As for the asserts, I was at a bit of a loss as to where to put an
> assert which will make it clear that heapgettup() and
> heapgettup_pagemode() do not handle NoMovementScanDirection but was
> at a higher level of the executor.
My
On Wed, 4 Jan 2023 at 23:06, vignesh C wrote:
> patching file src/backend/access/heap/heapam.c
> Hunk #1 FAILED at 451.
> 1 out of 6 hunks FAILED -- saving rejects to file
> src/backend/access/heap/heapam.c.rej
I've moved this patch to the next CF. This patch has a dependency on
what's being
"On Wed, 25 Jan 2023 at 10:17, Melanie Plageman
wrote:
> I've added a comment but I didn't include the function name in it -- I
> find it repetitive when the comments above functions do that -- however,
> I'm not strongly attached to that.
I think the general format for header comments is:
/*
On Sat, 28 Jan 2023 at 12:15, Tom Lane wrote:
> /*
> * Determine the net effect of two direction specifications.
> * This relies on having ForwardScanDirection = +1, BackwardScanDirection =
> -1,
> * and will probably not do what you want if applied to any other values.
> */
> #define
On Wed, 25 Jan 2023 at 02:39, Melanie Plageman
wrote:
>
> On Tue, Jan 24, 2023 at 02:00:33PM +1300, David Rowley wrote:
> > If you feel strongly about that, then feel free to show me what you
> > have in mind in more detail so I can think harder about it.
>
> Nah, I don'
On Fri, 27 Jan 2023 at 01:30, Amit Langote wrote:
> It seems that the planner currently elides an Append/MergeAppend that
> has run-time pruning info (part_prune_index) set, but which I think is
> a bug.
This is actually how I intended it to work. Whether it was a good idea
or not, I'm currently
On Thu, 26 Jan 2023 at 23:29, John Naylor wrote:
> Coming back to this, I wanted to sketch out this idea in a bit more detail.
>
> Have two memtuple arrays, one for first sortkey null and one for first
> sortkey non-null:
> - Qsort the non-null array, including whatever specialization is
On Wed, 25 Jan 2023 at 13:55, Melanie Plageman
wrote:
> David Rowley and I were discussing how to test the
> NoMovementScanDirection case for heapgettup() and heapgettup_pagemode()
> in [1] (since there is not currently coverage). We are actually
> wondering if it is dead code (in
On Wed, 25 Jan 2023 at 19:46, Laurenz Albe wrote:
> Did you see Justin's wording suggestion in
> https://postgr.es/m/20230118174919.GA9837%40telsasoft.com ?
> He didn't attach it as a patch, so you may have missed it.
> I was pretty happy with that.
I didn't pay too much attention as I tend to
On Wed, 18 Jan 2023 at 22:15, Laurenz Albe wrote:
> Attached is a new version of my patch that tries to improve the wording.
I had a look at this and agree that we should adjust the paragraph in
question if people are finding it confusing.
For your wording, I found I had a small problem with
On Fri, 20 Jan 2023 at 00:26, vignesh C wrote:
> CFBot shows some compilation errors as in [1], please post an updated
> version for the same:
I've attached a rebased patch.
While reading over this again, I wondered if instead of allocating the
memory for the LOCALLOCKOWNER in TopMemoryContext,
On Fri, 30 Dec 2022 at 16:00, Richard Guo wrote:
>
> On Fri, Dec 9, 2022 at 5:16 PM Richard Guo wrote:
>>
>> Actually we do have checked PHVs for lateral references, earlier in
>> create_lateral_join_info. But that time we only marked lateral_relids
>> and direct_lateral_relids, without
Thanks for having a look at this.
On Tue, 24 Jan 2023 at 13:26, Melanie Plageman
wrote:
> Silly question, but was there any reason these were omitted in the first
> commit?
Good question, it's just that I didn't think of it at the time and
nobody else did or if they did, they didn't mention it.
On Mon, 16 Jan 2023 at 22:27, Richard Guo wrote:
> I happened to notice we have the case in memoize.sql that tests for
> memoize node with LATERAL joins, which is
>
> -- Try with LATERAL joins
> SELECT explain_memoize('
> SELECT COUNT(*),AVG(t2.unique1) FROM tenk1 t1,
> LATERAL (SELECT t2.unique1
9d9c02ccd [1] added infrastructure in the query planner and executor
so that the executor would know to stop processing a monotonic
WindowFunc when its value went beyond what some qual in the outer
query could possibly match in future evaluations due to the
WindowFunc's monotonic nature.
In that
On Thu, 19 Jan 2023 at 00:04, Peter Eisentraut
wrote:
> In your v2 patch, you remove these assertions:
>
> - /* check that rs_cindex is in sync */
> - Assert(scan->rs_cindex < scan->rs_ntuples);
> - Assert(lineoff == scan->rs_vistuples[scan->rs_cindex]);
>
> Is that intentional?
On Thu, 19 Jan 2023 at 06:01, Vik Fearing wrote:
> Thank you for the review. Attached is a new version rebased to d540a02a72.
I've only a bunch of nit-picks, personal preferences and random
thoughts to offer as a review:
1. I'd be inclined *not* to mention the possible future optimisation in:
On Thu, 19 Jan 2023 at 20:44, David Rowley wrote:
> Thanks. Pending one last look, I'm planning to move ahead with it
> unless there are any further objections or concerns.
I've now pushed this.
Thank you to everyone who reviewed or gave input on this patch.
David
On Tue, 10 Jan 2023 at 15:08, Andres Freund wrote:
> Thanks for letting me now. Updated version attached.
I'm not too sure I've qualified for giving a meaningful design review
here, but I have started looking at the patches and so far only made
it as far as 0006.
I noted down the following
On Fri, 20 Jan 2023 at 08:26, Ankit Kumar Pandey wrote:
> > On 19/01/23 18:49, David Rowley wrote:
> > You can just switch to using that function in
> > create_final_distinct_paths(). You'll need to consider if the query is
> > a DISTINCT ON query and not try the unordered
On Fri, 20 Jan 2023 at 10:23, Peter Smith wrote:
> Should the add_indent function also have a check to avoid making
> unnecessary calls to appendStringInfoSpaces when the level is 0?
Although I didn't opt to do that, thank you for having a look.
I do think the patch is trivially simple and
On Fri, 20 Jan 2023 at 10:25, Tom Lane wrote:
>
> Peter Smith writes:
> > Should the add_indent function also have a check to avoid making
> > unnecessary calls to appendStringInfoSpaces when the level is 0?
>
> Seems like unnecessary extra notation, seeing that appendStringInfoSpaces
> will
On Wed, 18 Jan 2023 at 08:27, Ankit Kumar Pandey wrote:
> There is bit confusion in wording here:
>
> "returns a List of pathkeys
> which are in keys1 but not in keys2 and NIL if keys2 has a pathkey
> that does not exist as a pathkey in keys1."
>
> You mean extract common keys without ordering
In [1] I noticed a bit of a poor usage of appendStringInfoString which
just appends 4 spaces in a loop, one for each indent level of the
jsonb. It should be better just to use appendStringInfoSpaces and
just append all the spaces in one go rather than appending 4 spaces in
a loop. That'll save
On Thu, 19 Jan 2023 at 06:27, Ankit Kumar Pandey wrote:
> Hmm, not really sure why did I miss that. I tried this again (added
> following in postgres.c above
>
> PortalStart)
>
> List* l = NIL;
> l = lappend(l, 1);
> l = lappend(l, 2);
> l = lappend(l, 3);
> l = lappend(l, 4);
>
> ListCell *lc;
>
On Wed, 18 Jan 2023 at 22:37, Richard Guo 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
On Tue, 10 Jan 2023 at 06:15, Ankit Kumar Pandey wrote:
>
>
> > On 09/01/23 17:53, David Rowley wrote:
> > I gave some thought to whether doing foreach_delete_current() is safe
> > within a foreach_reverse() loop. I didn't try it, but I couldn't see
> > any reason wh
On Wed, 18 Jan 2023 at 11:51, Tom Lane wrote:
> If nobody has any comments on this, I'm going to go ahead and push
> it. The value of the improvement is rapidly paling in comparison
> to the patch's maintenance effort.
No objections from me.
David
On Wed, 18 Jan 2023 at 14:55, Richard Guo wrote:
> Yeah, I noticed this too yesterday. I reviewed through these two
> patches yesterday and I think they are in good shape now.
I'm currently reviewing the two patches.
David
On Tue, 17 Jan 2023 at 13:16, Dean Rasheed wrote:
>
> On Wed, 11 Jan 2023 at 05:24, David Rowley wrote:
> >
> > I'm wondering if 1349d279 should have just never opted to presort
> > Aggrefs which have volatile functions so that the existing behaviour
> > of un
On Mon, 16 Jan 2023 at 06:52, Ankit Kumar Pandey wrote:
> Hence, input_rel->pathlist returns null for select distinct b,a from ab
> where a < 10; even if index is created on a.
>
> In order to tackle this, I have added index_pathkeys in indexpath node
> itself.
I don't think we should touch
On Wed, 11 Jan 2023 at 19:21, Ankit Kumar Pandey wrote:
> HashAgg has better cost than Unique even with incremental sort (tried
> with other case
>
> where we have more columns pushed down but still hashAgg wins).
I don't think you can claim that one so easily. The two should have
quite
On Wed, 11 Jan 2023 at 17:32, Tom Lane wrote:
>
> David Rowley writes:
> > I think whatever the fix is here, we should likely ensure that the
> > results are consistent regardless of which Aggrefs are the presorted
> > ones. Perhaps the easiest way to do tha
On Wed, 11 Jan 2023 at 15:46, Richard Guo wrote:
> However the scan/join plan's
> tlist does not contain random(), which I think we need to fix.
I was wondering if that's true and considered that we don't want to
evaluate random() for the sort then again when doing the aggregate
transitions, but
On Tue, 10 Jan 2023 at 18:36, Tom Lane wrote:
>
> David Rowley writes:
> > Ideally, our sort costing would just be better, but I think that
> > raises the bar a little too high to start thinking of making
> > improvements to that for this patch.
>
> It's tricki
On Wed, 11 Jan 2023 at 08:17, Ankit Kumar Pandey wrote:
>
>
> > On 10/01/23 10:53, David Rowley wrote:
>
> > the total cost is the same for both of these, but the execution time
> > seems to vary quite a bit.
>
> This is really weird, I tried it different w
On Tue, 10 Jan 2023 at 16:07, Richard Guo wrote:
> Sorry I didn't make myself clear. I mean currently on HEAD in planner.c
> from line 4847 to line 4857, we have the code to make sure we always use
> the more rigorous clause for explicit-sort case. I think this code is
> not necessary, because
On Tue, 10 Jan 2023 at 06:15, Ankit Kumar Pandey wrote:
> Do we have any pending items for this patch now?
I'm just wondering if not trying this when the query has a DISTINCT
clause is a copout. What I wanted to avoid was doing additional
sorting work for WindowAgg just to have it destroyed by
Thanks for having a look at this.
On Tue, 10 Jan 2023 at 02:28, Richard Guo wrote:
> +1 for the changes. A minor comment is that previously on HEAD for
> SELECT DISTINCT case, if we have to do an explicit full sort atop the
> cheapest path, we try to make sure to always use the more rigorous
>
On Mon, 9 Jan 2023 at 21:34, Ankit Kumar Pandey wrote:
>
>
> > On 09/01/23 03:48, David Rowley wrote:
> > 3. I don't think you need to set "index" on every loop. Why not just
> set it to foreach_current_index(l) - 1; break;
>
> Consider this query
&
On Thu, 5 Jan 2023 at 20:06, Richard Guo wrote:
> I reviewed this patch and have some comments.
Thanks for looking at this. I think I've fixed all the issues you mentioned.
One extra thing I noticed was that I had to add a new
VALGRIND_MAKE_MEM_DEFINED in AllocSetAlloc when grabbing an item off
On Tue, 6 Dec 2022 at 13:43, Arne Roland wrote:
> That being said, attached patch should fix the issue reported below.
I took a look over the v10 patch and ended up making adjustments to
the tests. I didn't quite see the need for the test to be as extensive
as you had them in v10. Neither join
On Mon, 9 Jan 2023 at 06:17, Ankit Kumar Pandey wrote:
> I have addressed all points 1-6 in the attached patch.
A few more things:
1. You're still using the 'i' variable in the foreach loop.
foreach_current_index() will work.
2. I think the "index" variable needs a better name.
On Mon, 9 Jan 2023 at 05:06, Vik Fearing wrote:
> +EXPLAIN (COSTS OFF)
> +SELECT empno,
> + depname,
> + min(salary) OVER (PARTITION BY depname ORDER BY empno) depminsalary,
> + sum(salary) OVER (PARTITION BY depname) depsalary,
> + count(*) OVER (ORDER BY enroll_date
On Sun, 8 Jan 2023 at 23:21, Ankit Kumar Pandey wrote:
> Please find attached patch with addressed issues mentioned before.
Here's a quick review:
1. You can use foreach_current_index(l) to obtain the index of the list element.
2. I'd rather see you looping backwards over the list in the first
On Sun, 8 Jan 2023 at 05:45, Ankit Kumar Pandey wrote:
> Attached patch with test cases.
I can look at this in a bit more detail if you find a way to fix the
case you mentioned earlier. i.e, push the sort down to the deepest
WindowAgg that has pathkeys contained in the query's ORDER BY
pathkeys.
(your email client still seems broken)
On Sun, 8 Jan 2023 at 05:27, Ankit Kumar Pandey wrote:
>
>
> While writing test cases, I found that optimization do not happen for
> case #1
>
> (which is prime candidate for such operation) like
>
> EXPLAIN (COSTS OFF)
> SELECT empno,
> depname,
>
On Sun, 8 Jan 2023 at 01:52, Ankit Kumar Pandey wrote:
> I am just wondering though, why can we not do top-N sort
> in optimized version if we include limit clause? Is top-N sort is
> limited to non strict sorting or
> cases last operation before limit is sort? .
Maybe the sort bound can be
Your email client seems to be adding additional vertical space to your
emails. I've removed the additional newlines in the quotes. Are you
able to fix the client so it does not do that?
On Sun, 8 Jan 2023 at 00:10, Ankit Kumar Pandey wrote:
>
> On 07/01/23 09:58, David Rowley wrote:
> &
While working on the regression tests added in a14a58329, I noticed
that DISTINCT does not make use of Incremental Sort. It'll only ever
do full sorts on the cheapest input path or make use of a path that's
already got the required pathkeys. Also, I see that
create_final_distinct_paths() is a
On Sat, 7 Jan 2023 at 02:11, Ankit Kumar Pandey wrote:
> > On 05/01/23 12:53, David Rowley wrote:
> >>
> >> We *can* reuse Sorts where a more strict or equivalent sort order is
> >> available. The question is how do we get the final WindowClause to do
> >&
On Thu, 5 Jan 2023 at 04:11, Ankit Kumar Pandey wrote:
>
> Attaching test cases for this (+ small change in doc).
>
> Tested this in one of WIP branch where I had modified
> select_active_windows and it failed
>
> as expected.
>
> Please let me know if something can be improved in this.
Thanks
On Thu, 5 Jan 2023 at 19:14, Ankit Kumar Pandey wrote:
> We are already doing something like I mentioned.
>
> Consider this example:
>
> explain SELECT rank() OVER (ORDER BY a), count(*) OVER (ORDER BY a,b)
> FROM abcd;
> QUERY PLAN
>
On Thu, 5 Jan 2023 at 16:12, Tom Lane wrote:
>
> David Rowley writes:
> > Additionally, it's also not that clear to me that sorting by more
> > columns in the sort below the WindowAgg would always be a win over
> > doing the final sort for the ORDER BY. What if the WHER
On Thu, 5 Jan 2023 at 15:18, Vik Fearing wrote:
>
> On 1/4/23 13:07, Ankit Kumar Pandey wrote:
> > Also, one thing, consider the following query:
> >
> > explain analyze select row_number() over (order by a,b),count(*) over
> > (order by a) from abcd order by a,b,c;
> >
> > In this case, sorting
On Wed, 4 Jan 2023 at 20:11, Richard Guo wrote:
>
> When trying Valgrind I came across some compiling warnings with
> USE_VALGRIND defined and --enable-cassert not configured. This is
> mainly because in this case we have MEMORY_CONTEXT_CHECKING defined
> while USE_ASSERT_CHECKING not defined.
On Tue, 3 Jan 2023 at 10:25, Tom Lane wrote:
> The thing that I find really distressing here is that it's been
> like this for years and none of our automated testing caught it.
> You'd have expected valgrind testing to do so ... but it does not,
> because we've never marked that word NOACCESS.
On Wed, 4 Jan 2023 at 23:21, Spring Zhong wrote:
> The plan is apparently inefficient, since the hash aggregate goes after the
> Cartesian product. We could expect the query's performance get much improved
> if the HashAggregate node can be pushed down to the SCAN node.
> Is someone has
On Wed, 4 Jan 2023 at 17:39, Vladimir Churyukin wrote:
>
> On Tue, Jan 3, 2023 at 7:41 PM David Rowley wrote:
>> From what I can see here, the motivation to make this a useful feature
>> is backwards from what is normal. I think if you're keen to see a
>> featur
On Wed, 4 Jan 2023 at 03:11, Ankit Kumar Pandey wrote:
> #2. If order by clause in the Window function is superset of order by in query
>
> explain analyze select a,row_number() over (order by a,b,c),count(*) over
> (order by a,b) from abcd order by a;
>
>
On Wed, 4 Jan 2023 at 16:15, Vladimir Churyukin wrote:
> As an end user that spends a lot of time optimizing pretty complicated
> queries, I'd say that something like this could be useful.
I think we really need to at least see that it *is* useful, not that
it *could be* useful. For example,
On Tue, 3 Jan 2023 at 19:59, Ankit Kumar Pandey wrote:
>
>
> On 03/01/23 08:38, David Rowley wrote:
> > Do you actually have a need for this or are you just trying to tick
> > off some TODO items?
> >
> I would say Iatter but reason I picked it up was more on side
On Mon, 26 Dec 2022 at 08:05, Ankit Kumar Pandey wrote:
> Also, inputs from other hackers are welcomed here.
I'm with Tom on this. I've never once used this feature to try to
figure out why a certain plan was chosen or not chosen.
Do you actually have a need for this or are you just trying to
On Mon, 26 Dec 2022 at 02:04, Ankit Kumar Pandey wrote:
> Point #1
>
> In the above query Oracle 10g performs 2 sorts, DB2 and Sybase perform 3
> sorts. We also perform 3.
This shouldn't be too hard to do. See the code in
select_active_windows(). You'll likely want to pay attention to the
On Fri, 23 Dec 2022 at 22:04, Peter Eisentraut
wrote:
>
> On 19.12.22 23:48, David Rowley wrote:
> > On Tue, 20 Dec 2022 at 11:42, Tom Lane wrote:
> >> I think Peter is entirely right to question whether *this* type's
> >> output function is performance-cri
On Fri, 23 Dec 2022 at 17:04, Richard Guo wrote:
> Thanks for the test! I looked at this and found that with WCO
> constraints we can also hit the buggy code. Based on David's test case,
> I came up with the following in the morning.
I ended up going with a WCO option test in the end. I
On Fri, 23 Dec 2022 at 16:22, Amit Langote wrote:
> Attached shows a test case I was able to come up with that I can see
> is broken by a61b1f74 though passes after applying Richard's patch.
Thanks for the test case. I'll look at this now.
+UPDATE rootp SET b = b || 'd' RETURNING a, b, c, d;
+
On Fri, 23 Dec 2022 at 15:21, Richard Guo wrote:
>
> On Thu, Dec 22, 2022 at 5:22 PM David Rowley wrote:
>> I still think we should have a test to ensure this is actually
>> working. Do you want to write one?
>
>
> I agree that we should have a test. According to
On Wed, 26 Oct 2022 at 14:38, David Rowley wrote:
>
> On Sun, 23 Oct 2022 at 03:03, Vik Fearing wrote:
> > Shouldn't it be able to detect that these two windows are the same and
> > only do one WindowAgg pass?
> >
> >
> > explain (verbose, costs
On Thu, 22 Dec 2022 at 21:18, Richard Guo wrote:
> My best guess is that this function is intended to share the same code
> pattern as in adjust_appendrel_attrs_multilevel. The recursion is
> needed as 'rel' can be more than one inheritance level below the top
> parent. I think we can keep the
On Thu, 22 Dec 2022 at 20:56, Tom Lane wrote:
>
> David Rowley writes:
> > 22.57% postgres [.] escape_json
>
> Hmm ... shouldn't we do something like
>
> -appendStringInfoString(buf, "\\b");
> +
On Tue, 20 Dec 2022 at 11:26, David Rowley wrote:
> It would be good to see some measurements to find out how much adding
> the strlen calls back in would cost us.
I tried this out. I'm not pretending I found the best test which
highlights how much the performance will change in a real
On Wed, 16 Nov 2022 at 23:56, David Rowley wrote:
> I've attached an updated patch. The 0002 is just intended to exercise
> these allocations a little bit, it's not intended for commit. I was
> using that to ensure valgrind does not complain about anything. It
> seems happy now.
On Wed, 21 Dec 2022 at 16:28, Takamichi Osumi (Fujitsu)
wrote:
> TRAP: failed Assert("dlist_is_empty(blocklist)"), File: "slab.c", Line: 564,
> PID: 16148
> I'm not sure, but this could be related to a recent commit (d21ded75fd) in
> [4].
It was. I've just pushed a fix. Thanks for
On Wed, 21 Dec 2022 at 18:04, Michael Paquier wrote:
> Good catch. Yes, d21ded7 looks to have issues. I am adding David in
> CC.
Thanks. I'll look now.
David
On Wed, 21 Dec 2022 at 13:15, Ranier Vilela wrote:
> IMO, I think that commit a61b1f7, has an oversight.
> Currently is losing the result of recursion of function
> translate_col_privs_multilevel.
>
> Once the variable result (Bitmapset pointer) is reassigned.
>
> Without a test case for this
On Wed, 21 Dec 2022 at 04:47, Andrew Dunstan wrote:
> jsonb.c:appendBinaryStringInfo(out, "", 4);
>
> None of these really bother me much, TBH. In fact the last one is
> arguably nicer because it tells you without counting how many spaces
> there are.
appendStringInfoSpaces()
On Fri, 16 Dec 2022 at 12:47, David Rowley wrote:
> Normally we add some enable_* GUC to leave an escape hatch when we add
> some new feature like this. I likely should have done that when I
> added 1349d279, but I didn't and I want to now.
>
> I mainly just shifted this discu
On Tue, 20 Dec 2022 at 21:19, John Naylor wrote:
>
>
> On Tue, Dec 20, 2022 at 10:36 AM David Rowley wrote:
> >
> > I'm planning on pushing the attached v3 patch shortly. I've spent
> > several days reading over this and testing it in detail along with
>
On Tue, 20 Dec 2022 at 11:42, Tom Lane wrote:
> I think Peter is entirely right to question whether *this* type's
> output function is performance-critical. Who's got large tables with
> jsonpath columns? It seems to me the type would mostly only exist
> as constants within queries.
The patch
On Tue, 20 Dec 2022 at 09:23, Peter Eisentraut
wrote:
> AFAICT, the code in question is for the text output of the jsonpath
> type, which is used ... for barely anything?
I think the performance of a type's output function is quite critical.
I've seen huge performance gains in COPY TO
On Mon, 19 Dec 2022 at 21:12, Andres Freund wrote:
> Perhaps we should make appendStringInfoString() a static inline function
> - most compilers can compute strlen() of a constant string at compile
> time.
I had wondered about that, but last time I looked into it there was a
small increase in
On Fri, 16 Dec 2022 at 00:10, David Rowley wrote:
> v3-0002 removes the 1.5 x cost pessimism from incremental sort and
> also rewrites how we make incremental sort paths. I've now gone
> through the remaining places where we create an incremental sort path
> to give all those the sa
Over on [1] Pavel reports that PG16's performance is slower than
PG14's for his ORDER BY aggregate query. This seems to be mainly down
to how the costing works for Incremental Sort. Now, ever since
1349d279, the planner will make a plan that's ordered by the GROUP BY
clause and add on the path
On Thu, 15 Dec 2022 at 18:26, David Rowley wrote:
> I propose the attached which gets rid of the not-so-great casting
> method that was originally added to this test to try and force the seq
> scan. It seems a little dangerous to put in hacks like that to force
> a particul
On Mon, 12 Dec 2022 at 14:48, David Rowley wrote:
>
> On Sun, 11 Dec 2022 at 11:05, Andres Freund wrote:
> > What about moving make_completion_tag() to cmdtag.c? Then we could just get
> > the entire CommandTagBehaviour struct at once. It's not super pretty to pass
&
On Tue, 13 Dec 2022 at 20:53, David Rowley wrote:
> I think what we need to do is: Do our best to give incremental sort
> the most realistic costs we can and accept that it might choose a
> worse plan in some cases. Users can turn it off if they really have no
> other means
I'm in the middle of working on making some adjustments to the costs
of Incremental Sorts and I see the patch I wrote changes the plan in
the drop-index-concurrently-1 isolation test.
The particular plan changed currently expects:
---
Sort
Sort Key:
On Wed, 9 Nov 2022 at 14:58, David Rowley wrote:
> v2 attached.
I've been looking at this again and this time around understand why
the * 1.5 pessimism factor was included in the incremental sort code.
If we create a table with a very large skew in the number of rows per
what will be our
Thanks for testing the patch.
On Mon, 12 Dec 2022 at 20:14, John Naylor wrote:
> v13-0001 to 0005:
> 2.60% postgres postgres [.] SlabFree
> + v4 slab:
>4.98% postgres postgres [.] SlabFree
>
> While allocation is markedly improved, freeing looks worse here.
Thanks for having a look at this.
On Sun, 11 Dec 2022 at 11:05, Andres Freund wrote:
>
> On 2022-12-10 20:32:06 +1300, David Rowley wrote:
> > @@ -20,13 +20,14 @@
> > typedef struct CommandTagBehavior
> > {
> > const char *name;
> > + const uin
...@mail.gmail.com
From 5900b1662aea3d82c4aad1f8a94d9f9242067e9c Mon Sep 17 00:00:00 2001
From: David Rowley
Date: Sat, 10 Dec 2022 20:27:01 +1300
Subject: [PATCH v1] Speed up creation of command completion tags
Author: David Rowley, Andres Freund
---
src/backend/tcop/cmdtag.c | 10 +-
src/bac
.On Mon, 5 Dec 2022 at 23:18, John Naylor wrote:
>
>
> On Mon, Dec 5, 2022 at 3:02 PM David Rowley wrote:
> > Going by [2], the instructions are very different with each method, so
> > other machines with different latencies on those instructions might
> > show some
On Fri, 9 Dec 2022 at 03:38, Tom Lane wrote:
> It's true that the cost attributed to the Agg node won't impact any
> live decisions in the plan level in which it appears. However, if
> that's a subquery, then the total cost attributed to the subplan
> could in principle affect plan choices in
On Fri, 9 Dec 2022 at 01:12, David Geier wrote:
> Both plans were captured on 14.5, which is indeed prior to 1349d279.
>
> I disabled sequential scan to show that there's an alternative plan
> which is superior to the chosen plan: Index Only Scan is more expensive
> and takes longer than the Seq
601 - 700 of 2928 matches
Mail list logo
