pá 21. 8. 2020 v 9:44 odesílatel Peter Smith <smithpb2...@gmail.com> napsal:
> On Fri, Aug 21, 2020 at 5:21 AM Pavel Stehule <pavel.steh...@gmail.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 >
