Anybody using get_eclass_for_sort_expr in an extension?

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgreSQL(dot)org
Subject: Anybody using get_eclass_for_sort_expr in an extension?
Date: 2013-11-14 23:08:20
Message-ID: 672.1384470500@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I looked into bug #8591:
http://www.postgresql.org/message-id/E1Vgk41-00050x-Ck@wrigleys.postgresql.org
and was able to reproduce the problem. The proximate cause is that
get_eclass_for_sort_expr is wrong to suppose that it can always create new
equivalence class entries with empty em_nullable_relids; in the case
where the call is coming from initialize_mergeclause_eclasses, it's
essential to use the info supplied in restrictinfo->nullable_relids.
Otherwise, quals deduced from the equivalence class may have wrong
nullable_relids settings, allowing them to be pushed to unsafe places
in 9.2 and later.

This is relatively easy to fix as in the attached patch, but I'm wondering
how risky this is to back-patch. We can deal with the field added to
PlannerInfo by sticking it at the end of the struct in the back branches;
we've done that before and not heard squawks. However, the signature
change for get_eclass_for_sort_expr() would be problematic if any
third-party code is calling that function directly. I'm inclined to think
there probably isn't any such code, since none of the existing calls are
in index-specific code or other places that people would have been likely
to copy and modify. Still, I can't claim that the risk is zero.

We could hack around that in the back branches in more or less ugly ways,
such as by making get_eclass_for_sort_expr() be a wrapper around a new
function. However, it's far from clear what such a wrapper ought to do
--- it could pass NULL for nullable_relids, which would match the existing
behavior, but the odds seem good that the existing behavior might be wrong
for whatever an extension is trying to do. And having the code text
diverge between HEAD and back branches isn't so appetizing anyway.

I'm a bit inclined to take the risk of breaking anything that's calling
get_eclass_for_sort_expr() directly. Thoughts?

regards, tom lane

Attachment Content-Type Size
bug-8591-fix.patch text/x-diff 17.5 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Marko Kreen 2013-11-14 23:11:05 SSL: better default ciphersuite
Previous Message David Rowley 2013-11-14 23:07:39 strncpy is not a safe version of strcpy