Re: [PATCH] Add error hints for invalid COPY options

From: Nathan Bossart <nathandbossart(at)gmail(dot)com>
To: Sugamoto Shinya <shinya34892(at)gmail(dot)com>
Cc: Kirill Reshke <reshkekirill(at)gmail(dot)com>, 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-12-08 22:32:26
Message-ID: aTdR-n6mCB2nTYQa@nathan
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sun, Dec 07, 2025 at 11:32:28PM +0900, Sugamoto Shinya wrote:
> Attached is the new patch. One possible discussion point is that I choose
> FATAL over ERROR at src/backend/commands/copy.c#L816, since reaching that
> point would indicate an implementation problem. Please let me know if
> there is a better option.

Since it indicates a coding error, I would probably choose something like

Assert(false); /* should never happen */

That seems to be a standard way to handle these situations.

It's a little unfortunate that we are essentially validating the option
twice, but the way this is structured should at least prevent folks from
forgetting to add it in one place or another. One way to make this a
little better could be to add an enum that validate_copy_option() returns
(so that we don't have to repeat the strcmp()s). Or we could replace each
strcmp() in ProcessCopyOptions()'s option extraction block with a function
that does the strcmp() and also calls updateClosestMatch(). Then, by the
time we reach the final "else", we've already done the work for the hint.

--
nathan

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Noboru Saito 2025-12-08 22:33:17 [PATCH] Fix inconsistent title attribute tags in documentation
Previous Message Dave Cramer 2025-12-08 22:07:50 Re: Proposal to allow setting cursor options on Portals