Re: New "raw" COPY format

On Sat, Oct 19, 2024 at 11:33 PM Joel Jacobson <joel(at)compiler(dot)org> wrote:
>
> > ProcessCopyOptions
> > /* Extract options from the statement node tree */
> > foreach(option, options)
> > {
> > }
> > /* --- DELIMITER option --- */
> > /* --- NULL option --- */
> > /* --- QUOTE option --- */
> > Currently the regress test passed, i think that means your refactor is fine.
>
> I believe that a passing test indicates it might be okay,
> but a failing test definitely means it's not. :D
>
> I've meticulously refactored one option at a time, checking which code in
> ProcessCopyOptions depends on each option field to ensure the semantics
> are preserved.
>
> I think the changes are easy to follow, and it's clear that each change is
> correct when looking at them individually, though it might be more challenging
> when viewing the total change.
>
> I've tried to minimize code movement, preserving as much of the original
> code placement as possible.
>
> > in ProcessCopyOptions, maybe we can rearrange the code after the
> > foreach loop (foreach(option, options)
> > based on the parameters order in
> > https://www.postgresql.org/docs/devel/sql-copy.html Parameters section.
> > so we can review it by comparing the refactoring with the
> > sql-copy.html Parameters section's description.
>
> That would be nice, but unfortunately, it's not possible because the order of
> the option code blocks matters due to the setting of defaults in else/else
> if branches when an option is not specified.
>
> For example, in the documentation, DEFAULT precedes QUOTE,
> but in ProcessCopyOptions, the QUOTE code block must come before
> the DEFAULT code block due to the check:
>
> /* Don't allow the CSV quote char to appear in the default string. */
>

> I also believe there's value in minimizing code movement.
>
but v12-0001 was already hugely refactored.

make the ProcessCopyOptions process in following order:
1. Extract options from the statement node tree
2. checking each option, if not there set default value.
3. checking for interdependent options

I still think
making step2 aligned with the doc parameter section order will make it
more readable.

based on your patch
(v12-0001-Refactor-ProcessCopyOptions-introduce-CopyFormat-enu.patch)
I put some checking to step3, make step2 checking order aligned with doc.