Re: Add support for printing/reading MergeAction nodes

On 5 April 2018 at 11:31, Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com> wrote:

> Attached patch refactors the grammar/parser side per your comments. We no
> longer use InsertStmt/UpdateStmt/DeleteStmt/SelectStmt as part of
> MergeAction. Instead we only collect the necessary information for running
> the INSERT/UPDATE/DELETE actions. Speaking of MergeAction itself, I decided
> to use a new parser-only node named MergeWhenClause and removed unnecessary
> members from the MergeAction node which now gets to planner/executor.

That looks good to me. Simply separation of duty.

> Regarding the original report by Marina I suspect she may have turned
> debug_print_parse=on while running regression. I could reproduce the
> failures in the isolation tests by doing same. The attached patch however
> passes all tests with the following additional GUCs. So I am more confident
> that we should have got the outfuncs.c support ok.
>
> debug_print_parse=on
> debug_print_rewritten=on
> debug_print_plan=on
>
> Also, I am now running tests with -DCOPY_PARSE_PLAN_TREES
> -DRAW_EXPRESSION_COVERAGE_TEST since the buildfarm had earlier uncovered
> some issues with those flags. No problem there too.

OK, so $OP fixed.

> This now also enforces single VALUES clause in the grammar itself instead of
> doing that check at parse-analyse time. So that's a net improvement too.

OK, that's good. I've updated the docs to show this restriction correctly.

I'll commit this tomorrow morning unless further comments or edits.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services