On Thu, 27 Nov 2025 at 21:40, Tom Lane <[email protected]> wrote:
>
> Yeah, this looks pretty good.  Two nitpicky suggestions:
>
> * Perhaps using foreach_current_index() would be better than
> adding the separate loop variable "i" in RewriteQuery.

Ah, good point. I'd forgotten about that macro.

> * I think it would be wise to add a comment about this in
> rewriteRuleAction too, perhaps along the lines of
>
> -       /* OK, it's safe to combine the CTE lists */
> +       /*
> +        * OK, it's safe to combine the CTE lists.  Beware that RewriteQuery
> +        * knows we concatenate the lists in this order.
> +        */
>         sub_action->cteList = list_concat(sub_action->cteList,
>                                           copyObject(parsetree->cteList));

Agreed. Done that way.

Thanks for reviewing.

Regards,
Dean


Reply via email to