On Sat, Oct 12, 2024, at 02:48, jian he wrote: > git version 2.34.1 > cannot do `git apply`
Sorry about that, fixed.
> typedef enum CopyFormat
> {
> COPY_FORMAT_TEXT,
> COPY_FORMAT_BINARY,
> COPY_FORMAT_CSV
> } CopyFormat;
Thanks, fixed.
> CopyFormat should add to
> src/tools/pgindent/typedefs.list
Thanks, fixed.
> + /* Assert options have been set (defaults applied if not specified)
> */
> + Assert(opts_out->delim);
> + Assert(opts_out->null_print);
> +
> + /* Don't allow the delimiter to appear in the NULL or DEFAULT
> strings */
> +
> + if (strchr(opts_out->null_print, opts_out->delim[0]) != NULL)
>
> + Assert(opts_out->delim);
> + Assert(opts_out->quote);
> + Assert(opts_out->null_print);
> +
> + if (opts_out->delim[0] == opts_out->quote[0])
> these Asserts, no need? Without it, if conditions are not met, it will
> still segfault.
The asserts are only there to indicate that at this point in the code,
we can be certain the delim, quote and null_print have been set,
since the format is COPY_FORMAT_CSV, otherwise it would be a bug.
If you don't think they add any documentation value, I'm OK with removing them.
> there is no sql example, like
> copy the_table from :'filename' (format raw);
>
> in the patch.
> I thought you were going to implement something like that.
Sorry if that was unclear, yes, that's the plan, but as I wrote:
>> After having studied the code that will be affected,
>> I feel that before making any changes, I would like to try to improve
>> ProcessCopyOptions, in terms of readability and maintainability, first.
So, I just wanted to get some feedback first, if this reorganization of
ProcessCopyOptions,
would be OK to do first, which I think is needed for it to be easily
maintainable.
/Joel
v3-0001-Replace-binary-flags-binary-and-csv_mode-with-format.patch
Description: Binary data
v3-0002-Reorganize-ProcessCopyOptions-for-clarity-and-consis.patch
Description: Binary data
