Tweaking ResolveNew's API - Mailing list pgsql-hackers

From Tom Lane
Subject Tweaking ResolveNew's API
Date
Msg-id 718.1352402279@sss.pgh.pa.us
Whole thread Raw
Responses Re: Tweaking ResolveNew's API  (Alvaro Herrera <alvherre@2ndquadrant.com>)
List pgsql-hackers
The submitted patch for auto-updatable views uses rewriteManip.c's
ResolveNew() function to replace Vars referencing the view with Vars
referencing the underlying table.  That's mostly all right, except that
ResolveNew has some hard-wired choices about what it should do if a Var
to be replaced doesn't have any match in the replacement targetlist.
This should never occur in the auto-updatable view case, so really the
preferred behavior would be to throw an error, but that's not presently
one of the options.

What I'm thinking about doing is replacing ResolveNew's "event" argument
with a single-purpose enum listing the supported no-match actions,
along the lines of
enum {    RESOLVENEW_CHANGE_VARNO,    RESOLVENEW_SUBSTITUTE_NULL,    RESOLVENEW_REPORT_ERROR}

A possible objection to this is that most C compilers wouldn't complain
if a call site is still trying to use the old convention of passing a
CmdType value.  In the core code, there are only four call sites and
three are in rewriteHandler.c itself, so this isn't much of a problem
--- but if there's any third-party code such as FDWs that's trying to
make use of this function for querytree manipulation, there'd be a risk
of failing to notice the need to update the call.

One way to force a compile error would be to reorder the function's
argument list.  But doing so in a way that would definitely get the
compiler's attention seems to require a fairly arbitrary choice of
argument order, and also it would add a little extra risk of not
making the code changes correctly.  I'm inclined not to do that.

We have changed this function's API at least twice in the past, but each
time by adding new arguments, which will certainly draw a compile error;
so the lack of complaints about those changes doesn't necessarily prove
that nobody's using it outside core.

Thoughts, objections?
        regards, tom lane



pgsql-hackers by date:

Previous
From: Josh Berkus
Date:
Subject: Re: auto_explain WAS: RFC: Timing Events
Next
From: Tom Lane
Date:
Subject: Re: Proof of concept: auto updatable views [Review of Patch]