From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
cc: "Jung, Jinho" <jinho.jung@gatech.edu>, Jeff Janes <jeff.janes@gmail.com>,
        "pgsql-performance@postgresql.org" <pgsql-performance@postgresql.org>
Subject: Re: Performance regressions found using sqlfuzz
In-reply-to: <20190215164434.twk54vfogbpxqujm@alap3.anarazel.de>
References: 
 <BN6PR07MB3409922471073F2B619A8CA4EE640@BN6PR07MB3409.namprd07.prod.outlook.com>
 <CAMkU=1xoU06eW4CrEZyDDn+fnJaCe3b04rE3mdVu4Gsxmj9KFA@mail.gmail.com>
 <BN6PR07MB3409EE6CAAF8CCF43820AFB9EE670@BN6PR07MB3409.namprd07.prod.outlook.com>
 <20190215164434.twk54vfogbpxqujm@alap3.anarazel.de>
Comments: In-reply-to Andres Freund <andres@anarazel.de>
	message dated "Fri, 15 Feb 2019 08:44:34 -0800"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <27504.1550356669.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Sat, 16 Feb 2019 17:37:49 -0500
Message-ID: <27505.1550356669@sss.pgh.pa.us>
Precedence: bulk

Andres Freund <andres@anarazel.de> writes:
> On 2019-02-14 17:27:40 +0000, Jung, Jinho wrote:
>> - Our analysis: We believe that this regression has to do with two fact=
ors: 1) conditional expression (e.g., LEAST or NULLIF) are not reduced to =
constants unlike string functions (e.g., CHAR_LENGTH)  2) change in the co=
st estimation function for bitmap scan. Execution time grows by 3 orders o=
f magnitude.  We note that this regression is only observed on large datab=
ases (e.g., scale factor of 50).

> Hm. The primary problem here is that the estimation both before and
> after are really bad. So I don't think the commit you point out here is
> really to blame.  I'm not that bothered by the query not being great,
> given the weird construction with LEAST(), but we probably could fix
> that pretty easily.

We already did:

Author: Tom Lane <tgl@sss.pgh.pa.us>
Branch: master [6f19a8c41] 2018-12-30 13:42:04 -0500

    Teach eval_const_expressions to constant-fold LEAST/GREATEST expressio=
ns.
    =

    Doing this requires an assumption that the invoked btree comparison
    function is immutable.  We could check that explicitly, but in other
    places such as contain_mutable_functions we just assume that it's true=
,
    so we may as well do likewise here.  (If the comparison function's
    behavior isn't immutable, the sort order in indexes built with it woul=
d
    be unstable, so it seems certainly wrong for it not to be so.)
    =

    Vik Fearing
    =

    Discussion: https://postgr.es/m/c6e8504c-4c43-35fa-6c8f-3c0b80a912cc@2=
ndquadrant.com

BTW, const-folding NULLIF would not be a similarly tiny fix, because
it would need to check for immutability of the underlying operator
(since it is possibly a cross-type comparison, we can't get
away with just assuming it's immutable).  I'm not convinced that
case is worth carrying extra code for.

			regards, tom lane