On Sun, Jun 27, 2021 at 10:34:52AM -0400, Tom Lane wrote:
> 
> I'm not really excited by this, as it seems like it's exposing internal
> decisions we could change someday; to wit, first that there is any such
> thing as a separate rewriting pass

Sure, but the fact that views will significantly impact the query being
executed from the one written is not an internal decision.  In my opinion
knowing what the final "real" query will be is still a valid concern, whether
we have a rewriting pass or not.

> and second that its output is
> interpretable as pure SQL.  (TBH, I'm not 100% sure that the second
> assumption is true even today, although I know there are ancient comments
> that claim that.)

I totally agree.  Note that there was at least one gotcha handled in this
patch: rewritten views didn't get an alias, which is mandatory for an SQL
query.

> It's not very hard to imagine someday moving view
> expansion into the planner on efficiency grounds, leaving the rewriter
> handling only the rare uses of INSERT/UPDATE/DELETE rules.

Agreed.  One the other hand having such a function in core may ensure that any
significant change in those area will keep an API to retrieve the final query
representation.

> If there's functions in ruleutils.c that we'd need to make public to
> let somebody write a debugging extension that does this kind of thing,
> I'd be happier with that approach than with creating a core-server SQL
> function for it.  There might be more than one use-case for the
> exposed bits.

It would mean exposing at least get_query_def().  I thought that exposing this
function was already suggested and refused, but I may be wrong.  Maybe other
people would like to have nearby functions exposed too.

Note that if we go this way, we would still need at least something like this
patch's chunk in rewriteHandler.c applied, as otherwise the vast majority of
rewritten and deparsed queries won't be valid.


Reply via email to