Re: Propose a new function - list_is_empty

> On 16 Aug 2022, at 07:29, Peter Smith <smithpb2250@gmail.com> wrote:
> On Tue, Aug 16, 2022 at 11:27 AM Tom Lane <tgl@sss.pgh.pa.us> 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 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)
+    if (currentEventTriggerState->currentCommand->d.alterTable.subcmds)
Here the current coding gives context about the data structure used for the
subcmds member which is now lost.  I don't mind the change but rewording the
comment above to indicate that subcmds is a list would be good IMHO.


-    build_expressions = (list_length(stxexprs) > 0);
+    build_expressions = stxexprs != NIL;
Might be personal taste, but I think the parenthesis should be kept here as a
visual aid for the reader.


-    Assert(list_length(publications) > 0);
+    Assert(publications);
The more common (and clearer IMO) pattern would be Assert(publications != NIL);
I think.  The same applies for a few hunks in the patch.


-    Assert(clauses != NIL);
-    Assert(list_length(clauses) >= 1);
+    Assert(clauses);
Just removing the list_length() assertion would be enough here.


makeIndexArray() in jsonpath_gram.y has another Assert(list_length(list) > 0);
construction as well.  The other I found is in create_groupingsets_path() but
there I think it makes sense to keep the current coding based on the assertion
just prior to it being very similar and requiring list_length().

--
Daniel Gustafsson        https://vmware.com/