Re: Optimizing nested ConvertRowtypeExpr execution

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>
Cc: ashutosh(dot)bapat(at)enterprisedb(dot)com, 9erthalion6(at)gmail(dot)com, Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, pavel(dot)stehule(at)gmail(dot)com, andres(at)anarazel(dot)de, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Optimizing nested ConvertRowtypeExpr execution
Date: 2018-11-06 19:00:14
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk> writes:
> Here's the patch with my edits (more comments and the while/if change).

A couple minor thoughts:

* I dislike using castNode() in places where the code has just explicitly
verified the node type, which is true in both places where you used it
here. The assertion is just bulking the code up to no purpose, and it
creates an unnecessary discrepancy between older and newer code.

* As you have it here, a construct such as
will laboriously perform each of the intermediate steps, which seems
like exactly the case we're trying to prevent at runtime. I wonder
whether it is worth stripping off ConvertRowtypeExpr's before the
recursive eval_const_expressions_mutator call to prevent that.
You'd still want the check after the call, to handle situations where
something more complex got simplified to a ConvertRowtypeExpr; and this
would also complicate getting the convertformat right. So perhaps it's
not worth the trouble, but I thought I'd mention it.

* I find the hardwired logic about how to merge distinct convertformat
values a bit troublesome. Maybe use Min() instead? Although there
is not currently any expectation about the ordering of that enum ...

regards, tom lane

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Simon Riggs 2018-11-06 19:01:09 Re: ATTACH/DETACH PARTITION CONCURRENTLY
Previous Message Robert Haas 2018-11-06 18:56:37 Re: ATTACH/DETACH PARTITION CONCURRENTLY