Re: DecodeInterval fixes

2023-08-27 Thread Michael Paquier
On Sun, Aug 27, 2023 at 04:14:00PM -0400, Joseph Koshakow wrote: > On Tue, Aug 22, 2023 at 12:58 PM Jacob Champion > wrote: >> I wouldn't argue for backpatching, for sure, but I guess I saw this as >> falling into the same vein as 5b3c5953 and bcc704b52 which were >> already committed. > > I

Re: DecodeInterval fixes

2023-08-27 Thread Joseph Koshakow
On Tue, Aug 22, 2023 at 12:58 PM Jacob Champion wrote: > > On Mon, Aug 21, 2023 at 10:39 PM Michael Paquier wrote: > > 0002 and 0003 make this stuff fail, but isn't there a risk that this > > breaks applications that relied on these accidental behaviors? > > Assuming that this is OK makes me

Re: DecodeInterval fixes

2023-08-22 Thread Jacob Champion
On Mon, Aug 21, 2023 at 10:39 PM Michael Paquier wrote: > 0002 and 0003 make this stuff fail, but isn't there a risk that this > breaks applications that relied on these accidental behaviors? > Assuming that this is OK makes me nervous. I wouldn't argue for backpatching, for sure, but I guess I

Re: DecodeInterval fixes

2023-08-21 Thread Michael Paquier
On Mon, Jul 10, 2023 at 10:42:31AM -0700, Jacob Champion wrote: > On Mon, Jul 10, 2023 at 10:19 AM Reid Thompson > wrote: >> I made a another pass through the separated patches, it looks good to >> me. > > LGTM too. (Thanks Tom for the clarification on ECPG.) +SELECT INTERVAL '42 days 2

Re: DecodeInterval fixes

2023-07-10 Thread Jacob Champion
On Mon, Jul 10, 2023 at 10:19 AM Reid Thompson wrote: > I made a another pass through the separated patches, it looks good to > me. LGTM too. (Thanks Tom for the clarification on ECPG.) Marked RfC. --Jacob

Re: DecodeInterval fixes

2023-07-10 Thread Reid Thompson
On Sun, 2023-07-09 at 13:24 -0400, Joseph Koshakow wrote: > > I've broken up this patch into three logical commits and attached > them. > None of the actual code has changed. > > Thanks, > Joe Koshakow I made a another pass through the separated patches, it looks good to me. Thanks, Reid

Re: DecodeInterval fixes

2023-07-09 Thread reid . thompson
On Sat, 2023-07-08 at 13:18 -0400, Joseph Koshakow wrote: > Jacob Champion writes: > > Hi Joe, here's a partial review: > > Thanks so much for the review! > > > I'm new to this code, but I agree that the use of `type` and the > > lookahead are not particularly obvious/intuitive. At the very > >

Re: DecodeInterval fixes

2023-07-09 Thread Joseph Koshakow
On Sat, Jul 8, 2023 at 5:06 PM Gurjeet Singh wrote: > I feel the staleness/deficiencies you mention above are not > captured in the TODO wiki page. It'd be nice if these were documented, > so that newcomers to the community can pick up work that they feel is > an easy lift for them. I think

Re: DecodeInterval fixes

2023-07-08 Thread Gurjeet Singh
On Sat, Jul 8, 2023 at 1:33 PM Tom Lane wrote: > > Gurjeet Singh writes: > > On Fri, Jul 7, 2023 at 4:13 PM Tom Lane wrote: > >> The ECPG datetime datatype support code has been basically unmaintained > >> for years, and has diverged quite far at this point from the core code. > > > I was under

Re: DecodeInterval fixes

2023-07-08 Thread Tom Lane
Gurjeet Singh writes: > On Fri, Jul 7, 2023 at 4:13 PM Tom Lane wrote: >> The ECPG datetime datatype support code has been basically unmaintained >> for years, and has diverged quite far at this point from the core code. > I was under the impression that anything in the postgresql.git >

Re: DecodeInterval fixes

2023-07-08 Thread Gurjeet Singh
On Fri, Jul 7, 2023 at 4:13 PM Tom Lane wrote: > > The ECPG datetime datatype support code has been basically unmaintained > for years, and has diverged quite far at this point from the core code. I was under the impression that anything in the postgresql.git repository is considered core, and

Re: DecodeInterval fixes

2023-07-08 Thread Joseph Koshakow
Jacob Champion writes: > Hi Joe, here's a partial review: Thanks so much for the review! > I'm new to this code, but I agree that the use of `type` and the > lookahead are not particularly obvious/intuitive. At the very least, > they'd need some more explanation in the code. Your boolean flag

Re: DecodeInterval fixes

2023-07-07 Thread Tom Lane
Jacob Champion writes: > Hi Joe, here's a partial review: > On Sun, Apr 9, 2023 at 5:44 PM Joseph Koshakow wrote: >> 1) Removes dead code for handling unit type RESERVE. > Looks good. From a quick skim it looks like the ECPG copy of this code > (ecpg/pgtypeslib/interval.c) might need to be

Re: DecodeInterval fixes

2023-07-07 Thread Jacob Champion
Hi Joe, here's a partial review: On Sun, Apr 9, 2023 at 5:44 PM Joseph Koshakow wrote: > 1) Removes dead code for handling unit type RESERVE. Looks good. From a quick skim it looks like the ECPG copy of this code (ecpg/pgtypeslib/interval.c) might need to be updated as well? > 2) Restrict the

DecodeInterval fixes

2023-04-09 Thread Joseph Koshakow
Hi all, This patch does three things in the DecodeInterval function: 1) Removes dead code for handling unit type RESERVE. There used to be a unit called "invalid" that was of type RESERVE. At some point that unit was removed and there were no more units of type RESERVE. Therefore, the code for