Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lSUVK-0000WS-1J for pgsql-hackers@arkaria.postgresql.org; Sat, 03 Apr 2021 00:51:10 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1lSUVH-0001JF-Rz for pgsql-hackers@arkaria.postgresql.org; Sat, 03 Apr 2021 00:51:07 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lSUVH-0001J7-Jh for pgsql-hackers@lists.postgresql.org; Sat, 03 Apr 2021 00:51:07 +0000 Received: from mail-pf1-x429.google.com ([2607:f8b0:4864:20::429]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1lSUVE-00070q-FO for pgsql-hackers@lists.postgresql.org; Sat, 03 Apr 2021 00:51:06 +0000 Received: by mail-pf1-x429.google.com with SMTP id h3so4491786pfr.12 for ; Fri, 02 Apr 2021 17:51:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yugabyte.com; s=google; h=from:message-id:mime-version:subject:date:in-reply-to:cc:to :references; bh=MbSC50NTUoPLWN1UCSr4c3PuqKNG3ggPiEPgsZbb+hc=; b=NWtIpn3sF5L9CwHxmixrlCredD5gTmvykGW7+XHLQ4e2nmBKPItJvXB6HyX+O8GOdt ScASEPYpWEzhRV0F8F0rlNAbdr5nGCvFDhdui9PN5yRmGYlpMvC7bK/JorXEa77zvBqM 5VZ1xcPsEu5tKW1/zzHs0bN5ZFaRHAhQnCH+s= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:message-id:mime-version:subject:date :in-reply-to:cc:to:references; bh=MbSC50NTUoPLWN1UCSr4c3PuqKNG3ggPiEPgsZbb+hc=; b=FN5b3QDa516zGqUvA/LlCoKBCo7QvI6ZqXFlI2ujYz+eNTTOwlgKiJwk3CZS/K5Boa MaBrtvZSGXcAlobhVmeCc/WT1ufNP1WucmoSXfgc5kOECwYDEEJ9qQ1WoYOzWPUEdfz5 EPjoFORU/2tpo3TtTI+Q1JqbYc8Fivagz1NvT1v464uZIZWYZLZ+543NZYuMVjJZit03 hpOuEtnsbc4hnuXnZCvHNF6sbLP/6lOmVgQOpVTTawStz0OTZ7LqWofNCk1y3A/KnKYQ utbK3iuxtJaCgEN4hlwqb1qorKhC6YaUdW/58p211VXhbKyLdZMqDBiwnTr8fzyu9WsI VeQA== X-Gm-Message-State: AOAM531yaot55BKql9lwGjP505Z6+ds6pOtVAeS2VpOCP+j0x87l8jdx 94pTpVQ240YSof4hjtK8/VEJTA== X-Google-Smtp-Source: ABdhPJwjwqgp0PRQ9RRhd3wXyRlMtC6k+3kDPJVzqB0xA0zBiXfPo3U1UY6YuOfU00GyktFMjmMAIQ== X-Received: by 2002:a63:e715:: with SMTP id b21mr13649589pgi.300.1617411061196; Fri, 02 Apr 2021 17:51:01 -0700 (PDT) Received: from ?IPv6:2601:646:101:7fda:a519:674b:4a24:35b5? ([2601:646:101:7fda:a519:674b:4a24:35b5]) by smtp.gmail.com with ESMTPSA id h137sm8815130pfe.151.2021.04.02.17.51.00 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Fri, 02 Apr 2021 17:51:00 -0700 (PDT) From: Bryn Llewellyn Message-Id: <95D98B9E-CD54-4FAC-9574-D950E9741CAC@yugabyte.com> Content-Type: multipart/alternative; boundary="Apple-Mail=_5FD78592-BFCA-4AE6-92DA-1EB2F76BD712" Mime-Version: 1.0 (Mac OS X Mail 14.0 \(3654.60.0.2.21\)) Subject: Re: Have I found an interval arithmetic bug? Date: Fri, 2 Apr 2021 17:50:59 -0700 In-Reply-To: <20210402234943.GB29126@momjian.us> Cc: pgsql-hackers list To: Bruce Momjian References: <20210402180549.GF9270@momjian.us> <20210402234943.GB29126@momjian.us> X-Mailer: Apple Mail (2.3654.60.0.2.21) List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --Apple-Mail=_5FD78592-BFCA-4AE6-92DA-1EB2F76BD712 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 bruce@momjian.us wrote: > I have just posted a new version of the patch which I think covers all = the right areas. I found the relevant email from you to pgsql-hackers here: = https://www.postgresql.org/message-id/20210402234732.GA29125%40momjian.us You said: > I have modified the patch to prevent partial months from creating = partial hours/minutes/seconds=E2=80=A6 Partial years keeps it in months, = partial months takes it to days, and partial days take it to = hours/minutes/seconds. This seems like an improvement. I have written some PL/pgSQL code that faithfully emulates the behavior = that I see in my present vanilla PostgreSQL Version 13.2 system in a = wide range of tests. This is the key part: m1 int not null :=3D trunc(p.mo); m_remainder numeric not null :=3D p.mo - m1::numeric; m int not null :=3D trunc(p.yy*12) + m1; d_real numeric not null :=3D p.dd + m_remainder*30.0; d int not null :=3D floor(d_real); d_remainder numeric not null :=3D d_real - d::numeric; s numeric not null :=3D d_remainder*24.0*60.0*60.0 + p.hh*60.0*60.0 + p.mi*60.0 + p.ss; begin return (m, d, s)::modeled_interval_t; end; These quantities: p.yy, p.mo, p.dd, p.hh, p.mi, and p.ss are the user=E2=80=99s parameterization. All are real numbers. Because = non-integral values for years, months, days, hours, and minutes are = allowed when you specify a value using the ::interval typecast, my = reference doc must state the rules. I would have struggled to express = these rules in prose=E2=80=94especially given the use both of trunc() = and floor(). I would have struggled more to explain what requirements = these rules meet. I gather that by the time YugabyteDB has adopted your patch, my PL/pgSQL = will no longer be a correct emulation. So I=E2=80=99ll re-write it then. I intend to advise users always to constrain the values, when they = specify an interval value explicitly, so the the years, months, days, = hours, and minutes are integers. This is, after all, the discipline that = the make_interval() built-in imposes. So I might recommend using only = that. --Apple-Mail=_5FD78592-BFCA-4AE6-92DA-1EB2F76BD712 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 bruce@momjian.us = wrote:

I= have just posted a new version of the patch which I think covers = all the right areas.

I found the relevant email from you to pgsql-hackers = here:

https://www.postgresql.org/message-id/20210402234732.GA29125%40= momjian.us

You said:

I have modified the = patch to prevent partial months from creating partial = hours/minutes/seconds=E2=80=A6 Partial years keeps it in months, = partial months takes it to days, and partial days take it to = hours/minutes/seconds. This seems like an improvement.

I have written some PL/pgSQL code = that faithfully emulates the behavior that I see in my present vanilla = PostgreSQL Version 13.2 system in a wide range of tests. This is the key = part:

  m1           int =     not null :=3D trunc(p.mo);
  = m_remainder  numeric not null :=3D p.mo - m1::numeric;
  m            int   =   not null :=3D trunc(p.yy*12) + m1;

  d_real       numeric not null :=3D p.dd = + m_remainder*30.0;
  d         =    int     not null :=3D floor(d_real);
  d_remainder  numeric not null :=3D d_real - = d::numeric;

  s       =      numeric not null :=3D d_remainder*24.0*60.0*60.0 = +
                =                   =  p.hh*60.0*60.0 +
          =                     =      p.mi*60.0 +
      =                     =          p.ss;
begin
  return (m, d, s)::modeled_interval_t;
end;

These quantities:

p.yy, p.mo, p.dd, p.hh, p.mi, and = p.ss

are the = user=E2=80=99s parameterization. All are real numbers. Because = non-integral values for years, months, days, hours, and minutes are = allowed when you specify a value using the ::interval typecast, my = reference doc must state the rules. I would have struggled to express = these rules in prose=E2=80=94especially given the use both of trunc() = and floor(). I would have struggled more to explain what requirements = these rules meet.

I gather that by the time YugabyteDB has adopted your patch, = my PL/pgSQL will no longer be a correct emulation. So I=E2=80=99ll = re-write it then.

I intend to advise users always to constrain the values, when = they specify an interval value explicitly, so the the years, months, = days, hours, and minutes are integers. This is, after all, the = discipline that the make_interval() built-in imposes. So I might = recommend using only that.









= --Apple-Mail=_5FD78592-BFCA-4AE6-92DA-1EB2F76BD712--