Horton

Thread

Commits

  1. Remove dead code in get_param_path_clause_serials()

  1. Some dead code in get_param_path_clause_serials()

    Richard Guo <guofenglinux@gmail.com> — 2024-11-13T09:34:36Z

    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.

Thanks
Richard

  2. Re: Some dead code in get_param_path_clause_serials()

    Andrei Lepikhov <lepihov@gmail.com> — 2024-11-13T12:59:04Z

    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

  3. Re: Some dead code in get_param_path_clause_serials()

    Richard Guo <guofenglinux@gmail.com> — 2024-11-14T01:20:14Z

    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

  4. Re: Some dead code in get_param_path_clause_serials()

    Andrei Lepikhov <lepihov@gmail.com> — 2024-11-14T09:19:46Z

    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

  5. Re: Some dead code in get_param_path_clause_serials()

    Richard Guo <guofenglinux@gmail.com> — 2024-11-26T00:48:58Z

    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