From: Tom Lane <tgl@sss.pgh.pa.us>
To: Dean Rasheed <dean.a.rasheed@gmail.com>
cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Bogus use of canonicalize_qual
In-reply-to: 
 <CAEZATCU7zjHVtxeM0p857jr6s7TjfyVNDsHhF8-ad4O=qdbXhA@mail.gmail.com>
References: <24475.1520635069@sss.pgh.pa.us> <4911.1520713266@sss.pgh.pa.us>
 <CAEZATCU7zjHVtxeM0p857jr6s7TjfyVNDsHhF8-ad4O=qdbXhA@mail.gmail.com>
Comments: In-reply-to Dean Rasheed <dean.a.rasheed@gmail.com>
	message dated "Sun, 11 Mar 2018 09:09:22 -0000"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <17886.1520789035.1@sss.pgh.pa.us>
Date: Sun, 11 Mar 2018 13:23:55 -0400
Message-ID: <17887.1520789035@sss.pgh.pa.us>
Precedence: bulk

Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> On 10 March 2018 at 20:21, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> If we suppose that we only need to fix it in HEAD, the most attractive
>> answer is to add a parameter distinguishing WHERE and CHECK arguments
>> to canonicalize_qual.

> I agree that this looks like the best choice, but it feels a little
> unsatisfactory to not back-patch a fix for such a glaring bug. You
> could perhaps leave the signature of canonicalize_qual() the same, but
> add a new canonicalize_check() function, and make both thin wrappers
> on top of a local function accepting the is_check parameter.

Hm.  I'd be inclined to create canonicalize_qual_extended(qual, is_check)
and then make canonicalize_qual() call that with is_check = false.
But either way would avoid breaking API compatibility for the back
branches.

I guess the next question is whether we should do it the same way
in HEAD, avoiding a cross-branch difference.  But I don't like that,
because part of the point here IMO is to force any external callers
of canonicalize_qual() to reconsider what they're doing.

			regards, tom lane