On Wed, Mar 9, 2016 at 3:30 AM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
> Great! I changed the naming. I also updated docs as proposed by you in a
> previous email, and rebased the patch to the latest HEAD. Please find
> attached an updated version of the patch.
Thanks. The new naming looks much better (and better also than what I
suggested).
I see that you went and changed all of the places that tested for !=
CMD_SELECT and made them test for == CMD_INSERT || == CMD_UPDATE || ==
CMD_DELETE instead. I think that's the wrong direction. I think that
we should use the != CMD_SELECT version of the test everywhere.
That's a single test instead of three, so marginally faster, and maybe
marginally more future-proof.
I think deparsePushedDownUpdateSql should be renamed to use the new
"direct modify" naming, like deparseDirectUpdateSql, maybe.
I would suggest not numbering the tests in postgresPlanDirectModify.
That just becomes a nuisance to keep up to date as things change.
Overall, I think this is looking pretty good.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company