Re: proposal - function string_to_table

pá 21. 8. 2020 v 9:44 odesílatel Peter Smith <smithpb2250(at)gmail(dot)com> napsal:

> On Fri, Aug 21, 2020 at 5:21 AM Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
>
> > new patch attached
>
> Thanks for taking some of my previous review comments.
>
> I have re-checked the string_to_table_20200820.patch.
>
> Below are some remaining questions/comments:
>
> ====
>
> COMMENT (help text)
>
> + Splits the <parameter>string</parameter> at occurrences
> + of <parameter>delimiter</parameter> and forms the remaining data
> + into a <type>text</type> tavke.
>
> What did you mean by "remaining" in that description?
> It gets a bit strange thinking about remaining NULLs, or remaining
> empty strings.
>
> Why not just say "... and forms the data into a <type>text</type> table."
>
> ---
>
> + Splits the <parameter>string</parameter> at occurrences
> + of <parameter>delimiter</parameter> and forms the remaining data
> + into a <type>text</type> tavke.
>
> Typo: "tavke." -> "table."
>

This text is taken from doc for string_to_array

> ====
>
> COMMENT (help text reference to regexp_split_to_table)
>
> + input <parameter>string</parameter> can be done by function
> + <function>regexp_split_to_table</function> (see <xref
> linkend="functions-posix-regexp"/>).
> + </para>
>
> In the previous review I suggested adding a reference to the
> regexp_split_to_table function.
> A hyperlink would be a bonus, but maybe it is not possible.
>
> The hyperlink added in the latest patch is to page for POSIX Regular
> Expressions, which doesn't seem appropriate.
>

ok I remove it

>
> ====
>
> QUESTION (test cases)
>
> Thanks for merging lots of my additional test cases!
>
> Actually, the previous PDF I sent was 2 pages long but you only merged
> the tests of page 1.
> I wondered was it accidental to omit all those 2nd page tests?
>

I'll check it

>
> ====
>
> QUESTION (function name?)
>
> I noticed that ALL current string functions that use delimiters have
> the word "split" in their name.
>
> e.g.
> * regexp_split_to_array
> * regexp_split_to_table
> * split_part
>
> But "string_to_table" is not following this pattern.
>
> Maybe a different choice of function name would be more consistent
> with what is already there?
> e.g. split_to_table, string_split_to_table, etc.
>

I don't agree. This function is twin (with almost identical behaviour) for
"string_to_array" function, so I think so the name is correct.

> ====
>
> Kind Regards,
> Peter Smith.
> Fujitsu Australia
>