Re: POC PATCH: copy from ... exceptions to: (was Re: VLDB Features)
От | torikoshia |
---|---|
Тема | Re: POC PATCH: copy from ... exceptions to: (was Re: VLDB Features) |
Дата | |
Msg-id | 80131d3e817a60d736f628db33a5f65b@oss.nttdata.com обсуждение исходный текст |
Ответ на | Re: POC PATCH: copy from ... exceptions to: (was Re: VLDB Features) (Masahiko Sawada <sawada.mshk@gmail.com>) |
Ответы |
Re: POC PATCH: copy from ... exceptions to: (was Re: VLDB Features)
(Masahiko Sawada <sawada.mshk@gmail.com>)
|
Список | pgsql-hackers |
On 2024-04-16 13:16, Masahiko Sawada wrote: > On Tue, Apr 2, 2024 at 7:34 PM torikoshia <torikoshia@oss.nttdata.com> > wrote: >> >> On 2024-04-01 11:31, Masahiko Sawada wrote: >> > On Fri, Mar 29, 2024 at 11:54 AM torikoshia >> > <torikoshia@oss.nttdata.com> wrote: >> >> >> >> On 2024-03-28 21:54, Masahiko Sawada wrote: >> >> > On Thu, Mar 28, 2024 at 9:38 PM torikoshia <torikoshia@oss.nttdata.com> >> >> > wrote: >> >> >> >> >> >> On 2024-03-28 10:20, Masahiko Sawada wrote: >> >> >> > Hi, >> >> >> > >> >> >> > On Thu, Jan 18, 2024 at 5:33 PM Masahiko Sawada <sawada.mshk@gmail.com> >> >> >> > wrote: >> >> >> >> >> >> >> >> On Thu, Jan 18, 2024 at 4:59 PM Alexander Korotkov >> >> >> >> <aekorotkov@gmail.com> wrote: >> >> >> >> > >> >> >> >> > On Thu, Jan 18, 2024 at 4:16 AM torikoshia <torikoshia@oss.nttdata.com> wrote: >> >> >> >> > > On 2024-01-18 10:10, jian he wrote: >> >> >> >> > > > On Thu, Jan 18, 2024 at 8:57 AM Masahiko Sawada <sawada.mshk@gmail.com> >> >> >> >> > > > wrote: >> >> >> >> > > >> On Thu, Jan 18, 2024 at 6:38 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: >> >> >> >> > > >> > Kyotaro-san's suggestion isn't bad, though I might shorten it to >> >> >> >> > > >> > error_action {error|ignore|log} (or perhaps "stop" instead of "error")? >> >> >> >> > > >> > You will need a separate parameter anyway to specify the destination >> >> >> >> > > >> > of "log", unless "none" became an illegal table name when I wasn't >> >> >> >> > > >> > looking. I don't buy that one parameter that has some special values >> >> >> >> > > >> > while other values could be names will be a good design. Moreover, >> >> >> >> > > >> > what if we want to support (say) log-to-file along with log-to-table? >> >> >> >> > > >> > Trying to distinguish a file name from a table name without any other >> >> >> >> > > >> > context seems impossible. >> >> >> >> > > >> >> >> >> >> > > >> I've been thinking we can add more values to this option to log errors >> >> >> >> > > >> not only to the server logs but also to the error table (not sure >> >> >> >> > > >> details but I imagined an error table is created for each table on >> >> >> >> > > >> error), without an additional option for the destination name. The >> >> >> >> > > >> values would be like error_action {error|ignore|save-logs|save-table}. >> >> >> >> > > >> >> >> >> >> > > > >> >> >> >> > > > another idea: >> >> >> >> > > > on_error {error|ignore|other_future_option} >> >> >> >> > > > if not specified then by default ERROR. >> >> >> >> > > > You can also specify ERROR or IGNORE for now. >> >> >> >> > > > >> >> >> >> > > > I agree, the parameter "error_action" is better than "location". >> >> >> >> > > >> >> >> >> > > I'm not sure whether error_action or on_error is better, but either way >> >> >> >> > > "error_action error" and "on_error error" seems a bit odd to me. >> >> >> >> > > I feel "stop" is better for both cases as Tom suggested. >> >> >> >> > >> >> >> >> > OK. What about this? >> >> >> >> > on_error {stop|ignore|other_future_option} >> >> >> >> > where other_future_option might be compound like "file 'copy.log'" or >> >> >> >> > "table 'copy_log'". >> >> >> >> >> >> >> >> +1 >> >> >> >> >> >> >> > >> >> >> > I realized that ON_ERROR syntax synoposis in the documentation is not >> >> >> > correct. The option doesn't require the value to be quoted and the >> >> >> > value can be omitted. The attached patch fixes it. >> >> >> > >> >> >> > Regards, >> >> >> >> >> >> Thanks! >> >> >> >> >> >> Attached patch fixes the doc, but I'm wondering perhaps it might be >> >> >> better to modify the codes to prohibit abbreviation of the value. >> >> >> >> >> >> When seeing the query which abbreviates ON_ERROR value, I feel it's >> >> >> not >> >> >> obvious what happens compared to other options which tolerates >> >> >> abbreviation of the value such as FREEZE or HEADER. >> >> >> >> >> >> COPY t1 FROM stdin WITH (ON_ERROR); >> >> >> >> >> >> What do you think? >> >> > >> >> > Indeed. Looking at options of other commands such as VACUUM and >> >> > EXPLAIN, I can see that we can omit a boolean value, but non-boolean >> >> > parameters require its value. The HEADER option is not a pure boolean >> >> > parameter but we can omit the value. It seems to be for backward >> >> > compatibility; it used to be a boolean parameter. I agree that the >> >> > above example would confuse users. >> >> > >> >> > Regards, >> >> >> >> Thanks for your comment! >> >> >> >> Attached a patch which modifies the code to prohibit omission of its >> >> value. >> >> >> >> I was a little unsure about adding a regression test for this, but I >> >> have not added it since other COPY option doesn't test the omission of >> >> its value. >> > >> > Probably should we change the doc as well since ON_ERROR value doesn't >> > necessarily need to be single-quoted? >> >> Agreed. >> Since it seems this issue is independent from the omission of ON_ERROR >> option value, attached a separate patch. >> > > Thank you for the patches! These patches look good to me. I'll push > them, barring any objections. > > Regards, Thanks for your review and apply! -- Regards, -- Atsushi Torikoshi NTT DATA Group Corporation
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Marcel HofstetterДата:
Сообщение: Re: Solaris tar issues, or other reason why margay fails 010_pg_basebackup?
Следующее
От: Masahiko SawadaДата:
Сообщение: Re: POC PATCH: copy from ... exceptions to: (was Re: VLDB Features)