Re: Row pattern recognition

Hi Tatsuo,

Two things before the next review round.  Both aim at the same thing:
keeping the v50 diff to what RPR genuinely changes, so the next round
reviews RPR on its own merits rather than hunks that drifted in or touch
pre-existing window code.

First, the cfbot is currently red on v49 across all platforms.  The cause
is not RPR: master recently reworded the RESPECT/IGNORE NULLS error, which
left rpr_base's expected output stale, so the regression diff fails "make
check" everywhere.  That is exactly the one-line fix you already agreed to
fold in; I have posted it here as v50-0021 so the series builds green on
its own.  This one is required regardless.

Second, while going over the v49 diff against master I found several hunks
that are either unrelated to RPR or that touch pre-existing window code.  I
split them into small patches so each can be decided on its own.

Patch list
----------
Every patch in this reply.  revert-* removes the change from RPR;
optional-* re-applies the same change to master separately.

  v50-0021       cfbot build fix (required regardless)
  revert-0001    drop unrelated include and stray blank line  (required
cleanup)
  revert-0004    restore doc paragraph line wrapping          (required
cleanup)
  revert-0002    #2 drop the mark-position diagnostic         (your call)
  optional-0001  #2 land the diagnostic on master
  revert-0003    #3 restore the funcname guard                (your call)
  optional-0002  #3 land the guard removal on master
  revert-0005    #5 drop the EXCLUDE TIES tests               (your call)
  optional-0003  #5 land the tests on master

Required cleanups
-----------------
These only remove changes that crept in but have nothing to do with RPR.
I split them out so you can fold them into the feature patch; being
required cleanups, I recommend applying them:

  revert-0001    drop an unrelated "nodes/plannodes.h" include and a stray
                 blank line in eval_windowaggregates()
  revert-0004    restore the line wrapping of two pre-existing doc
                 paragraphs (advanced.sgml, ref/select.sgml)

Your call
---------
The next three touch pre-existing window code, so I would rather you decide
where each belongs.  I prepared the building blocks so you can pick any
option:

  #2  window_gettupleslot() mark-position elog -- add row positions
      v49 extended the can't-happen error with the pos/markpos values.
        (a) keep it in RPR                 -> apply neither patch
        (b) drop it                        -> apply revert-0002
        (c) land it on master separately   -> revert-0002 + optional-0001
      A tiny additive diagnostic; I lean to (c), but (a) is fine too.

  #3  funcname guard in WinCheckAndInitializeNullTreatment()
      v49 removed the "could not get function name" guard as unreachable.
        (a) keep the removal in RPR        -> apply neither patch
        (b) restore the guard, drop it     -> apply revert-0003
        (c) remove it on master separately -> revert-0003 + optional-0002
      The branch is genuinely unreachable, but it drops a deliberate guard
      in pre-existing code, so of the three this one most warrants caution.
      Preserving the guard via (b) or (c) looks safest, but I will leave the
      decision to you.

  #5  EXCLUDE TIES window-frame test coverage
      Two nth_value/last_value cases.  The accounting they exercise already
      exists on master and the tests pass without RPR, so they are not RPR
      coverage.
        (a) keep them in RPR               -> apply neither patch
        (b) land them on master separately -> revert-0005 + optional-0003
      I lean to (b).

For my part, I will carry out the deeper review against v50 with v50-0021
and revert-0001/0004 applied, plus whichever of the above you choose, as
the minimal green baseline.

Best,
Henson