On Fri, May 17, 2024 at 03:53:58PM -0400, Robert Haas wrote:
> The usual pattern for using hooks like this is to call the next
> implementation, or the standard implementation, and pass down the
> arguments that you got. If you try to do that and mess it up, you're
> going to get a crash really, really quickly and it shouldn't be very
> difficult at all to figure out what you did wrong. Honestly, that
> doesn't seem like it would be hard even without the assertions: for
> the most part, a quick glance at the stack backtrace ought to be
> enough. If you're trying to do something more sophisticated, like
> mutating the node tree before passing it down, the chances of your
> mistakes getting caught by these assertions are pretty darn low, since
> they're very superficial checks.
Perhaps, still that would be something.
Hmm. We've had in the past bugs where DDL paths were playing with the
manipulation of Querys in core, corrupting their contents. It seems
like what you would mean is to have a check at the *end* of
ProcessUtility() that does an equalFuncs() on the Query, comparing it
with a copy of it taken at its beginning, all that hidden within two
USE_ASSERT_CHECKING blocks, when we'd expect the tree to not change.
With readOnlyTree, that would be just changing from one policy to
another, which is not really interesting at this stage based on how
ProcessUtility() is shaped.
--
Michael