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
I've committed 0001. 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 indicate that it might produce better code,
too
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
On Wed, Aug 14, 2024 at 01:41:40PM -0400, Joseph Koshakow wrote:
> Since we know that a does not equal PG_INT64_MIN, could we shorten the
> last three lines and do the following?
>
> static inline uint64
> pg_abs_s64(int64 a)
> {
> if (unlikely(a == PG_INT64_MIN))
> r
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
On Wed, Aug 14, 2024 at 10:02:28AM +0300, Heikki Linnakangas wrote:
> On 14/08/2024 06:07, Nathan Bossart wrote:
>> And here's a new version of the patch in which I've attempted to fix the
>> silly mistakes.
>
> LGTM, just a few small comments:
Thanks for reviewing.
>> * - If a * b overflows,
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
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 short/int implementations. For
> the
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 short/int implementations. For
the latter, I've tried to use __builtin_sub_overflow() when possible, as
that
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
> >
On Mon, Jul 22, 2024 at 5:52 PM Alexander Lakhin
wrote:
> Also there are several trap-producing cases with date types:
> SELECT to_timestamp('10,999', 'Y,YYY');
I attached patch 5 that fixes the to_timestamp overflow.
Thank you,
Matthew Kim
From a01c5d67894de89f14f0dfc54d32e92258a1a3a7
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(
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.
Thanks,
Matthew Kim
From c142581fb5f4a26de40cdca0d8ca7d39abdb2e15 Mon Sep 17 00:00:00 2001
From: Joseph Koshakow
Date: Sat, 6 Jul 2024 15:41:09 -0400
Subject: [
On Wed, Aug 7, 2024 at 11:08 AM Nathan Bossart
wrote:
>
> 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 pos
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
On Mon, Jul 22, 2024 at 5:52 PM Alexander Lakhin
wrote:
> Also there are several trap-producing cases with date types:
> SELECT make_date(-2147483648, 1, 1);
Hi, I’ve attached a patch that fixes the make_date overflow. I’ve upgraded
the patches accordingly to reflect Joseph’s committed fix.
Tha
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
On Mon, Jul 22, 2024 at 6:07 PM Matthew Kim wrote:
>
> On Mon, Jul 22, 2024 at 5:52 PM Alexander Lakhin
wrote:
>
>> Also there are several trap-producing cases with date types:
>> SELECT to_date('1', 'CC');
>
> Hi, I’ve attached a patch that fixes the to_date() overflow. Patches 1
through
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 arroverflowtest(i[-2147483648:2147483647]) VALUES ('{1}');
>
> So we may want to add that test back in.
>
I agree with you.
also v13-0
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
>
> On Mon, Jul 22, 2024 at 5:52 PM Alexander Lakhin
> wrote:
>
> > Also there are several trap-producing cases with date types:
> > SELECT to_date('1', 'CC');
>
> Hi, I’ve attached a patch that fixes the to_date() overflow. Patches 1
> through 3 remain unchanged.
>
> Thank you,
> Matthew
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,
>> +
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,
> +(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
> +
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
On Wed, Jul 17, 2024 at 9:31 PM jian he wrote:
>
> i think "INSERT INTO arroverflowtest(i[2147483647:2147483647]) VALUES
('{}');"
> means to insert one element (size) to a customized lower/upper bounds.
Ah, thank you, I mistakenly understood that as an array with size
2147483647, with the first 2
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
On Tue, Jul 16, 2024 at 1:57 PM Nathan Bossart
wrote:
>>
>>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(
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((
Thanks for the review!
On Mon, Jul 15, 2024 at 11:31 AM Nathan Bossart
wrote:
>
>I took a closer look at 0002.
>
>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((float8) c,
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
On Thu, Jun 13, 2024 at 12:00 AM Alexander Lakhin
wrote:
> SELECT '[]'::jsonb -> -2147483648;
>
> #4 0x7efe232d67f3 in __GI_abort () at ./stdlib/abort.c:79
> #5 0x55e8fde9f211 in __negvsi2 ()
> #6 0x55e8fdcca62c in jsonb_array_element (fcinfo=0x55e8fec28220) at
jsonfuncs.c:948
>
> (
On Thu, Jun 13, 2024 at 12:00 AM Alexander Lakhin
wrote:
>
> And one more with array...
> CREATE TABLE t (ia int[]);
> INSERT INTO t(ia[2147483647:2147483647]) VALUES ('{}');
I've added another patch, 0003, to resolve this wrap-around. In fact I
discovered a bug that the following statement is ac
On Thu, Jun 13, 2024 at 10:56 PM Joseph Koshakow wrote:
On Thu, Jun 13, 2024 at 10:48 PM Joseph Koshakow
wrote:
>I've attached
>v4-0002-Handle-overflow-in-money-arithmetic.patch which adds some
>overflow checks and tests. I didn't address the float
multiplication
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',
On Thu, Jun 13, 2024 at 10:48 PM Joseph Koshakow wrote:
>I've attached
>v4-0002-Handle-overflow-in-money-arithmetic.patch which adds some
>overflow checks and tests. I didn't address the float multiplication
>because I didn't see any helper methods in int.h. I did some some
>us
On Thu, Jun 13, 2024 at 12:00 AM Alexander Lakhin
wrote:
>
>Let me remind you of bug #18240. Yes, that was about float8, but with
>-ftrapv we can get into the trap with:
>SELECT 1_000_000_000::money * 1_000_000_000::int;
>server closed the connection unexpectedly
Interesting, it l
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
On Tue, Jun 11, 2024 at 12:22 PM Nathan Bossart
wrote:
>I personally find that much easier to read. Since the existing
open-coded
>overflow check is apparently insufficient, I think there's a reasonably
>strong case for centralizing this sort of thing so that we don't
continue
>t
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
>> /* check the negative equivalent will fit without
overflowing */
>> if (unlikely(tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1))
>> goto out_of_range;
>> +
>> + /*
>> + * special case the minimum integer because its negation
c
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.
84 matches
Mail list logo
