public inbox for [email protected]  

help / color / mirror / Atom feed

From: David Rowley <[email protected]>
To: Andy Fan <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: Zhang Mingli <[email protected]>
Subject: Re: make add_paths_to_append_rel aware of startup cost
Date: Fri, 15 Sep 2023 19:15:24 +1200
Message-ID: <CAApHDvry0nSV62kAOH3iccvfPhGPLN0Q97+=b1RsDPXDz3=CiQ@mail.gmail.com> (raw)
In-Reply-To: <CAKU4AWrXSkUV=Pt-gRxQT7EbfUeNssprGyNsB=5mJibFZ6S3ww@mail.gmail.com>
References: <CAKU4AWrXSkUV=Pt-gRxQT7EbfUeNssprGyNsB=5mJibFZ6S3ww@mail.gmail.com>

On Thu, 7 Sept 2023 at 04:37, Andy Fan <[email protected]> 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 are still more tables to join,
>   the reason is similar to has_multiple_baserels(root) in
>   set_subquery_pathlist. But the trouble here is we may inherit multiple
>   levels to build an appendrel, so I have to keep the 'top_relids' all the
>   time and compare it with PlannerInfo.all_baserels. If they are the same,
>   then it is the case we want to optimize.

I think you've likely gone to the trouble of trying to determine if
there are any joins pending because you're considering using a cheap
startup path *instead* of the cheapest total path and you don't want
to do that when some join will cause all the rows to be read thus
making the plan more expensive if a cheap startup path was picked.

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 optimise this and only do it when
rel->consider_startup is true.

Does the attached do anything less than what your patch does?

David


Attachments:

  [application/octet-stream] consider_cheapest_startup_appendpath.patch (1.7K, ../CAApHDvry0nSV62kAOH3iccvfPhGPLN0Q97+=b1RsDPXDz3=CiQ@mail.gmail.com/2-consider_cheapest_startup_appendpath.patch)
  download | inline diff:
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 9bdc70c702..b50f1d58a7 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -1307,6 +1307,8 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo *rel,
 {
 	List	   *subpaths = NIL;
 	bool		subpaths_valid = true;
+	List	   *startup_subpaths = NIL;
+	bool		startup_subpaths_valid = true;
 	List	   *partial_subpaths = NIL;
 	List	   *pa_partial_subpaths = NIL;
 	List	   *pa_nonpartial_subpaths = NIL;
@@ -1346,6 +1348,20 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo *rel,
 		else
 			subpaths_valid = false;
 
+		/*
+		 * When the planner is considering cheap startup plans, we'll also
+		 * collect all the cheapest_startup_paths and build an AppendPath
+		 * containing those as subpaths.
+		 */
+		if (rel->consider_startup && childrel->pathlist != NIL &&
+			childrel->cheapest_startup_path->param_info == NULL)
+			accumulate_append_subpath(childrel->cheapest_startup_path,
+									  &startup_subpaths,
+									  NULL);
+		else
+			startup_subpaths_valid = false;
+
+
 		/* Same idea, but for a partial plan. */
 		if (childrel->partial_pathlist != NIL)
 		{
@@ -1478,6 +1494,11 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo *rel,
 												  NIL, NULL, 0, false,
 												  -1));
 
+	/* build an AppendPath for the cheap startup paths, if valid */
+	if (startup_subpaths_valid)
+		add_path(rel, (Path *) create_append_path(root, rel, startup_subpaths,
+												  NIL, NIL, NULL, 0, false, -1));
+
 	/*
 	 * Consider an append of unordered, unparameterized partial paths.  Make
 	 * it parallel-aware if possible.

view thread (18+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: make add_paths_to_append_rel aware of startup cost
  In-Reply-To: <CAApHDvry0nSV62kAOH3iccvfPhGPLN0Q97+=b1RsDPXDz3=CiQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox