MIME-Version: 1.0
References: <CALkF+nvtuas-2kydG-WfofbRSJpyODAJWun==W-yO5j2R4meqA@mail.gmail.com>
 <6200ACB3-8ECF-487B-8F9D-290BBEE146B4@yesql.se>
In-Reply-To: <6200ACB3-8ECF-487B-8F9D-290BBEE146B4@yesql.se>
From: Moaaz Assali <ma5679@nyu.edu>
Date: Tue, 27 Feb 2024 14:27:46 +0400
Message-ID: <CALkF+ns_ghEdVv2_mh=iCuirGnJHowtV0e_wg9_0RgkADo6dJg@mail.gmail.com>
Subject: Re: Fix for edge case in date_bin() function
To: daniel@yesql.se
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Content-Type: multipart/mixed; boundary="000000000000d5d1e106125a7aef"
Archived-At: <https://www.postgresql.org/message-id/CALkF%2Bns_ghEdVv2_mh%3DiCuirGnJHowtV0e_wg9_0RgkADo6dJg%40mail.gmail.com>
Precedence: bulk

--000000000000d5d1e106125a7aef
Content-Type: multipart/alternative; boundary="000000000000d5d1df06125a7aed"

--000000000000d5d1df06125a7aed
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hello Daniel,

I have added a test case for this in timestamp.sql and timestamp.out, and
tests pass when using the bug fix patch in the first email.
I have attached a new patch in this email below with the new tests only
(doesn't include the bug fix).

P.S. I forgot to CC the mailing list in my previous reply. This is just a
copy of it.

Best regards,
Moaaz Assali

On Tue, Feb 27, 2024 at 12:48=E2=80=AFPM Daniel Gustafsson <daniel@yesql.se=
> wrote:

> > On 27 Feb 2024, at 09:42, Moaaz Assali <ma5679@nyu.edu> wrote:
>
> > To account for this edge, we simply add another condition in the if
> statement to not perform the subtraction by one stride interval if the ti=
me
> difference is divisible by the stride.
>
> I only skimmed the patch, but I recommend adding a testcase to it to keep
> the
> regression from reappearing.  src/test/regress/sql/timestamp.sql might be=
 a
> good candidate testsuite.
>
> --
> Daniel Gustafsson
>
>

--000000000000d5d1df06125a7aed
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div>Hello Daniel,</div><div><br></div><div>I have added a=
 test case for
 this in timestamp.sql and timestamp.out, and tests pass when using the=20
bug fix patch in the first email.</div><div>I have attached a new patch in =
this email below with the new tests only (doesn&#39;t include the bug fix).=
</div><div><br></div><div>P.S. I forgot to CC the mailing list in my previo=
us reply. This is just a copy of it.<br></div><div><br></div><div>Best rega=
rds,</div><div>Moaaz Assali</div></div><br><div class=3D"gmail_quote"><div =
dir=3D"ltr" class=3D"gmail_attr">On Tue, Feb 27, 2024 at 12:48=E2=80=AFPM D=
aniel Gustafsson &lt;<a href=3D"mailto:daniel@yesql.se">daniel@yesql.se</a>=
&gt; wrote:<br></div><blockquote class=3D"gmail_quote" style=3D"margin:0px =
0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">&gt;=
 On 27 Feb 2024, at 09:42, Moaaz Assali &lt;<a href=3D"mailto:ma5679@nyu.ed=
u" target=3D"_blank">ma5679@nyu.edu</a>&gt; wrote:<br>
<br>
&gt; To account for this edge, we simply add another condition in the if st=
atement to not perform the subtraction by one stride interval if the time d=
ifference is divisible by the stride.<br>
<br>
I only skimmed the patch, but I recommend adding a testcase to it to keep t=
he<br>
regression from reappearing.=C2=A0 src/test/regress/sql/timestamp.sql might=
 be a<br>
good candidate testsuite.<br>
<br>
--<br>
Daniel Gustafsson<br>
<br>
</blockquote></div>

--000000000000d5d1df06125a7aed--

--000000000000d5d1e106125a7aef
Content-Type: application/octet-stream; 
	name="v1-0002-Regression-test-in-timestamp.sql-for-date_bin.patch"
Content-Disposition: attachment; 
	filename="v1-0002-Regression-test-in-timestamp.sql-for-date_bin.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_lt483p3z0>
X-Attachment-Id: f_lt483p3z0

RnJvbSBmNmI3ZjNkOGI5NDRjOWRkNGJkNzUwMjgzYzkzNmJjOWM2OTY3MDQxIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBNb2FheiBBc3NhbGkgPG1hNTY3OUBueXUuZWR1PgpEYXRlOiBU
dWUsIDI3IEZlYiAyMDI0IDA5OjU5OjE5ICswMDAwClN1YmplY3Q6IFtQQVRDSCB2MV0gUmVncmVz
c2lvbiB0ZXN0IGluIHRpbWVzdGFtcC5zcWwgZm9yIGRhdGVfYmluKCkgZnVuY3Rpb24uCgpBZGRl
ZCB0aGUgdGVzdCBjYXNlIGZvciB0aGUgYnVnIGZpeCB0aGF0IGhhbmRsZXMgdGhlIGVkZ2UgY2Fz
ZSBpbiBkYXRlX2JpbigpIHdoZW4Kc291cmNlIDwgb3JpZ2luIGFuZCBvcmlnaW4gaXMgZXF1aXZh
bGVudCB0byBhIHZhbGlkIGJpbm5lZCBkYXRlLgoKUmVmZXIgdG8gQ0FMa0YrbnZ0dWFzLTJreWRH
LVdmb2ZiUlNKcHlPREFKV3VuPT1XLXlPNWoyUjRtZXFBQG1haWwuZ21haWwuY29tIGZvciB0aGUg
YnVnIGZpeCBjb21taXQuCgpBZGRlZCB0aGUgdGVzdCBjYXNlIFNFTEVDVCBkYXRlX2JpbignMzAg
bWludXRlcyc6OmludGVydmFsLCB0aW1lc3RhbXAgJzIwMjQtMDItMDEgMTU6MDA6MDAnLCB0aW1l
c3RhbXAgJzIwMjQtMDItMDEgMTc6MDA6MDAnKTsKVGhlIGV4cGVjdGVkIHJldHVybmVkIHZhbHVl
IGZvciB0aGlzIHRlc3QgaXMgYSBzaW5nbGUgcm93IHdpdGggVGh1IEZlYiAwMSAxNTowMDowMCAy
MDI0CgpQcmlvciB0byB0aGUgYnVnIGZpeCwgZGF0ZV9iaW4oKSB3b3VsZCByZXR1cm4gVGh1IEZl
YiAwMSAxNDozMDowMCAyMDI0IGluc3RlYWQgb2YgVGh1IEZlYiAwMSAxNTowMDowMCAyMDI0LgpU
aGlzIHRlc3QgY2FzZSBlbnN1cmVzIHRoZSBjb3JyZWN0IHJldHVybiB2YWx1ZSBhbG9uZ3NpZGUg
dGhlIGJ1ZyBmaXggcGF0Y2guCi0tLQogc3JjL3Rlc3QvcmVncmVzcy9leHBlY3RlZC90aW1lc3Rh
bXAub3V0IHwgNyArKysrKysrCiBzcmMvdGVzdC9yZWdyZXNzL3NxbC90aW1lc3RhbXAuc3FsICAg
ICAgfCAzICsrKwogMiBmaWxlcyBjaGFuZ2VkLCAxMCBpbnNlcnRpb25zKCspCgpkaWZmIC0tZ2l0
IGEvc3JjL3Rlc3QvcmVncmVzcy9leHBlY3RlZC90aW1lc3RhbXAub3V0IGIvc3JjL3Rlc3QvcmVn
cmVzcy9leHBlY3RlZC90aW1lc3RhbXAub3V0CmluZGV4IDgzNWYwZTU3NjIuLjM0ZjJjZjM2Yjcg
MTAwNjQ0Ci0tLSBhL3NyYy90ZXN0L3JlZ3Jlc3MvZXhwZWN0ZWQvdGltZXN0YW1wLm91dAorKysg
Yi9zcmMvdGVzdC9yZWdyZXNzL2V4cGVjdGVkL3RpbWVzdGFtcC5vdXQKQEAgLTczNiw2ICs3MzYs
MTMgQEAgU0VMRUNUIGRhdGVfYmluKCc1IG1pbic6OmludGVydmFsLCB0aW1lc3RhbXAgJzIwMjAt
MDItMDEgMDE6MDE6MDEnLCB0aW1lc3RhbXAgJzIKICBTYXQgRmViIDAxIDAwOjU3OjMwIDIwMjAK
ICgxIHJvdykKIAorLS0gdGVzdCBkYXRlX2JpbiBvdXRwdXQgaXMgY29ycmVjdCB3aGVuIHNvdXJj
ZSA8IG9yaWdpbiBhbmQgc291cmNlIGlzIGVxdWl2YWxlbnQgdG8gYSB2YWxpZCBiaW5uZWQgZGF0
ZQorU0VMRUNUIGRhdGVfYmluKCczMCBtaW51dGVzJzo6aW50ZXJ2YWwsIHRpbWVzdGFtcCAnMjAy
NC0wMi0wMSAxNTowMDowMCcsIHRpbWVzdGFtcCAnMjAyNC0wMi0wMSAxNzowMDowMCcpOworICAg
ICAgICAgZGF0ZV9iaW4gICAgICAgICAKKy0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tCisgVGh1
IEZlYiAwMSAxNTowMDowMCAyMDI0CisoMSByb3cpCisKIC0tIGRpc2FsbG93IGludGVydmFscyB3
aXRoIG1vbnRocyBvciB5ZWFycwogU0VMRUNUIGRhdGVfYmluKCc1IG1vbnRocyc6OmludGVydmFs
LCB0aW1lc3RhbXAgJzIwMjAtMDItMDEgMDE6MDE6MDEnLCB0aW1lc3RhbXAgJzIwMDEtMDEtMDEn
KTsKIEVSUk9SOiAgdGltZXN0YW1wcyBjYW5ub3QgYmUgYmlubmVkIGludG8gaW50ZXJ2YWxzIGNv
bnRhaW5pbmcgbW9udGhzIG9yIHllYXJzCmRpZmYgLS1naXQgYS9zcmMvdGVzdC9yZWdyZXNzL3Nx
bC90aW1lc3RhbXAuc3FsIGIvc3JjL3Rlc3QvcmVncmVzcy9zcWwvdGltZXN0YW1wLnNxbAppbmRl
eCBlYTEyZmZkMThkLi5iMDZhZmQxNWRiIDEwMDY0NAotLS0gYS9zcmMvdGVzdC9yZWdyZXNzL3Nx
bC90aW1lc3RhbXAuc3FsCisrKyBiL3NyYy90ZXN0L3JlZ3Jlc3Mvc3FsL3RpbWVzdGFtcC5zcWwK
QEAgLTI2OCw2ICsyNjgsOSBAQCBGUk9NICgKIC0tIHNoaWZ0IGJpbnMgdXNpbmcgdGhlIG9yaWdp
biBwYXJhbWV0ZXI6CiBTRUxFQ1QgZGF0ZV9iaW4oJzUgbWluJzo6aW50ZXJ2YWwsIHRpbWVzdGFt
cCAnMjAyMC0wMi0wMSAwMTowMTowMScsIHRpbWVzdGFtcCAnMjAyMC0wMi0wMSAwMDowMjozMCcp
OwogCistLSB0ZXN0IGRhdGVfYmluIG91dHB1dCBpcyBjb3JyZWN0IHdoZW4gc291cmNlIDwgb3Jp
Z2luIGFuZCBzb3VyY2UgaXMgZXF1aXZhbGVudCB0byBhIHZhbGlkIGJpbm5lZCBkYXRlCitTRUxF
Q1QgZGF0ZV9iaW4oJzMwIG1pbnV0ZXMnOjppbnRlcnZhbCwgdGltZXN0YW1wICcyMDI0LTAyLTAx
IDE1OjAwOjAwJywgdGltZXN0YW1wICcyMDI0LTAyLTAxIDE3OjAwOjAwJyk7CisKIC0tIGRpc2Fs
bG93IGludGVydmFscyB3aXRoIG1vbnRocyBvciB5ZWFycwogU0VMRUNUIGRhdGVfYmluKCc1IG1v
bnRocyc6OmludGVydmFsLCB0aW1lc3RhbXAgJzIwMjAtMDItMDEgMDE6MDE6MDEnLCB0aW1lc3Rh
bXAgJzIwMDEtMDEtMDEnKTsKIFNFTEVDVCBkYXRlX2JpbignNSB5ZWFycyc6OmludGVydmFsLCAg
dGltZXN0YW1wICcyMDIwLTAyLTAxIDAxOjAxOjAxJywgdGltZXN0YW1wICcyMDAxLTAxLTAxJyk7
Ci0tIAoyLjM0LjEKCg==
--000000000000d5d1e106125a7aef--