Re: Review: UNNEST (and other functions) WITH ORDINALITY

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Greg Stark <stark(at)mit(dot)edu>
Cc: Josh Berkus <josh(at)agliodbs(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Review: UNNEST (and other functions) WITH ORDINALITY
Date: 2013-07-22 15:42:23
Message-ID: 15861.1374507743@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Greg Stark <stark(at)mit(dot)edu> writes:
> I do find the logic and variable names a bit confusing so I'm tempted
> to add some comments based on my initial confusion. And I'm tempted to
> add an ordinalityAttNum field to the executor node so we don't need to
> make these odd "scanslot means this unless we have ordinality in which
> case it means that and funcslot means this" logic.

I haven't read this patch, but that comment scares the heck out of me.
Even if the patch isn't broken today, it will be tomorrow, if it's
making random changes like that in data structure semantics.
Also, if you're confused, so will be everybody else who has to deal with
the code later --- so I don't think fixing the comments and variable
names is optional.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Natalie Wenz 2013-07-22 15:44:27 Re: Insert result does not match record count
Previous Message Greg Stark 2013-07-22 15:32:35 Re: Review: UNNEST (and other functions) WITH ORDINALITY