On Wed, Dec 7, 2016 at 1:57 AM, Robert Haas <robertmh...@gmail.com> wrote: > On Mon, Dec 5, 2016 at 6:20 AM, Ashutosh Bapat > <ashutosh.ba...@enterprisedb.com> wrote: >> 4. I am still not happy with this change >> + /* >> + * Since (1) the expressions in foreignrel's reltarget doesn't >> contain >> + * any PHVs and (2) foreignrel's local_conds is empty, the tlist >> + * created by build_tlist_to_deparse must be one-to-one with the >> + * expressions. >> + */ >> + Assert(list_length(tlist) == >> list_length(foreignrel->reltarget->exprs)); >> the assertion only checks that the number of elements in both the lists are >> same but does not check whether those lists are same i.e. they contain the >> same >> elements in the same order. This equality is crucial to deparsing logic. If >> somehow build_tlist_to_deparse() breaks that assumption in future, we have no >> way to detect it, unless a regression test fails. > > If there's an easy way to do a more exact comparison, great. But if > we can't get an awesome Assert(), a helpful Assert() is still better > than a kick in the head.
The assert is not a problem in itself, but the reason we have to add the assert. The problem is explained in [1], point #9. [1] https://www.postgresql.org/message-id/CAFjFpRfwoSsJr9418b2jA7g0nwagjZSWhPQPUmK9M6z5XSOAqQ%40mail.gmail.com -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
