On Thu, 14 May 2020 at 14:39, Andy Fan <zhihui.fan1213@gmail.com> wrote:
>
> On Thu, May 14, 2020 at 6:20 AM David Rowley <dgrowleyml@gmail.com> wrote:
>> Having the "onerow" flag was not how I intended it to work.
>>
> Thanks for the detailed explanation. So I think we do need to handle onerow
> specially, (It means more things than adding each column as a UniqueKey).
> but we don't need the onerow flag since we can tell it by ukey->exprs == NIL.
>
> During the developer of this feature, I added some Asserts as double checking
> for onerow and exprs. it helps me to find some special cases. like
> SELECT FROM multirows union SELECT FROM multirows; where targetlist is NIL.
> (I find the above case returns onerow as well just now). so onerow flag allows us
> check this special things with more attention. Even this is not the original intention
> but looks it is the one purpose now.
But surely that special case should just go in
populate_unionrel_uniquekeys(). If the targetlist is empty, then add a
UniqueKey with an empty set of exprs.
> However I am feeling that removing onerow flag doesn't require much of code
> changes. Almost all the special cases which are needed before are still needed
> after that and all the functions based on that like relation_is_onerow
> /add_uniquekey_onerow is still valid, we just need change the implementation.
> so do you think we need to remove onerow flag totally?
Well, at the moment I'm not quite understanding why it's needed. If
it's not needed then we should remove it. If it turns out there is
some valid reason for it, then we should keep it.
David