On Mon, Jun 13, 2016 at 5:51 PM, Robert Haas <robertmhaas@gmail.com> wrote: > On Fri, Jun 10, 2016 at 4:14 PM, Robert Haas <robertmhaas@gmail.com> wrote: >> Although I have done a bit of review of this patch, it needs more >> thought than I have so far had time to give it. I will update again >> by Tuesday. > > I've reviewed this a bit further and have discovered an infelicity.
Also, independent of the fix for this particular issue, I think it would be smart to apply the attached patch to promote the assertion that failed here to an elog(). If we have more bugs of this sort, now or in the future, I'd like to catch them even in non-assert-enabled builds by getting a sensible error rather than just by failing an assertion. I think it's our general practice to check node types with elog() rather than Assert() when the nodes are coming from some far-distant part of the code, which is certainly the case here.
I plan to commit this without delay unless there are vigorous, well-reasoned objections.
Fine with me. Serves the purpose for which I added the Assert, but in a better manner. May be the error message can read "non-Var nodes/targets/expressions not expected in target list". I am not sure what do we call individual (whole) members of target list.
--
Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
