On 2016/12/07 5:27, Robert Haas 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.

Ashutosh proposed this to do the comparison:

On 2016/11/22 18:28, Ashutosh Bapat wrote:
I guess, the reason why you are doing it this way, is SELECT clause for the
outermost query gets deparsed before FROM clause. For later we call
deparseRangeTblRef(), which builds the tlist. So, while deparsing SELECT
clause, we do not have tlist to build from. In that case, I guess, we have to
build the tlist in get_subselect_alias_id() if it's not available and stick it
in fpinfo. Subsequent calls to get_subselect_alias_id() should find tlist
there. Then in deparseRangeTblRef() assert that there's a tlist in fpinfo
and use it to build the SELECT clause of subquery. That way, we don't build
tlist unless it's needed and also use the same tlist for all searches. Please
use tlist_member() to search into the tlist.

This would probably work, but seems to me a bit complicated. Instead, I'd like to propose that we build the tlist for each relation being deparsed as a subquery in a given join tree, right before deparsing the SELECT clause in deparseSelectStmtForRel, if is_subquery is false and lower_subquery_rels isn't NULL, and store the tlist into the relation's fpinfo. That would allow us to build the tlist only when we need it, and to use tlist_member for the exact comparison. I think it would be much easier to implement that.

Best regards,
Etsuro Fujita




--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to