On Thu, Aug 18, 2022 at 1:14 AM Tom Lane wrote:
>
> I wrote:
> > I'd drop the parens in these particular examples because they are
> > inconsistent with the other parts of the same "if" condition.
>
> After going through the patch I removed most but not all of the
> newly-added parens on those gro
I wrote:
> I'd drop the parens in these particular examples because they are
> inconsistent with the other parts of the same "if" condition.
After going through the patch I removed most but not all of the
newly-added parens on those grounds. I also found a couple more
spots that could be converte
Junwang Zhao writes:
> There are some places that add extra parenthesis like here
> while (list_length(sortclause) > list_length(previous) &&
> -list_length(new_elems) > 0)
> +(new_elems != NIL))
> Is it necessary to add that extra parenthesis?
I'd drop the parens in these part
> On 17 Aug 2022, at 10:13, Junwang Zhao wrote:
>
> There are some places that add extra parenthesis like here
>
> ...
>
> Is it necessary to add that extra parenthesis?
It's not necessary unless needed for operator associativity, but also I don't
object to grouping with parenthesis as a visua
There are some places that add extra parenthesis like here
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -3097,7 +3097,7 @@ reorder_grouping_sets(List *groupingsets, List
*sortclause)
GroupingSetData *gs = makeNode(GroupingSetData);
while (list_lengt
> On 17 Aug 2022, at 03:09, Peter Smith wrote:
>
> On Wed, Aug 17, 2022 at 6:34 AM Daniel Gustafsson wrote:
>>
>>> On 16 Aug 2022, at 16:03, Tom Lane wrote:
>>
>>> I agree though that while simplifying list_length() calls, I'd lean to using
>>> explicit comparisons to NIL.
>>
>>
>> Agreed,
On Wed, Aug 17, 2022 at 6:34 AM Daniel Gustafsson wrote:
>
> > On 16 Aug 2022, at 16:03, Tom Lane wrote:
>
> > I agree though that while simplifying list_length() calls, I'd lean to using
> > explicit comparisons to NIL.
>
>
> Agreed, I prefer that too.
>
Thanks for the feedback.
PSA patch v3 w
> On 16 Aug 2022, at 16:03, Tom Lane wrote:
> I agree though that while simplifying list_length() calls, I'd lean to using
> explicit comparisons to NIL.
Agreed, I prefer that too.
--
Daniel Gustafsson https://vmware.com/
Robert Haas writes:
> On Mon, Aug 15, 2022 at 9:28 PM Tom Lane wrote:
>> That's because the *correct* way to write it is either "alist == NIL"
>> or just "!alist".
> I think the alist == NIL (or alist != NIL) style often makes the code
> easier to read. I recommend we standardize on that one.
I
On Mon, Aug 15, 2022 at 9:28 PM Tom Lane wrote:
> Peter Smith writes:
> > During a recent code review I was going to suggest that some new code
> > would be more readable if the following:
> > if (list_length(alist) == 0) ...
>
> > was replaced with:
> > if (list_is_empty(alist)) ...
>
> > but th
Daniel Gustafsson writes:
> I think these are nice cleanups to simplify and streamline the code, just a
> few
> small comments from reading the patch:
> /* If no subcommands, don't collect */
> - if
> (list_length(currentEventTriggerState->currentCommand->d.alterTable.subcmds)
> != 0
> On 16 Aug 2022, at 07:29, Peter Smith wrote:
> On Tue, Aug 16, 2022 at 11:27 AM Tom Lane wrote:
>> if you want to get rid of overcomplicated uses of
>> list_length() in favor of one of those spellings, have at it.
>
> Done, and tested OK with make check-world.
I think these are nice cleanups
On Tue, Aug 16, 2022 at 11:27 AM Tom Lane wrote:
>
> Peter Smith writes:
> > During a recent code review I was going to suggest that some new code
> > would be more readable if the following:
> > if (list_length(alist) == 0) ...
>
> > was replaced with:
> > if (list_is_empty(alist)) ...
>
> > but
On Tue, Aug 16, 2022 at 11:27 AM Tom Lane wrote:
>
> Peter Smith writes:
> > During a recent code review I was going to suggest that some new code
> > would be more readable if the following:
> > if (list_length(alist) == 0) ...
>
> > was replaced with:
> > if (list_is_empty(alist)) ...
>
> > but
Peter Smith writes:
> During a recent code review I was going to suggest that some new code
> would be more readable if the following:
> if (list_length(alist) == 0) ...
> was replaced with:
> if (list_is_empty(alist)) ...
> but then I found that actually no such function exists.
That's because
15 matches
Mail list logo
