| From: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
|---|---|
| To: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
| Cc: | Sugamoto Shinya <shinya34892(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-24 21:50:26 |
| Message-ID: | aSTTIpFKC2Qg6GgV@nathan |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Joel Jacobson | 2025-11-24 21:53:40 | Re: [PATCH] Avoid pallocs in async.c's SignalBackends critical section |
| Previous Message | Michael Paquier | 2025-11-24 21:43:43 | Re: BUG #19095: Test if function exit() is used fail when linked static |