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