> 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/
