| From: | Sugamoto Shinya <shinya34892(at)gmail(dot)com> |
|---|---|
| To: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
| Cc: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: [PATCH] Add error hints for invalid COPY options |
| Date: | 2025-11-26 08:55:09 |
| Message-ID: | CAAe3y+85VpE860m+T0m2LzKQWnZ_r6FzO1_1ZNSixYP5F24ahg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
2025年11月25日(火) 6:50 Nathan Bossart <nathandbossart(at)gmail(dot)com>:
> On Mon, Nov 24, 2025 at 11:56:34AM -0800, Masahiko Sawada wrote:
> > On Sat, Nov 22, 2025 at 8:33 PM Sugamoto Shinya <shinya34892(at)gmail(dot)com>
> wrote:
> >> This follows the pattern already used elsewhere in PostgreSQL for
> providing
> >> helpful error hints to users.
> >
> > Given we have 15 COPY options now, it sounds like a reasonable idea.
> >
> > One concern about the patch is that when adding a new COPY option, we
> > could miss updating valid_copy_options list, resulting in providing a
> > wrong suggestion. I think we can consider refactoring the COPY option
> > handling so that we check the given option is a valid name or not by
> > checking valid_copy_options array and then process the option value.
>
> +1. Ideally, folks wouldn't need to update a separate list when adding new
> options.
>
> >> Additionally, this patch corrects a misleading comment for the
> >> convert_selectively option. The comment stated it was
> "not-accessible-from-SQL",
> >> but actualy it has been accessible from SQL due to PostgreSQL's generic
> option parser.
> >> The updated comment clarifies that while technically accessible, it's
> intended for
> >> internal use and not recommended for end-user use due to potential data
> loss.
> >
> > Hmm, I'm not sure the proposed comment improves the clarification.
> > It's essentially non-accessible from SQL since we cannot provide a
> > valid value for convert_selectively from SQL commands.
>
> Yeah, I'd leave it alone, at least for this patch.
>
> --
> nathan
>
Thanks for checking my proposal.
For the refactoring of the COPY options, it sounds reasonable to me. Let me
take that changes in my patch.
Regarding the option “convert_selectively”, at first i thought it should be
blocked to be used like in the lexer and parser level but later I succeeded
in specifying and using it in the SQL interface, psql , in my local
environment, and also these accept the grammar for the option. You can
verify that it’s actually accessible in any SQL interface using the
following SQLs. Please let me know if there might be something I missed.
Also I would be happy to make another thread about this matter
‘’’sql
CREATE TABLE conv_test (
a int,
b int,
c text
);
COPY conv_test FROM STDIN (
FORMAT csv,
convert_selectively (a, b)
);
— for stdin
1,2,foo
3,4,bar
—
SELECT * FROM conv_test;
— result is
a | b | c
---+---+------
1 | 2 | NULL
3 | 4 | NULL
(2 rows)
—
‘’’
Regards,
>
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Dewei Dai | 2025-11-26 09:14:52 | Re: Re: Serverside SNI support in libpq |
| Previous Message | Zhijie Hou (Fujitsu) | 2025-11-26 08:53:34 | RE: How can end users know the cause of LR slot sync delays? |