On Wed, Mar 9, 2016 at 3:30 AM, Etsuro Fujita <fujita.ets...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
