Re: Row pattern recognition

Hi jian,

> I don't think keeping the above EXPLAIN from failing justifies
> adding a new field (validate) to EvalDefineOffsetsContext.

The complexity cost is a fair point, and I do not have a settled
answer to it. Where validate lives, and when the error should be
raised, are both things I would rather spend attention on later, in a
pass over the finished series. I think we are weighting attention
differently more than we are disagreeing -- I have been spending mine
on the correctness defects, and beside those this reads to me as a
question of where an error is best raised rather than whether a wrong
answer can come out. Adjusting what already behaves correctly is what
I have been putting behind them, and not this proposal in particular.

One note for that later pass, so it is not lost: dropping validate
moves the check earlier only for a literal. An offset written as a
bind parameter, or the PARAM_EXEC a correlated SRF leaves behind, has
no value at executor init, so it still plans and still fails at
execution.

> Maybe we can just let such cases error out. Parser guarantees that
> PREV() and friends contain at least one column reference ...
> Hitting this requires deliberately building a one row table that
> also inlines to a constant, which is a corner case.

I would not concede that one. The pull-up there did not fold a
subexpression of the argument -- it took the argument's only column
reference away. pull_up_simple_values() fires only for a single-row
VALUES, so what it leaves behind is a scan PREV has no row to navigate
to:

DEFINE A AS PREV(v / 0) > 0 -> DEFINE A AS NULL

That is the fold the query is owed, and null is what it returns today;
rpr_base.sql has the row to show for it. Raising instead is not
accepting a corner case, it is producing the wrong answer where the
right one is a constant.

And the cost of skipping the argument is not that corner case anyway:

DEFINE A AS PREV(v + 2 * 3) > 0 -> DEFINE A AS PREV(v + 6) > 0

That one does not happen today either. The case copies arg through
untouched, so the multiply is evaluated again for every row the
navigation is asked about, and the XXX already in clauses.c says as
much -- "every row of the match pays for what was not."

Folding all the way to that constant null is the right end state, but
it takes knowing the navigation can never reach a row, which is not in
front of eval_const_expressions(). On the complexity ground you
raised above, I would rather not build that here, and nothing
observable is lost by not building it: the query returns the same row
either way, the DEFINE just stays an expression instead of collapsing.

So the line I would draw is the column reference, not the corner case.
Recurse into arg and fold it as usual while it still has one --
PREV(v + 2 * 3) then folds, and PREV(v + 1 / 0) raises exactly where
v + 1 / 0 raises outside a navigation. Once it has none left the
argument no longer depends on the row the navigation lands on, so
folding it can only manufacture an error, and that is the case your
example builds.

The rewrites are owed either way, since they are what makes the
argument executable at all, and that is the half the three XXX cases
in rpr_base.sql are waiting on.

Best regards,
Henson