Re: small erreport bug over partitioned table pgrowlocks module

2023-10-30 Thread David Rowley
On Tue, 31 Oct 2023 at 13:18, David Rowley wrote: > Here's a patch that puts the relam check last. I've pushed that patch. David

Re: small erreport bug over partitioned table pgrowlocks module

2023-10-30 Thread David Rowley
On Tue, 31 Oct 2023 at 13:00, jian he wrote: > BEGIN; > CREATE TABLE fk_parted_pk (a int PRIMARY KEY) PARTITION BY LIST (a); > SELECT * FROM pgrowlocks('fk_parted_pk'); > ERROR: only heap AM is supported > > error should be the following part: > if (rel->rd_rel->relkind ==

Re: Why is DEFAULT_FDW_TUPLE_COST so insanely low?

2023-10-30 Thread David Rowley
On Tue, 31 Oct 2023 at 03:01, Bruce Momjian wrote: > I think you just go and change it. Your number is better than what we > have, and if someone wants to suggest a better number, we can change it > later. I did some more experimentation on the actual costs of getting a tuple from a foreign

Re: Making aggregate deserialization (and WAL receive) functions slightly faster

2023-10-30 Thread David Rowley
On Mon, 30 Oct 2023 at 23:48, Amit Kapila wrote: > > On Fri, Oct 27, 2023 at 3:23 AM David Rowley wrote: > > * parallel.c in HandleParallelMessages(): > > * applyparallelworker.c in HandleParallelApplyMessages(): > > Both the above calls are used to handle ERROR/NOTICE

Re: Why is DEFAULT_FDW_TUPLE_COST so insanely low?

2023-10-29 Thread David Rowley
On Sun, 29 Oct 2023 at 12:45, Bruce Momjian wrote: > Has anything been done about this issue? Nothing has been done. I was hoping to get the attention of a few people who have dealt more with postgres_fdw in the past. I've attached a patch with adjusts DEFAULT_FDW_TUPLE_COST and sets it to 0.2.

Re: Use virtual tuple slot for Unique node

2023-10-29 Thread David Rowley
On Fri, 27 Oct 2023 at 22:05, Ashutosh Bapat wrote: > > On Fri, Oct 27, 2023 at 8:48 AM David Rowley wrote: > > I was uncertain if the old behaviour of when srcslot contains fewer > > attributes than dstslot was intended or not. What happens there is > > that we'd

Re: Use virtual tuple slot for Unique node

2023-10-26 Thread David Rowley
On Wed, 25 Oct 2023 at 22:48, Ashutosh Bapat wrote: > We may save the size of data in VirtualTupleTableSlot, thus avoiding > the first loop. I assume that when allocating > VirtualTupleTableSlot->data, we always know what size we are > allocating so it should be just a matter of saving it in >

Re: Making aggregate deserialization (and WAL receive) functions slightly faster

2023-10-26 Thread David Rowley
On Thu, 26 Oct 2023 at 17:00, David Rowley wrote: > Thanks for looking at this again. I fixed up each of those and pushed > the result, mentioning the incompatibility in the commit message. > > Now that that's done, I've attached a patch which makes use of the new > initRea

Re: Making aggregate deserialization (and WAL receive) functions slightly faster

2023-10-25 Thread David Rowley
On Thu, 26 Oct 2023 at 08:43, Tom Lane wrote: > I think that we can make that assumption starting with v17. > Back-patching it would be hazardous perhaps; but if there's some > function out there that depends on NUL termination, testing should > expose it before too long. Wouldn't hurt to

Re: Add null termination to string received in parallel apply worker

2023-10-25 Thread David Rowley
On Wed, 11 Oct 2023 at 19:54, Zhijie Hou (Fujitsu) wrote: > The parallel apply worker didn't add null termination to the string received > from the leader apply worker via the shared memory queue. This action doesn't > bring bugs as it's binary data but violates the rule established in >

Re: Document aggregate functions better w.r.t. ORDER BY

2023-10-25 Thread David Rowley
On Thu, 26 Oct 2023 at 13:10, David G. Johnston wrote: > Question: Do you know whether we for certain always sort ascending here to > compute the unique values or whether if, say, there is an index on the column > in descending order (or ascending and traversed backwards) that the data >

Re: Add new for_each macros for iterating over a List that do not require ListCell pointer

2023-10-24 Thread David Rowley
On Wed, 25 Oct 2023 at 06:00, Jelte Fennema wrote: > Attached is a fixed version. With foreach(), we commonly do "if (lc == NULL)" at the end of loops as a way of checking if we did "break" to terminate the loop early. Doing the equivalent with the new macros won't be safe as the list element's

Re: Making aggregate deserialization (and WAL receive) functions slightly faster

2023-10-24 Thread David Rowley
On Tue, 17 Oct 2023 at 20:39, David Rowley wrote: > > On Mon, 16 Oct 2023 at 05:56, Tom Lane wrote: > > I guess the next question is whether we want to stop here or > > try to relax the requirement about NUL-termination. I'd be inclined > > to call that a separate is

Re: Use virtual tuple slot for Unique node

2023-10-23 Thread David Rowley
on and we may end up not de-toasting toasted Datums. > On Thu, Oct 19, 2023 at 4:26 PM David Rowley wrote: > > We can see that Q2 and Q3 become a bit slower. This makes sense as > > tts_virtual_materialize() is quite a bit more complex than > > heap_copy_minimal_tuple() which is a

Re: Use virtual tuple slot for Unique node

2023-10-19 Thread David Rowley
On Thu, 19 Oct 2023 at 22:29, David Rowley wrote: > It's hard to imagine why there would be a slowdown as this query uses > a TTSOpsMinimalTuple slot type in the patch and the unpatched version. I shrunk down your table sizes to 10k rows instead of 1 million rows to reduce the CPU cache pr

Re: Use virtual tuple slot for Unique node

2023-10-19 Thread David Rowley
On Thu, 12 Oct 2023 at 23:06, Ashutosh Bapat wrote: > Q7 select distinct a,b from (select string_agg(left(a, 100), ', ') > over (order by a rows 2 preceding) a, b from t_text) q > HEAD: 16070.62 ms > patched: 16182.16 ms Did you time the SELECT or EXPLAIN ANALYZE? With SELECT, I'm unable to

Re: run pgindent on a regular basis / scripted manner

2023-10-18 Thread David Rowley
ts. I may have missed something on the thread, but it could be that, or it could be due to the fact that pgindent just simply does more adjustments to comments than it does with code lines. On Wed, 18 Oct 2023 at 06:40, David Rowley wrote: > > I agree that it's not nice to add yet another way of

Re: run pgindent on a regular basis / scripted manner

2023-10-17 Thread David Rowley
On Wed, 18 Oct 2023 at 01:47, Robert Haas wrote: > On Sat, Aug 12, 2023 at 5:53 PM Peter Geoghegan wrote: > > This policy isn't working. > > +1. I think this is more annoying than the status quo ante. Maybe there are two camps of committers here; ones who care about committing correctly

Re: Making aggregate deserialization (and WAL receive) functions slightly faster

2023-10-17 Thread David Rowley
On Mon, 16 Oct 2023 at 05:56, Tom Lane wrote: > * in initStringInfoFromString, str->maxlen must be set to len+1 not len > > * comment in exec_bind_message doesn't look like pgindent will like it > > * same in record_recv, plus it has a misspelling "Initalize" > > * in stringinfo.c, inclusion of

Re: [Doc] Glossary Term Definitions Edits

2023-10-13 Thread David Rowley
On Sat, 14 Oct 2023, 5:20 pm Andrew Atkinson, wrote: > >- Many examples of “an SQL”. I changed those to “a SQL...”. For >example I changed “An SQL command which” to “A SQL command that”. I'm not >an English major so maybe I'm missing something here. > > It would depend on how you

Re: Making aggregate deserialization (and WAL receive) functions slightly faster

2023-10-12 Thread David Rowley
On Wed, 11 Oct 2023 at 08:52, Tom Lane wrote: > > David Rowley writes: > > I've attached a slightly more worked on patch that makes maxlen == 0 > > mean read-only. Unsure if a macro is worthwhile there or not. > > A few thoughts: Thank you for the review. I spent more

Re: Problem, partition pruning for prepared statement with IS NULL clause.

2023-10-12 Thread David Rowley
On Mon, 9 Oct 2023 at 12:26, David Rowley wrote: > > On Sat, 7 Oct 2023 at 03:11, Sergei Glukhov wrote: > > I noticed that combination of prepared statement with generic plan and > > 'IS NULL' clause could lead partition pruning to crash. > > > Test case: > &g

Re: Special-case executor expression steps for common combinations

2023-10-12 Thread David Rowley
On Thu, 12 Oct 2023 at 22:54, Daniel Gustafsson wrote: > EEOP_FUNCEXPR_STRICT_* (10M iterations): > master : (7503.317, 7553.691, 7634.524) > patched : (7422.756, 7455.120, 7492.393) > > pgbench: > master : (3653.83, 3792.97, 3863.70) > patched : (3743.04, 3830.02, 3869.80) > >

Re: Some performance degradation in REL_16 vs REL_15

2023-10-12 Thread David Rowley
On Thu, 12 Oct 2023 at 21:01, Anton A. Melnikov wrote: > > Greetengs! > > Found that simple test pgbench -c20 -T20 -j8 gives approximately > for REL_15_STABLE at 5143f76: 336+-1 TPS > and > for REL_16_STABLE at 4ac7635f: 324+-1 TPS > > And is it worth spending time bisecting for the commit where

Re: Doc: Minor update for enable_partitionwise_aggregate

2023-10-12 Thread David Rowley
On Wed, 11 Oct 2023 at 19:38, David Rowley wrote: > > On Wed, 11 Oct 2023 at 16:26, Andrew Atkinson wrote: > > "which allows grouping or aggregation on partitioned tables to be performed > > separately for each partition." > > This looks good to me

Re: Problem, partition pruning for prepared statement with IS NULL clause.

2023-10-12 Thread David Rowley
On Wed, 11 Oct 2023 at 22:59, Sergei Glukhov wrote: > > Unfortunately, I'd not long sent the last email and realised that the > > step_lastkeyno parameter is now unused and can just be removed from > > both get_steps_using_prefix() and get_steps_using_prefix_recurse(). > > This requires some

Re: Add null termination to string received in parallel apply worker

2023-10-11 Thread David Rowley
On Thu, 12 Oct 2023 at 05:04, Tom Lane wrote: > > Alvaro Herrera writes: > > I was thinking about this when skimming the other StringInfo thread a > > couple of days ago. I wondered if it wouldn't be more convenient to > > change the convention that all StringInfos are null-terminated: what is

Re: Problem, partition pruning for prepared statement with IS NULL clause.

2023-10-11 Thread David Rowley
On Wed, 11 Oct 2023 at 22:09, Sergei Glukhov wrote: > Thanks for fixing this! > > I verified that issues are fixed. Thanks for having a look. Unfortunately, I'd not long sent the last email and realised that the step_lastkeyno parameter is now unused and can just be removed from both

Re: Problem, partition pruning for prepared statement with IS NULL clause.

2023-10-11 Thread David Rowley
On Wed, 11 Oct 2023 at 15:49, David Rowley wrote: > It might have been better if PartClauseInfo could also describe IS > NULL quals, but I feel if we do that now then it would require lots of > careful surgery in partprune.c to account for that. Probably the fix > should

Re: Doc: Minor update for enable_partitionwise_aggregate

2023-10-11 Thread David Rowley
On Wed, 11 Oct 2023 at 16:26, Andrew Atkinson wrote: > > Thank you for the feedback and clarifications Ashutosh. How about this? > > "which allows grouping or aggregation on partitioned tables to be performed > separately for each partition." This looks good to me. I can take care of this.

Re: Problem, partition pruning for prepared statement with IS NULL clause.

2023-10-10 Thread David Rowley
On Tue, 10 Oct 2023 at 21:31, Sergei Glukhov wrote: > create table hp (a int, b text, c int, d int) >partition by hash (a part_test_int4_ops, b part_test_text_ops, c > part_test_int4_ops); > create table hp0 partition of hp for values with (modulus 4, remainder 0); > create table hp3

Re: Check each of base restriction clauses for constant-FALSE-or-NULL

2023-10-10 Thread David Rowley
On Sat, 7 Oct 2023 at 22:44, Richard Guo wrote: > > In relation_excluded_by_constraints() when we're trying to figure out > whether the relation need not be scanned, one of the checks we do is to > detect constant-FALSE-or-NULL restriction clauses. Currently we perform > this check only when

Re: Use virtual tuple slot for Unique node

2023-10-10 Thread David Rowley
On Wed, 27 Sept 2023 at 20:01, David Rowley wrote: > > On Sat, 23 Sept 2023 at 03:15, Heikki Linnakangas wrote: > > So not a win in this case. Could you peek at the outer slot type, and > > use the same kind of slot for the Unique's result? Or some more > > complicated l

Re: Crash in add_paths_to_append_rel

2023-10-09 Thread David Rowley
On Mon, 9 Oct 2023 at 22:49, Richard Guo wrote: > I came across a crash in add_paths_to_append_rel() with the query below. > It was introduced by commit a8a968a8 where we referenced > cheapest_startup_path->param_info without first checking that we have a > valid cheapest_startup_path. I pushed

Re: Making aggregate deserialization (and WAL receive) functions slightly faster

2023-10-09 Thread David Rowley
On Tue, 10 Oct 2023 at 06:38, Tom Lane wrote: > Hm. I'd be inclined to use maxlen == 0 as the indicator of a read-only > buffer, just because that would not create a problem if we ever want > to change it to an unsigned type. Other than that, I agree with the > idea of using a special maxlen

Re: Crash in add_paths_to_append_rel

2023-10-09 Thread David Rowley
On Mon, 9 Oct 2023 at 22:49, Richard Guo wrote: > I came across a crash in add_paths_to_append_rel() with the query below. > It was introduced by commit a8a968a8 where we referenced > cheapest_startup_path->param_info without first checking that we have a > valid cheapest_startup_path. Thank

Re: Draft LIMIT pushdown to Append and MergeAppend patch

2023-10-09 Thread David Rowley
On Mon, 9 Oct 2023 at 23:35, Ashutosh Bapat wrote: > > On Mon, Oct 9, 2023 at 6:25 AM David Rowley wrote: > > > > However, it may also be worth you reading over [3] and the ultimate > > reason I changed my mind on that being a good idea. Pushing LIMITs > > below an

Re: Making aggregate deserialization (and WAL receive) functions slightly faster

2023-10-09 Thread David Rowley
On Mon, 9 Oct 2023 at 21:17, David Rowley wrote: > Here are some more thoughts on how we could improve this: > > 1. Adjust the definition of StringInfoData.maxlen to define that -1 > means the StringInfoData's buffer is externally managed. > 2. Adjust enlargeStringInfo() to add a c

Re: Making aggregate deserialization (and WAL receive) functions slightly faster

2023-10-09 Thread David Rowley
On Mon, 9 Oct 2023 at 17:37, Tom Lane wrote: > Sorry for not having paid more attention to this thread ... but > I'm pretty desperately unhappy with the patch as-pushed. I agree > with the criticism that this is a very repetitive coding pattern > that could have used a macro. But my real

Re: Making aggregate deserialization (and WAL receive) functions slightly faster

2023-10-08 Thread David Rowley
On Thu, 5 Oct 2023 at 21:24, David Rowley wrote: > > On Thu, 5 Oct 2023 at 18:23, Michael Paquier wrote: > > Ahem, well. Based on this argument my own argument does not hold > > much. Perhaps I'd still use a macro at the top of array_userfuncs.c > > and numeric.c, to

Re: pg16: XX000: could not find pathkey item to sort

2023-10-08 Thread David Rowley
On Mon, 9 Oct 2023 at 12:42, David Rowley wrote: > Maybe it's worth checking the total planning time spent in a run of > the regression tests with and without the patch to see how much > overhead it adds to the "average case". I've now pushed the patch that trims off the Pat

Re: Does anyone ever use OPTIMIZER_DEBUG?

2023-10-08 Thread David Rowley
On Mon, 9 Oct 2023 at 10:28, Tom Lane wrote: > > David Rowley writes: > > It looks like nobody is objecting to this. I understand that not > > everyone who might object will have read this email thread, so what I > > propose to do here is move along and just comm

Re: Draft LIMIT pushdown to Append and MergeAppend patch

2023-10-08 Thread David Rowley
On Sun, 8 Oct 2023 at 18:32, Michał Kłeczek wrote: > On 8 Oct 2023, at 03:33, Andy Fan wrote: >> For the patches for performance improvement, it is better to provide >> an example to show how much benefits we can get. As for this case, >> I'm doubtful it can work as an improvement. > Could

Re: pg16: XX000: could not find pathkey item to sort

2023-10-08 Thread David Rowley
On Sun, 8 Oct 2023 at 23:52, Richard Guo wrote: > On Thu, Oct 5, 2023 at 2:26 PM David Rowley wrote: >> >> So in short, I propose the attached fix without any regression tests >> because I feel that any regression test would just mark that there was >> a big in creat

Re: Problem, partition pruning for prepared statement with IS NULL clause.

2023-10-08 Thread David Rowley
On Sat, 7 Oct 2023 at 03:11, Sergei Glukhov wrote: > I noticed that combination of prepared statement with generic plan and > 'IS NULL' clause could lead partition pruning to crash. > Test case: > -- > set plan_cache_mode to force_generic_plan; > prepare stmt AS select * from hp where a is

Re: Does anyone ever use OPTIMIZER_DEBUG?

2023-10-08 Thread David Rowley
On Tue, 3 Oct 2023 at 12:29, David Rowley wrote: > > On Fri, 29 Sept 2023 at 10:59, Tom Lane wrote: > > We could also discuss keeping the "tracing" aspect of it, but > > replacing debug_print_rel with pprint(rel), which'd still allow > > removal of all the &

Re: Making aggregate deserialization (and WAL receive) functions slightly faster

2023-10-05 Thread David Rowley
On Thu, 5 Oct 2023 at 18:23, Michael Paquier wrote: > > On Wed, Oct 04, 2023 at 07:47:11PM +1300, David Rowley wrote: > > The original patch had a new function in stringinfo.c which allowed a > > StringInfoData to be initialised from an existing string with some > > g

Re: make add_paths_to_append_rel aware of startup cost

2023-10-05 Thread David Rowley
On Thu, 5 Oct 2023 at 14:11, Andy Fan wrote: > Patch LGTM. Thanks. Pushed. David

Re: [PATCH] Fix memory leak in memoize for numeric key

2023-10-05 Thread David Rowley
On Wed, 4 Oct 2023 at 21:08, Orlov Aleksej wrote: > I've finished testing the patch. > I confirm that the patch solves the problem and works just as fast. Thanks for checking that. I've pushed the patch now. David

Re: pg16: XX000: could not find pathkey item to sort

2023-10-05 Thread David Rowley
On Tue, 3 Oct 2023 at 20:16, David Rowley wrote: > I wonder if the attached patch is too much of a special case fix. I > guess from the lack of complaints previously that there are no other > cases where we could possibly have pathkeys that belong to columns > that are aggregated. I

Re: Making aggregate deserialization (and WAL receive) functions slightly faster

2023-10-04 Thread David Rowley
Thanks for taking a look at this. On Wed, 4 Oct 2023 at 16:57, Michael Paquier wrote: > + buf.len = VARSIZE_ANY_EXHDR(sstate); > + buf.maxlen = 0; > + buf.cursor = 0; > > Perhaps it would be worth hiding that in a macro defined in > stringinfo.h? The original patch had a new

Re: make add_paths_to_append_rel aware of startup cost

2023-10-03 Thread David Rowley
On Sun, 1 Oct 2023 at 21:26, Andy Fan wrote: >> But overall, I'm more inclined to just go with the more simple "add a >> cheap unordered startup append path if considering cheap startup >> plans" version. I see your latest patch does both. So, I'd suggest two >> patches as I do see the merit in

Re: [PATCH] Fix memory leak in memoize for numeric key

2023-10-03 Thread David Rowley
On Tue, 3 Oct 2023 at 19:38, Orlov Aleksej wrote: > I found a query which consumes a lot of memory and triggers OOM killer. > Memory leak occurs in memoize node for numeric key. Thanks for the analysis and the patch. > I've attached memoize_memory_leak_numeric_key.patch to address this. Yeah,

Re: pg16: XX000: could not find pathkey item to sort

2023-10-03 Thread David Rowley
On Tue, 3 Oct 2023 at 09:11, David Rowley wrote: > I'm concerned that this patch will be too much overhead when creating > paths when a PathKey's EquivalenceClass has a large number of members > from partitioned tables. I just tried out the patch to see how much it affects the pe

Re: Making aggregate deserialization (and WAL receive) functions slightly faster

2023-10-02 Thread David Rowley
On Sun, 12 Feb 2023 at 23:43, David Rowley wrote: > > On Sun, 12 Feb 2023 at 19:39, Tom Lane wrote: > > It could maybe be okay if we added the capability for StringInfoData > > to understand (and enforce) that its "data" buffer is read-only. > > However, that'd

Re: Fixup some more appendStringInfo misusages

2023-10-02 Thread David Rowley
On Tue, 3 Oct 2023 at 11:24, David Rowley wrote: > This is along the same lines as 8b26769bc, f736e188c, 110d81728 and 8abc13a88. I've pushed the patch to fix the misusages of the functions. David

Re: Does anyone ever use OPTIMIZER_DEBUG?

2023-10-02 Thread David Rowley
On Fri, 29 Sept 2023 at 10:59, Tom Lane wrote: > We could also discuss keeping the "tracing" aspect of it, but > replacing debug_print_rel with pprint(rel), which'd still allow > removal of all the "DEBUG SUPPORT" stuff at the bottom of allpaths.c. > That's pretty much all of the

Fixup some more appendStringInfo misusages

2023-10-02 Thread David Rowley
The attached v1-0001 patch adjusts some code in stringinfo.h to find misusages of the appendStringInfo functions. I don't intend to commit that, but I do intend to commit the patch for the new misusages that it found, which is also attached. This is along the same lines as 8b26769bc, f736e188c,

Re: pg16: XX000: could not find pathkey item to sort

2023-10-02 Thread David Rowley
On Tue, 19 Sept 2023 at 23:45, Richard Guo wrote: > My first thought about the fix is that we artificially add resjunk > target entries to parse->targetList for the ordered aggregates' > arguments that are ORDER BY expressions, as attached. While this can > fix the given query, it would cause

Re: Avoid a possible out-of-bounds access (src/backend/optimizer/util/relnode.c)

2023-09-28 Thread David Rowley
On Fri, 29 Sept 2023 at 01:00, Ranier Vilela wrote: > Perhaps, and using your own words, the leaders on this project seem > to be against reviewers armed with blunderbuss, too. I don't have any ideas on what you're talking about here, but if this is a valid concern that you think is unfair then

Does anyone ever use OPTIMIZER_DEBUG?

2023-09-28 Thread David Rowley
In c4a1933b4 I pushed a fix for a 4 year old bug in print_path() where I'd forgotten to add handling for TidRangePaths while working on bb437f995. 4 years is quite a long time for such a bug. Maybe that's because nobody uses OPTIMIZER_DEBUG. I certainly don't, and Tom mentions [1] he doesn't

Re: Is it worth adding Assert(false) for unknown paths in print_path()?

2023-09-28 Thread David Rowley
On Fri, 29 Sept 2023 at 03:23, Tom Lane wrote: > FWIW, I'd argue for dropping print_path rather than continuing to > maintain it. I never use it, finding pprint() to serve the need > better and more reliably. Then perhaps we just need to open a thread with an appropriate subject to check if

Is it worth adding Assert(false) for unknown paths in print_path()?

2023-09-28 Thread David Rowley
In [1] Andrey highlighted that I'd forgotten to add print_path() handling for TidRangePaths in bb437f995. I know the OPTIMIZER_DEBUG code isn't exactly well used. I never personally use it and I work quite a bit in the planner, however, if we're keeping it, I thought maybe we might get the memo

Re: Avoid a possible out-of-bounds access (src/backend/optimizer/util/relnode.c)

2023-09-27 Thread David Rowley
On Thu, 28 Sept 2023 at 02:37, Ranier Vilela wrote: >> Please check [1] for the mention of: >> >> "The fastest way to get your patch rejected is to make unrelated >> changes. Reformatting lines that haven't changed, changing unrelated >> comments you felt were poorly worded, touching code not

Re: Set enable_seqscan doesn't take effect?

2023-09-27 Thread David Rowley
On Thu, 28 Sept 2023 at 13:47, jacktby jacktby wrote: > > postgres=# SET enable_seqscan = off; > SET > postgres=# explain select * from t; >QUERY PLAN > - > Seq Scan on t

Re: Should consider materializing the cheapest inner path in consider_parallel_nestloop()

2023-09-27 Thread David Rowley
On Fri, 8 Sept 2023 at 19:14, Richard Guo wrote: > explain select * from partsupp join lineitem on l_partkey > ps_partkey; > QUERY PLAN > -- > Gather (cost=0.00..1807085.44

Re: Should consider materializing the cheapest inner path in consider_parallel_nestloop()

2023-09-27 Thread David Rowley
On Fri, 8 Sept 2023 at 09:41, Robert Haas wrote: > > On Tue, Sep 5, 2023 at 8:07 AM Richard Guo wrote: > > Yeah, this seems an omission in commit 45be99f8. > > It's been a while, but I think I omitted this deliberately because I > didn't really understand the value of it and wanted to keep the >

Re: make add_paths_to_append_rel aware of startup cost

2023-09-27 Thread David Rowley
On Mon, 18 Sept 2023 at 22:55, Andy Fan wrote: > Here is an updated version to show what's in my mind. My current thoughts on this are that the fractional cost part adds quite a bit more complexity than the minimalistic approach of just also considering the cheapest startup path. There's also

Re: Avoid a possible out-of-bounds access (src/backend/optimizer/util/relnode.c)

2023-09-27 Thread David Rowley
On Wed, 27 Sept 2023 at 06:10, Ranier Vilela wrote: > As suggested, casting is the best option that does not add any overhead and > improves the robustness of the find_base_rel function. > I propose patch v1, with the additional addition of fixing the > find_base_rel_ignore_join function, >

Re: Use virtual tuple slot for Unique node

2023-09-27 Thread David Rowley
On Sat, 23 Sept 2023 at 03:15, Heikki Linnakangas wrote: > So not a win in this case. Could you peek at the outer slot type, and > use the same kind of slot for the Unique's result? Or some more > complicated logic, like use a virtual slot if all the values are > pass-by-val? I'd also like to

Re: Correct the documentation for work_mem

2023-09-26 Thread David Rowley
On Tue, 12 Sept 2023 at 03:03, Bruce Momjian wrote: > > On Mon, Sep 11, 2023 at 10:02:55PM +1200, David Rowley wrote: > > It's certainly not a show-stopper. I do believe the patch makes some > > improvements. The reason I'd prefer to see either "and" or "and/or

Re: Avoid a possible out-of-bounds access (src/backend/optimizer/util/relnode.c)

2023-09-26 Thread David Rowley
On Wed, 27 Sept 2023 at 01:31, Ranier Vilela wrote: > It seems to me that it adds a LEA instruction. > https://godbolt.org/z/b4jK3PErE There's a fairly significant difference in the optimisability of a comparison with a compile-time constant vs a variable. For example, would you expect the

Re: Avoid a possible out-of-bounds access (src/backend/optimizer/util/relnode.c)

2023-09-26 Thread David Rowley
On Tue, 26 Sept 2023 at 21:45, Ashutosh Bapat wrote: > However, I agree that changing find_base_rel() the way you have done > in your patch is fine and mildly future-proof. +1 to that idea > irrespective of what bitmapset functions do. I'm not a fan of adding additional run-time overhead for

Re: Bug fix in vacuumdb --buffer-usage-limit xxx -Z

2023-09-21 Thread David Rowley
On Thu, 21 Sept 2023 at 17:59, Michael Paquier wrote: > That was fast. If I may ask, why don't you have some regression tests > for the two code paths of vacuumdb that append this option to the > commands generated for VACUUM and ANALYZE? I think we have differing standards for what constitutes

Re: Bug fix in vacuumdb --buffer-usage-limit xxx -Z

2023-09-20 Thread David Rowley
On Thu, 21 Sept 2023 at 16:18, David Rowley wrote: > Thanks for the report and the patch. I agree this has been overlooked. > > I also wonder if we should be escaping the buffer-usage-limit string > sent in the comment. It seems quite an unlikely attack vector, as the > user wou

Re: Bug fix in vacuumdb --buffer-usage-limit xxx -Z

2023-09-20 Thread David Rowley
On Thu, 21 Sept 2023 at 13:45, Ryoga Yoshida wrote: > When --buffer-usage-limit option is specified, vacuumdb issues VACUUM or > VACUUM ANALYZE command with BUFFER_USAGE_LIMIT option. Also if > --buffer-usage-limit and -Z options are specified, vacuumdb should issue > ANALYZE command with

Re: Comment about set_join_pathlist_hook()

2023-09-20 Thread David Rowley
On Wed, 20 Sept 2023 at 22:06, Etsuro Fujita wrote: > So I would like to propose to extend the comment to explain what they > can do, as in the comment about set_rel_pathlist_hook() in allpaths.c. > Attached is a patch for that. Looks good to me. I see you've copy/edited the comment just above

Re: disfavoring unparameterized nested loops

2023-09-20 Thread David Rowley
On Wed, 20 Sept 2023 at 19:56, Andrey Lepikhov wrote: > What could you say about a different way: hybrid join? In MS SQL Server, > they have such a feature [1], and, according to their description, it > requires low overhead. They start from HashJoin and switch to NestLoop > if the inner input

Re: Speed up transaction completion faster after many relations are accessed in a transaction

2023-09-17 Thread David Rowley
On Fri, 15 Sept 2023 at 22:37, Heikki Linnakangas wrote: > > I've added a call to LockAssertNoneHeld(false) in there. > > I don't see it in the patch? hmm. I must've git format-patch before committing that part. I'll try that again... see attached. David

Re: make add_paths_to_append_rel aware of startup cost

2023-09-17 Thread David Rowley
On Mon, 18 Sept 2023 at 01:42, Andy Fan wrote: > On Fri, Sep 15, 2023 at 3:15 PM David Rowley wrote: >> Instead of doing that, why don't you just create a completely new >> AppendPath containing all the cheapest_startup_paths and add that to >> the append rel. You can opti

Re: make add_paths_to_append_rel aware of startup cost

2023-09-15 Thread David Rowley
On Thu, 7 Sept 2023 at 04:37, Andy Fan wrote: > Currently add_paths_to_append_rel overlooked the startup cost for creating > append path, so it may have lost some optimization chances. After a glance, > the following 4 identifiers can be impacted. > - We shouldn't do the optimization if there

Re: What's the eviction algorithm of newest pg version?

2023-09-14 Thread David Rowley
On Fri, 15 Sept 2023 at 00:53, jacktby jacktby wrote: > A normal LRU cache or implement it reference to a research paper? src/backend/storage/buffer/README David

Re: Surely this code in setrefs.c is wrong?

2023-09-13 Thread David Rowley
On Sun, 10 Sept 2023 at 21:07, David Rowley wrote: > > On Sun, 10 Sept 2023 at 11:22, Tom Lane wrote: > > if (!OidIsValid(saop->hashfuncid)) > > record_plan_function_dependency(root, saop->hashfuncid); > > > >

Re: Redundant Unique plan node for table with a unique index

2023-09-13 Thread David Rowley
On Thu, 14 Sept 2023 at 02:28, Damir Belyalov wrote: > create table a (n int); > insert into a (n) select x from generate_series(1, 14) as g(x); > create unique index on a (n); > explain select distinct n from a; > QUERY PLAN >

Re: Speed up transaction completion faster after many relations are accessed in a transaction

2023-09-11 Thread David Rowley
Thank you for having a look at this. Apologies for not getting back to you sooner. On Wed, 5 Jul 2023 at 21:44, Heikki Linnakangas wrote: > > On 10/02/2023 04:51, David Rowley wrote: > > I've attached another set of patches. I do need to spend longer > > looking at this. I'

Re: Correct the documentation for work_mem

2023-09-11 Thread David Rowley
On Sat, 9 Sept 2023 at 14:25, Imseih (AWS), Sami wrote: > > > This looks mostly fine to me modulo "sort or hash". I do see many > > instances of "and/or" in the docs. Maybe that would work better. > > "sort or hash operations at the same time" is clear explanation IMO. Just for anyone else

Re: Surely this code in setrefs.c is wrong?

2023-09-10 Thread David Rowley
On Sun, 10 Sept 2023 at 11:22, Tom Lane wrote: > if (!OidIsValid(saop->hashfuncid)) > record_plan_function_dependency(root, saop->hashfuncid); > > if (!OidIsValid(saop->negfuncid)) > record_plan_function_dependency(root, saop->negfuncid); > > Surely those

Re: Correct the documentation for work_mem

2023-09-07 Thread David Rowley
On Fri, 8 Sept 2023 at 15:24, Bruce Momjian wrote: > Adjusted patch attached. This looks mostly fine to me modulo "sort or hash". I do see many instances of "and/or" in the docs. Maybe that would work better. David

Re: Use virtual tuple slot for Unique node

2023-08-30 Thread David Rowley
On Thu, 31 Aug 2023 at 05:37, Денис Смирнов wrote: > I have inspected the performance of the GROUP BY and DISTINCT queries for the > sorted data streams and found out, that Group node (produced by GROUP BY) > works faster then the Unique node (produced by DISTINCT). The flame graph > should

Re: Sync scan & regression tests

2023-08-30 Thread David Rowley
On Tue, 29 Aug 2023 at 22:35, Heikki Linnakangas wrote: > Looking the new heapgettup_advance_block() function and the code that it > replaced, it's now skipping this ss_report_location() on the last call, > when it has reached the end of the scan: > > > > > /* > >* Report our new

Re: Debian 12 gcc warning

2023-08-28 Thread David Rowley
On Tue, 29 Aug 2023 at 07:37, Bruce Momjian wrote: > nargs = 0; > foreach(lc, args) > { > actual_arg_types[nargs++] = exprType((Node *) lfirst(lc)); > } Does it still produce the warning if you form the above more like? nargs =

Re: meson uses stale pg_config_paths.h left over from make

2023-08-24 Thread David Rowley
On Thu, 24 Aug 2023 at 18:25, Andres Freund wrote: > > On 2023-08-24 08:18:14 +0200, Peter Eisentraut wrote: > > Surely meson should not be required to detect that? > > I think we should try to detect included files, due to the nasty and hard to > debug issues that creates. I've spent quite a bit

Re: Support run-time partition pruning for hash join

2023-08-24 Thread David Rowley
On Thu, 24 Aug 2023 at 21:27, Richard Guo wrote: > I performed some performance comparisons of the worst case with two > tables as below: > > 1. The partitioned table has 1000 children, and 100,000 tuples in total. > > 2. The other table is designed that > a) its tuples occupy every partition

Re: Avoid a possible overflow (src/backend/utils/sort/logtape.c)

2023-08-24 Thread David Rowley
On Fri, 25 Aug 2023 at 13:19, Michael Paquier wrote: > Still that looks entirely different to me. Here we have a problem > where the number of free blocks stored may cause an overflow in the > internal routine retrieving a free block, but your other thread > is about long being not enough on

Re: meson uses stale pg_config_paths.h left over from make

2023-08-23 Thread David Rowley
On Thu, 24 Aug 2023 at 00:52, David Rowley wrote: > Are there any objections to the attached being applied? Pushed. David

Re: PostgreSQL 16 release announcement draft

2023-08-23 Thread David Rowley
On Thu, 24 Aug 2023 at 05:55, Jonathan S. Katz wrote: > We could add something about 1349d2790 -- do you have suggested wording? I think it's worth a mention. See the text added in square brackets below: PostgreSQL 16 improves the performance of existing PostgreSQL functionality through new

meson uses stale pg_config_paths.h left over from make

2023-08-23 Thread David Rowley
I've been having some problems running the regression tests using meson on Windows. This seems to be down to initdb being compiled with a version of pg_config_paths.h left over from the msvc build which had been used on that source tree previously. Generally when there are files left over the

Re: PostgreSQL 16 release announcement draft

2023-08-23 Thread David Rowley
On Wed, 23 Aug 2023 at 22:21, jian he wrote: > > >>> > PostgreSQL 16 improves the performance of existing PostgreSQL functionality > through new query planner optimizations. In this latest release, the query > planner can parallelize `FULL` and `RIGHT` joins, utilize incremental sorts > for >

Re: Support run-time partition pruning for hash join

2023-08-22 Thread David Rowley
On Tue, 22 Aug 2023 at 00:34, Andy Fan wrote: > > On Mon, Aug 21, 2023 at 11:48 AM Richard Guo wrote: >> 1. All the join partition prunning decisions are made in createplan.c >>where the best path tree has been decided. This is not great. Maybe >>it's better to make it happen when we

Re: PG 16 draft release notes ready

2023-08-21 Thread David Rowley
s://www.postgresql.org/docs/16/release-16.html#RELEASE-16-PERFORMANCE > > > > Same as here: > > https://momjian.us/pgsql_docs/release-16.html#RELEASE-16-PERFORMANCE > > > > Allow window functions to use ROWS mode internally when RANGE mode is > > specified but un

<    1   2   3   4   5   6   7   8   9   10   >