On Tue, Dec 9, 2025 at 7:32 AM Nathan Bossart <[email protected]> wrote:
> 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 > Hi, thank you for checking my patch. I took Nathan's comment into consideration and made my patch much simpler by putting the compare logic and the update of ClosestMatch into one place. It allows us to avoid having two separate for-loops to compare the specified options and the list of the available options. Also, it minimizes the necessary amount of changes for making error hints richer. Here is my patch. Regards
v5-0001-Improve-error-hints-for-invalid-COPY-options.patch
Description: Binary data
