> On 18 Jan 2024, at 05:49, Kyotaro Horiguchi <horikyota....@gmail.com> wrote: > > Thank you for upicking this up. > > At Wed, 17 Jan 2024 23:47:41 +0100, Daniel Gustafsson <dan...@yesql.se> wrote > in >>> On 17 Jan 2024, at 21:33, Tom Lane <t...@sss.pgh.pa.us> wrote: >>> >>> I wrote: >>>> However ... I don't like the patch much. It seems to have left >>>> the code in a rather random state. Why, for example, didn't you >>>> keep all the code that constructs the "newline" value together? >>> >>> After thinking about it a bit more, I don't see why you didn't just >>> s/strncmp/strncasecmp/ and call it good. The messiness seems to be >>> a result of your choice to replace the GUC's case as shown in the >>> file with the case used on the command line, which is not better >>> IMO. We don't change our mind about the canonical spelling of a >>> GUC because somebody varied the case in a SET command. >> >> The original patch was preserving the case of the file throwing away the case >> from the commandline. The attached is a slimmed down version which only >> moves >> the assembly of newline to the different cases (replace vs. new) keeping the >> rest of the code intact. Keeping it in one place gets sort of messy too >> since >> it needs to use different values for a replacement and a new variable. Not >> sure if there is a cleaner approach? > > Just to clarify, the current code breaks out after processing the > first matching line. I haven't changed that behavior.
Yup. > The reason I > moved the rewrite processing code out of the loop was I wanted to > avoid adding more lines that are executed only once into the > loop. However, it is in exchange of additional complexity to remember > the original spelling of the parameter name. Personally, I believe > separating the search and rewrite processing is better, but I'm not > particularly attached to the approach, so either way is fine with me. I'll give some more time for opinions, then I'll go ahead with one of the patches with a backpatch to v16. -- Daniel Gustafsson
