On Fri, Dec 06, 2024 at 02:12:51PM -0600, Nathan Bossart wrote:
> I seem to have a knack for picking patches that take an entire afternoon to
> back-patch... Here's what I have staged for commit early next week.
Committed. I chickened out of back-patching this because 1) it adds new
routines to
On Fri, Dec 06, 2024 at 10:22:44AM -0600, Nathan Bossart wrote:
> On Thu, Dec 05, 2024 at 09:58:29PM -0600, Nathan Bossart wrote:
>> Thanks for reviewing. In v28, I fixed a silly mistake revealed by cfbot's
>> Windows run. I also went ahead and tried to fix most of the issues
>> reported in a nea
On Thu, Dec 05, 2024 at 09:58:29PM -0600, Nathan Bossart wrote:
> Thanks for reviewing. In v28, I fixed a silly mistake revealed by cfbot's
> Windows run. I also went ahead and tried to fix most of the issues
> reported in a nearby thread [0]. The only one I haven't tracked down yet
> is the "IS
On Thu, Dec 05, 2024 at 08:00:12PM -0500, Joseph Koshakow wrote:
> On Thu, Dec 5, 2024 at 5:50 PM Nathan Bossart
> wrote:
>> Good point. Here is a v27 patch that extracts the bug fix portions of the
>> v26 patch. If/when this is committed, I think we should close the
>> commitfest entry.
>
> I
On Thu, Dec 5, 2024 at 5:50 PM Nathan Bossart wrote:
> Good point. Here is a v27 patch that extracts the bug fix portions of the
> v26 patch. If/when this is committed, I think we should close the
> commitfest entry.
I looked through this patch and it looks good to me, assuming cfbot is
happy.
On Sat, Aug 24, 2024 at 08:44:40AM -0400, Joseph Koshakow wrote:
> FWIW, Matthew's patch actually does resolve a bug with `to_timestamp`
> and `to_date`. It converts the following incorrect queries
>
> test=# SELECT to_timestamp('2147483647,999', 'Y,YYY');
> to_timestamp
> --
On Wed, Aug 21, 2024 at 11:37 AM Nathan Bossart
wrote:
>
> Hm. It seems pretty clear that removing -fwrapv won't be happening
anytime
> soon. I don't mind trying to fix a handful of cases from time to time,
but
> unless there's a live bug, I'm probably not going to treat this stuff as
> high pri
On Wed, Aug 21, 2024 at 10:00:00AM +0300, Alexander Lakhin wrote:
> I'd like to add some info to show how big the iceberg is.
Hm. It seems pretty clear that removing -fwrapv won't be happening anytime
soon. I don't mind trying to fix a handful of cases from time to time, but
unless there's a liv
Hello Nathan,
21.08.2024 00:21, Nathan Bossart wrote:
I've combined all the current proposed changes into one patch. I've also
introduced signed versions of the negation functions into int.h to avoid
relying on multiplication.
Thank you for taking care of this!
I'd like to add some info to
I've combined all the current proposed changes into one patch. I've also
introduced signed versions of the negation functions into int.h to avoid
relying on multiplication.
--
nathan
>From 2364ba4028f879a22b9f69f999aee3ea9c013ec0 Mon Sep 17 00:00:00 2001
From: Nathan Bossart
Date: Tue, 20 Aug 2
18.08.2024 00:52, Joseph Koshakow wrote:
The largest possible (theoretical) value for `nbuckets` is
`1073741824`, the largest power of 2 that fits into an `int`. So, the
largest possible value for `nbuckets << 1` is `2147483648`. This can
fully fit in a `uint32`, so the simple fix for this case i
Hello Joe,
17.08.2024 22:16, Joseph Koshakow wrote:
Hi,
I wanted to take this opportunity to provide a brief summary of
outstanding work.
> Also there are several trap-producing cases with date types:
> SELECT to_date('1', 'CC');
> SELECT to_timestamp('10,999', 'Y,YYY');
> SELE
>>> SET temp_buffers TO 10;
>>>
>>> CREATE TEMP TABLE t(i int PRIMARY KEY);
>>> INSERT INTO t VALUES(1);
>>>
>>> #4 0x7f385cdd37f3 in __GI_abort () at ./stdlib/abort.c:79
>>> #5 0x5620071c4f51 in __addvsi3 ()
>>> #6 0x562007143f3c in init_htab (hashp=0x562008facb20,
nelem=610
Hi,
I wanted to take this opportunity to provide a brief summary of
outstanding work.
> Also there are several trap-producing cases with date types:
> SELECT to_date('1', 'CC');
> SELECT to_timestamp('10,999', 'Y,YYY');
> SELECT make_date(-2147483648, 1, 1);
This is resolved with
On Fri, Aug 16, 2024 at 01:35:01PM -0500, Nathan Bossart wrote:
> On Fri, Aug 16, 2024 at 09:00:00PM +0300, Alexander Lakhin wrote:
>> Sp it looks like jsonb_array_element_text() still needs the same
>> treatment as jsonb_array_element().
>
> D'oh. I added a test for that but didn't actually fix
On Fri, Aug 16, 2024 at 01:35:01PM -0500, Nathan Bossart wrote:
> On Fri, Aug 16, 2024 at 09:00:00PM +0300, Alexander Lakhin wrote:
>> #6 0x5576cf627c68 in bms_singleton_member (a=0x5576d09f7fb0) at
>> bitmapset.c:691
>> 691 if (result >= 0 || HAS_MULTIPLE_ONES(w))
On Fri, Aug 16, 2024 at 09:00:00PM +0300, Alexander Lakhin wrote:
> Sp it looks like jsonb_array_element_text() still needs the same
> treatment as jsonb_array_element().
D'oh. I added a test for that but didn't actually fix the code. I think
we just need something like the following.
diff --gi
Hello Nathan and Joe,
16.08.2024 19:52, Nathan Bossart wrote:
On Thu, Aug 15, 2024 at 10:49:46PM -0400, Joseph Koshakow wrote:
This updated version LGTM, I agree it's a good use of pg_abs_s32().
Committed.
Thank you for working on that issue!
I've tried `make check` with CC=gcc-13 CPPFLAGS=
On Thu, Aug 15, 2024 at 10:49:46PM -0400, Joseph Koshakow wrote:
> This updated version LGTM, I agree it's a good use of pg_abs_s32().
Committed.
--
nathan
On Thu, Aug 15, 2024 at 5:34 PM Nathan Bossart
wrote:
> Now to 0002...
>
> - if (-element > nelements)
> + if (element == PG_INT32_MIN || -element > nelements)
>
> This seems like a good opportunity to use our new pg_abs_s32() function,
> and godbolt.org [0] seems to i
om: Joseph Koshakow
Date: Sat, 6 Jul 2024 15:41:09 -0400
Subject: [PATCH v23 1/1] Remove dependence on integer wrapping for jsonb
This commit updates various jsonb operators and functions to no longer
rely on integer wrapping for correctness. Not all compilers support
-fwrapv, so it's best not
On Thu, Aug 15, 2024 at 02:56:00PM +0800, jian he wrote:
> i am confused with
> "
> +#elif defined(HAVE_INT128)
> + uint128 res = -((int128) a);
> "
> I thought "unsigned" means non-negative, therefore uint128 means non-negative.
> therefore "int128 res = -((int128) a);" makes sense to me.
Ah, th
Hello Joe,
05.08.2024 02:55, Joseph Koshakow wrote:
On Fri, Jun 14, 2024 at 8:00 AM Alexander Lakhin wrote:
>
> And the most interesting case to me:
> SET temp_buffers TO 10;
>
> CREATE TEMP TABLE t(i int PRIMARY KEY);
> INSERT INTO t VALUES(1);
>
...
Alex, are you able to
On Thu, Aug 15, 2024 at 2:16 AM Nathan Bossart wrote:
>
+static inline bool
+pg_neg_u64_overflow(uint64 a, int64 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(0, a, result);
+#elif defined(HAVE_INT128)
+ uint128 res = -((int128) a);
+
+ if (unlikely(res < PG_
On Wed, Aug 14, 2024 at 10:29:39PM +0300, Heikki Linnakangas wrote:
> Hmm, that still doesn't say what operation it's referring to. They existing
> comments say "a + b", "a - b" or "a * b", but this one isn't referring to
> anything at all. IMHO the existing comments are not too clear on that eithe
On 14/08/2024 20:20, Nathan Bossart wrote:
On Wed, Aug 14, 2024 at 10:02:28AM +0300, Heikki Linnakangas wrote:
On 14/08/2024 06:07, Nathan Bossart wrote:
* - If a * b overflows, return true, otherwise store the result of a * b
* into *result. The content of *result is implementation defin
16:46 -0400
Subject: [PATCH v22 1/1] Remove dependence on integer wrapping
This commit updates various parts of the code to no longer rely on
integer wrapping for correctness. Not all compilers support -fwrapv, so
it's best not to rely on it.
---
src/backend/utils/adt/cash.c
Thanks for the improvements Nathan. The current iteration LGTM, just a
single comment on `pg_abs_s64`
> +static inline uint64
> +pg_abs_s64(int64 a)
> +{
> + if (unlikely(a == PG_INT64_MIN))
> + return (uint64) PG_INT64_MAX + 1;
> + if (a < 0)
> + return -a;
> + return
comments to make this clear. I got too excited
about trimming it down and ended up obfuscating these important details.
> There's also some code in libpq's pqCheckOutBufferSpace() function that
> could use these functions.
Duly noted.
--
nathan
>From 08b32a02629a762835364396
On 14/08/2024 06:07, Nathan Bossart wrote:
On Tue, Aug 13, 2024 at 04:46:34PM -0500, Nathan Bossart wrote:
I've been preparing 0001 for commit. I've attached what I have so far.
The main changes are the implementations of pg_abs_* and pg_neg_*. For the
former, I've used abs()/i64abs() for the
hich I've attempted to fix the
silly mistakes.
--
nathan
>From 2b7b38f764f16b49594d0f8cc6192b7ba74ef906 Mon Sep 17 00:00:00 2001
From: Joseph Koshakow
Date: Sat, 8 Jun 2024 22:16:46 -0400
Subject: [PATCH v20 1/1] Remove dependence on integer wrapping
This commit updates various parts of t
[PATCH v19 1/1] Remove dependence on integer wrapping
This commit updates various parts of the code to no longer rely on
integer wrapping for correctness. Not all compilers support -fwrapv, so
it's best not to rely on it.
---
src/backend/utils/adt/cash.c | 12 +--
src
On Sat, Aug 10, 2024 at 11:41 PM Joseph Koshakow wrote:
>
>
>
> On Thu, Aug 8, 2024 at 9:01 PM jian he wrote:
> >
> > Should the error about integers be out of range?
> >
> > SELECT make_date(-2147483648, 1, 1);
> > "-2147483648" is not an allowed integer.
> >
> > \df make_date
> >
: 24:00:2.1
+SELECT make_date(-2147483648, 1, 1);
+ERROR: date field value out of range
diff --git a/src/test/regress/sql/date.sql b/src/test/regress/sql/date.sql
index 1c58ff6966..9a4e5832b9 100644
--- a/src/test/regress/sql/date.sql
+++ b/src/test/regress/sql/date.sql
@@ -373,3 +373,4 @@ select make
On Thu, Aug 8, 2024 at 9:01 PM jian he wrote:
>
> Should the error about integers be out of range?
>
> SELECT make_date(-2147483648, 1, 1);
> "-2147483648" is not an allowed integer.
>
> \df make_date
> List of functions
>Schema | Name| Result data
On Fri, Aug 9, 2024 at 6:16 AM Matthew Kim wrote:
>
> I've updated patch 0004 to check the return value of pg_mul_s32_overflow().
> Since tm.tm_year overflowed, the error message is hardcoded.
>
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -257,7 +257,10 @@ make_date(
bject: [PATCH 2/4] Remove dependence on integer wrapping for jsonb
This commit updates various jsonb operators and functions to no longer
rely on integer wrapping for correctness. Not all compilers support
-fwrapv, so it's best not to rely on it.
---
src/backend/utils/adt/jsonfuncs.c | 6 +
YYY DDD'); -- ok
SELECT to_date('2016 367', ' DDD');
SELECT to_date('0000-02-01','-MM-DD'); -- allowed, though it shouldn't be
+SELECT to_date('1', 'CC');
-- to_char's TZ format code produces zone ab
I started looking at 0001 again with the intent of committing it, and this
caught my eye:
-/* make the amount positive for digit-reconstruction loop */
-value = -value;
+/*
+ * make the amount positive for digit-reconstruction loop, we can
+ * leave INT64_MI
On Wed, Jul 24, 2024 at 05:49:41PM -0400, Matthew Kim wrote:
> Hi, I´ve attached a patch that fixes the make_date overflow. I´ve upgraded
> the patches accordingly to reflect Joseph´s committed fix.
cfbot is unhappy with this one on Windows [0], and I think I see why.
While the patch uses pg_mul_s
On Fri, Jun 14, 2024 at 8:00 AM Alexander Lakhin
wrote:
>
>And the most interesting case to me:
>SET temp_buffers TO 10;
>
>CREATE TEMP TABLE t(i int PRIMARY KEY);
>INSERT INTO t VALUES(1);
>
>#4 0x7f385cdd37f3 in __GI_abort () at ./stdlib/abort.c:79
>#5 0x000
ed fix.
Thank you,
Matthew Kim
v16-0004-Handle-negative-years-overflow-in-make_date.patch
Description: Binary data
v16-0002-Remove-dependence-on-integer-wrapping-for-jsonb.patch
Description: Binary data
v16-0001-Remove-dependence-on-integer-wrapping.patch
Description: Binary data
v16-0003-
On Tue, Jul 23, 2024 at 05:41:18PM -0400, Joseph Koshakow wrote:
> On Tue, Jul 23, 2024 at 2:14 AM jian he wrote:
>> On Tue, Jul 23, 2024 at 6:56 AM Joseph Koshakow wrote:
>>> The specific bug that this patch fixes is preventing the following
>>> statement:
>>>
>>> # INSERT INTO arroverflowte
following
>> statement:
>>
>> # INSERT INTO arroverflowtest(i[-2147483648:2147483647]) VALUES
('{1}');
>>
>> So we may want to add that test back in.
>>
> I agree with you.
I've updated the patch to add this test back in.
> also v13-0003-Re
>
I agree with you.
also v13-0003-Remove-dependence-on-integer-wrapping-for-jsonb.patch
in setPathArray we change to can
if (idx == PG_INT32_MIN || -idx > nelems)
{
/*
* If asked to keep elements position consistent, it's not a
On Mon, Jul 22, 2024 at 6:27 PM Nathan Bossart
wrote:
>
> Actually, I think my concerns about prohibiting more than necessary go
away
> if we do the subtraction first. If "upperIndx[i] - lowerIndx[i]"
> overflows, we know the array size is too big. Similarly, if adding one to
> that result overf
On Mon, Jul 22, 2024 at 04:36:33PM -0500, Nathan Bossart wrote:
> Okay. I'll plan on committing v13-0002 in the next couple of days, then.
Actually, I think my concerns about prohibiting more than necessary go away
if we do the subtraction first. If "upperIndx[i] - lowerIndx[i]"
overflows, we kn
through 3 remain unchanged.
>
> Thank you,
> Matthew Kim
>
v13-0003-Remove-dependence-on-integer-wrapping-for-jsonb.patch
Description: Binary data
v13-0004-Handle-overflows-in-do_to_timestamp.patch
Description: Binary data
v13-0001-Remove-dependence-on-integer-wrappi
On Mon, Jul 22, 2024 at 05:20:15PM -0400, Joseph Koshakow wrote:
> On Mon, Jul 22, 2024 at 11:17 AM Nathan Bossart
> wrote:
>> Am I understanding correctly that the main
>> behavioral difference between these two approaches is that users will see
>> different error messages?
>
> Yes, you are unde
On Mon, Jul 22, 2024 at 11:17 AM Nathan Bossart
wrote:
> On Fri, Jul 19, 2024 at 07:32:18PM -0400, Joseph Koshakow wrote:
>> On Fri, Jul 19, 2024 at 2:45 PM Nathan Bossart
>> wrote:
>>> +/* dim[i] = 1 + upperIndx[i] - lowerIndx[i]; */
>>> +if (pg_add_s32_overflow(1, upperI
On Fri, Jul 19, 2024 at 07:32:18PM -0400, Joseph Koshakow wrote:
> On Fri, Jul 19, 2024 at 2:45 PM Nathan Bossart
> wrote:
>> +/* dim[i] = 1 + upperIndx[i] - lowerIndx[i]; */
>> +if (pg_add_s32_overflow(1, upperIndx[i], &dim[i]))
>> +ereport(ERROR,
>> +
}');
+INSERT INTO arroverflowtest(i[2147483647:2147483647]) VALUES ('{}');
+INSERT INTO arroverflowtest(i[2147483647:2147483647]) VALUES ('{1}');
+INSERT INTO arroverflowtest(i[2147483646:2147483647]) VALUES ('{1,2}');
+INSERT INTO arroverflowtest(i[10:-21474836
I took a look at 0003.
+/* dim[i] = 1 + upperIndx[i] - lowerIndx[i]; */
+if (pg_add_s32_overflow(1, upperIndx[i], &dim[i]))
+ereport(ERROR,
+(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+ errmsg("array upper bound
On Thu, Jul 18, 2024 at 09:08:30PM -0400, Joseph Koshakow wrote:
> As a reminder:
> - 0001 is reviewed.
> - 0002 is reviewed and a bug fix.
> - 0003 is currently under review and a bug fix.
> - 0004 needs a review.
I've committed/back-patched 0002. I plan to review 0003 next.
--
nathan
;::money * 2::float4;
+SELECT '-92233720368547758.08'::money * 2::float4;
+SELECT 2::float4 * '92233720368547758.07'::money ;
+SELECT 2::float4 * '-92233720368547758.08'::money;
+SELECT '1'::money / 1.175494e-38::float8;
+SELECT '-1'::money / 1.17
On Wed, Jul 17, 2024 at 9:23 AM Joseph Koshakow wrote:
>
> Updated in the attached patch.
>
> Once again, the other patches, 0001, 0003, and 0004 are unchanged but
> have their version number incremented.
>
+-- Test for overflow in array slicing
+CREATE temp table arroverflowtest (i int[]);
+INS
On Tue, Jul 16, 2024 at 11:17 PM Nathan Bossart
wrote:
> I've attached an editorialized version of 0002 based on my thoughts above.
Looks great, thanks!
Thanks,
Joe Koshakow
On Tue, Jul 16, 2024 at 09:23:27PM -0400, Joseph Koshakow wrote:
> On Tue, Jul 16, 2024 at 1:57 PM Nathan Bossart
> wrote:
>> The reason I suggested this is so that we could omit all the prerequisite
>> isinf(), isnan(), etc. checks in the cash_mul_float8() and friends. The
>> checks are slighly
,{3,NULL}},{{5,6},{7,8}},{{9,10},{11,12}}}'::int[], 2));
SELECT array_sample('{1,2,3,4,5,6}'::int[], -1); -- fail
SELECT array_sample('{1,2,3,4,5,6}'::int[], 7); --fail
+
+-- Test for overflow in array slicing
+CREATE temp table arroverflowtest (i int[]);
+INSERT INTO arroverf
On Mon, Jul 15, 2024 at 07:55:22PM -0400, Joseph Koshakow wrote:
> On Mon, Jul 15, 2024 at 11:31 AM Nathan Bossart
> wrote:
>>I'm curious why you aren't using float8_mul/float8_div here, i.e.,
>>
>>fresult = rint(float8_mul((float8) c, f));
>>fresult = rint(float8_div((
27;);
select age(timestamp '-infinity', timestamp 'infinity');
select age(timestamp '-infinity', timestamp '-infinity');
+
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamp '1999-12-31 24:00:00';
+select make_timestamp(1999, 12, 31, 24, 0,
I took a closer look at 0002.
+if (unlikely(isinf(f) || isnan(f)))
+ereport(ERROR,
+(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("invalid float value")));
+
+fresult = rint(f * c);
+if (unlikely(f == 0.0))
+ereport(ERROR,
+
On Sat, Jul 13, 2024 at 10:24:16AM -0400, Joseph Koshakow wrote:
> On Fri, Jul 12, 2024 at 12:49 PM Nathan Bossart
> wrote:
>> IIUC some of these changes are bug fixes. Can we split out the bug fixes
>> to their own patches so that they can be back-patched?
>
> They happen to already be split ou
On Fri, Jul 12, 2024 at 12:49 PM Nathan Bossart
wrote:
> On Sat, Jul 06, 2024 at 07:04:38PM -0400, Joseph Koshakow wrote:
>> I've added another patch, 0004, to resolve the jsonb wrap-arounds.
>>
>> The other patches, 0001, 0002, and 0003 are unchanged but have their
>> version number incremented.
On Sat, Jul 06, 2024 at 07:04:38PM -0400, Joseph Koshakow wrote:
> I've added another patch, 0004, to resolve the jsonb wrap-arounds.
>
> The other patches, 0001, 0002, and 0003 are unchanged but have their
> version number incremented.
IIUC some of these changes are bug fixes. Can we split out
Thanks,
Joe Koshakow
From c8725de5f6e1ed476992c33f87b7e7c9475a952b Mon Sep 17 00:00:00 2001
From: Joseph Koshakow
Date: Sat, 6 Jul 2024 15:41:09 -0400
Subject: [PATCH 4/4] Remove dependence on integer wrapping for jsonb
This commit updates various jsonb operators and functions to no longer
rely
is reviewed and waiting for v18 and a committer.
0002 and 0003 are unreviewed. So, I'm going to mark this as waiting for
a reviewer.
Thanks,
Joe Koshakow
From f3747da14c887f97e50d9a3104881cbd3d5f60de Mon Sep 17 00:00:00 2001
From: Joseph Koshakow
Date: Sat, 8 Jun 2024 22:16:46 -0400
Subject
in-money-arithmetic.patch is ready for review.
v6-0001-Remove-dependence-on-integer-wrapping.patch is unchanged, I
just incremented the version number.
Thanks,
Joe Koshakow
From 6eec604618ee76227ee33fcddcc121d9915ff0ab Mon Sep 17 00:00:00 2001
From: Joseph Koshakow
Date: Sat, 8 Jun 2024 22:16:46 -0
14.06.2024 05:48, Joseph Koshakow wrote:
v4-0001-Remove-dependence-on-integer-wrapping.patch is unchanged, I
just incremented the version number.
> Also there are several trap-producing cases with date types:
> SELECT to_date('1', 'CC');
> SELECT to_timestamp('10,999',
2233720368547758.07'::money ;
+SELECT 2::int4 * '-92233720368547758.08'::money;
+SELECT '92233720368547758.07'::money * 2::int2;
+SELECT '-92233720368547758.08'::money * 2::int2;
+SELECT 2::int2 * '92233720368547758.07'::money ;
+SELECT 2::int2
e whole iceberg too.
+1
Thanks,
Joe Koshakow
From 31e8de30a82e60151848439143169e562bc848a3 Mon Sep 17 00:00:00 2001
From: Joseph Koshakow
Date: Sat, 8 Jun 2024 22:16:46 -0400
Subject: [PATCH 1/2] Remove dependence on integer wrapping
This commit updates various parts of the code to no longer rely on
i
10.06.2024 04:57, Tom Lane wrote:
BTW, while I approve of trying to get rid of our need for -fwrapv,
I'm quite scared of actually doing it. Whatever cases you may have
discovered by running the regression tests will be at best the
tip of the iceberg. Is there any chance of using static analysis
On Mon, Jun 10, 2024 at 2:30 PM Andres Freund wrote:
> On 2024-06-09 21:57:54 -0400, Tom Lane wrote:
> > BTW, while I approve of trying to get rid of our need for -fwrapv,
> > I'm quite scared of actually doing it.
IMV it's perfectly fine to defensively assume that we need -fwrapv,
even given a t
On Wed, Jun 12, 2024 at 11:45:20AM -0400, Tom Lane wrote:
> Nathan Bossart writes:
>> Thanks. This looks pretty good to me after a skim, so I'll see about
>> committing/back-patching it in the near future. IIUC there is likely more
>> to come in this area, but I see no need to wait.
>
> Uh ...
Nathan Bossart writes:
> Thanks. This looks pretty good to me after a skim, so I'll see about
> committing/back-patching it in the near future. IIUC there is likely more
> to come in this area, but I see no need to wait.
Uh ... what of this would we back-patch? It seems like v18
material to me
On Tue, Jun 11, 2024 at 09:10:44PM -0400, Joseph Koshakow wrote:
> The attached patch has updated this file too. For some reason I was
> under the impression that I should leave the ecpg/ files alone, though
> I can't remember why.
Thanks. This looks pretty good to me after a skim, so I'll see ab
t, 8 Jun 2024 22:16:46 -0400
Subject: [PATCH] Remove dependence on integer wrapping
This commit updates various parts of the code to no longer rely on
integer wrapping for correctness. Not all compilers support -fwrapv, so
it's best not to rely on it.
---
src/backend/utils/adt/cash.c
On Tue, Jun 11, 2024 at 09:31:39AM -0400, Joseph Koshakow wrote:
> tmp is an uint16 here, it seems like you might have read it as an
> int16? We would need some helper method like
>
> static inline bool
> pg_neg_u16_overflow(uint16 a, int16 *result);
>
> and then we could replace
th -ftrapv enabled will reveal a lot
of issues. Honestly just grepping for +,-,* in certain directories
(like backend/utils/adt) would probably be fairly fruitful for anyone
with the patience. My previous overflow patch was the result of looking
through all the arithmetic in datetime.c.
Thanks,
Joe
Hi,
On 2024-06-09 21:57:54 -0400, Tom Lane wrote:
> BTW, while I approve of trying to get rid of our need for -fwrapv,
> I'm quite scared of actually doing it.
I think that's a quite fair concern. One potentially relevant datapoint is
that we actually don't have -fwrapv equivalent on all platform
Nathan Bossart writes:
> On Sun, Jun 09, 2024 at 09:57:54PM -0400, Tom Lane wrote:
>> I think you could replace the whole thing by using overflow-aware
>> multiplication and addition primitives in the result calculation.
> I was still confused by the comment about 1999, but I tracked it down to
>
On Sun, Jun 09, 2024 at 09:57:54PM -0400, Tom Lane wrote:
> Nathan Bossart writes:
>> On Sun, Jun 09, 2024 at 04:59:22PM -0400, Joseph Koshakow wrote:
>>> The following comment was in the code for parsing timestamps:
>>> /* check for just-barely overflow (okay except time-of-day wraps) */
>>> /* c
Nathan Bossart writes:
> On Sun, Jun 09, 2024 at 04:59:22PM -0400, Joseph Koshakow wrote:
>> The following comment was in the code for parsing timestamps:
>> /* check for just-barely overflow (okay except time-of-day wraps) */
>> /* caution: we want to allow 1999-12-31 24:00:00 */
>>
>> I wasn't
On Sun, Jun 09, 2024 at 04:59:22PM -0400, Joseph Koshakow wrote:
> I originally added the helper functions to int.h thinking I'd find
> many more instances of overflow due to integer negation, however I
> didn't find that many. So let me know if you think we'd be better
> off without the functions.
,
Joe Koshakow
[0]
https://www.postgresql.org/message-id/20240213191401.jjhsic7et4tiahjs%40awork3.anarazel.de
From 319bc904858ad8fbcc687a923733defd3358c7b9 Mon Sep 17 00:00:00 2001
From: Joseph Koshakow
Date: Sat, 8 Jun 2024 22:16:46 -0400
Subject: [PATCH] Remove dependence on integer wrapping
This commit upd
85 matches
Mail list logo
