Re: OK for ABI break of PlannerInfo in 8.4?

Tom Lane wrote:
> Marc Cousins pointed out here
> http://archives.postgresql.org/pgsql-general/2010-03/msg01123.php
> that the "constraint_exclusion = partition" feature added in 8.4
> does not do what you'd expect for the target relation of an UPDATE
> or DELETE. That's because expansion of an inheritance set is managed
> differently for an UPDATE/DELETE target rel than for other cases.
>
> I'm intending to apply the attached patch to fix this in HEAD.
> I am tempted to back-patch it to 8.4 as well, but there is a potential
> problem for external modules that may be touching PlannerInfo (eg,
> planner hooks): the added field in that struct is an ABI break for them.
> We can minimize the risk by adding the new field at the end rather than
> in any more logical position; but it would still be a problem for
> modules that palloc'd or copied a PlannerInfo struct. AFAICS though the
> only real risk would be for relation_excluded_by_constraints to see a
> garbage value of root->hasInheritedTarget and possibly make an
> unexpected choice of what to do. I think that's probably a small enough
> problem to be acceptable. Comments?

Seems OK to me. It's worth noting though that if a module does do
palloc+memcpy of PlannerInfo, and it's compiled against the new sources
with the extra field, but used on an old server version, it will
memcpy() from beyond the end of the struct. If you're seriously unlucky
and the struct is at the end of allocated address space, that can segfault.

Unfortunately there doesn't seem to be any alignment padding in the
struct either. You could've stuck the new field there and avoided
changing sizeof(PlannerInfo).

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com