Thread: Re: Some dead code in get_param_path_clause_serials()
On 11/13/24 16:34, Richard Guo wrote: > The function get_param_path_clause_serials() is used to get the set of > pushed-down clauses enforced within a parameterized Path. Since we > don't currently support parameterized MergeAppend paths, and it > doesn't look like that is going to change anytime soon (as explained > in the comments for generate_orderedappend_paths), we don't need to > consider MergeAppendPath in this function. Is it worth removing the > related code, as attached? > > This change won't make any measurable difference in performance; it's > just for clarity's sake. I've passed through the logic of get_param_path_clause_serials/reparameterize_path_by_child/reparameterize_path. Agree, it seems not useful to parameterise ordered appends in the near future. Even So, it would be better to insert changes, induced by new feature by one commit. By the way, Do you know if anyone gave a speech on the current state of internal plan parameterisation and its possible development directions? It would be helpful to have such an explanation. -- regards, Andrei Lepikhov
On Wed, Nov 13, 2024 at 9:59 PM Andrei Lepikhov <lepihov@gmail.com> wrote: > On 11/13/24 16:34, Richard Guo wrote: > > The function get_param_path_clause_serials() is used to get the set of > > pushed-down clauses enforced within a parameterized Path. Since we > > don't currently support parameterized MergeAppend paths, and it > > doesn't look like that is going to change anytime soon (as explained > > in the comments for generate_orderedappend_paths), we don't need to > > consider MergeAppendPath in this function. Is it worth removing the > > related code, as attached? > > > > This change won't make any measurable difference in performance; it's > > just for clarity's sake. > I've passed through the logic of > get_param_path_clause_serials/reparameterize_path_by_child/reparameterize_path. > Agree, it seems not useful to parameterise ordered appends in the near > future. Thanks for the review. > Even So, it would be better to insert changes, induced by new > feature by one commit. I'm not sure I understand what you mean by this sentence. > By the way, Do you know if anyone gave a speech on the current state of > internal plan parameterisation and its possible development directions? > It would be helpful to have such an explanation. Not that I'm aware of, but I found the "Parameterized Paths" section in optimizer/README to be very helpful. Thanks Richard
On 11/14/24 08:20, Richard Guo wrote: > On Wed, Nov 13, 2024 at 9:59 PM Andrei Lepikhov <lepihov@gmail.com> wrote: >> Even So, it would be better to insert changes, induced by new >> feature by one commit. > > I'm not sure I understand what you mean by this sentence. Yeah, it's my bad English. nothing special, I just wanted to say that this code can be added later. > >> By the way, Do you know if anyone gave a speech on the current state of >> internal plan parameterisation and its possible development directions? >> It would be helpful to have such an explanation. > > Not that I'm aware of, but I found the "Parameterized Paths" section > in optimizer/README to be very helpful. Although parameterisation itself quite clearly described, the concept and code, related to re-parameterisation (in presence of inheritance/partitioning) seems complicated. -- regards, Andrei Lepikhov
On Wed, Nov 13, 2024 at 9:59 PM Andrei Lepikhov <lepihov@gmail.com> wrote: > On 11/13/24 16:34, Richard Guo wrote: > > The function get_param_path_clause_serials() is used to get the set of > > pushed-down clauses enforced within a parameterized Path. Since we > > don't currently support parameterized MergeAppend paths, and it > > doesn't look like that is going to change anytime soon (as explained > > in the comments for generate_orderedappend_paths), we don't need to > > consider MergeAppendPath in this function. Is it worth removing the > > related code, as attached? > I've passed through the logic of > get_param_path_clause_serials/reparameterize_path_by_child/reparameterize_path. > Agree, it seems not useful to parameterise ordered appends in the near > future. Pushed. Thank you for review. Thanks Richard