A semantics-preserving conversion would have looked something like
if (node->sortgrouprefs) WRITE_INDEX_ARRAY(sortgrouprefs, list_length(node->exprs));
I suppose that Peter was trying to remove special cases from the outfuncs.c code, but do we want to put this one back? Richard's proposal would not accurately reflect the contents of the data structure, so I'm not too thrilled with it.
The commit message in bdeb2c4ec mentions that:
" This also changes the behavior slightly: Before, the field name was skipped if the length was zero. Now it prints the field name even in that case. This is more consistent with how other array fields are handled. "
So I suppose we are trying to print the field name even if the length is zero. Should we keep this behavior in the fix?