Re: Row pattern recognition

Hi Henson,

>   0006: Fix DEFINE expression handling in RPR window
>         planning (revised)
> 
>         Integration tests in 0007 revealed two crashes:
>         (1) subquery wrapping with outer aggregate causes
>         WindowAgg removal when RPR window function output
>         is unused, (2) RPR and non-RPR windows coexisting
>         causes SIGSEGV from RPRNavExpr propagating to the
>         wrong WindowAgg. This version extends the fix to
>         extract only Var nodes via pull_var_clause()
>         instead of adding the whole DEFINE expression to
>         the targetlist. The allpaths.c guard is extended
>         to also preserve columns referenced by DEFINE
>         clauses.

I took a look at this and have a question.

--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -4750,6 +4750,74 @@ remove_unused_subquery_outputs(Query *subquery, RelOptInfo *rel,
 		if (contain_volatile_functions(texpr))
 			continue;
 
+		/*
+		 * If any RPR window clause references this column in its DEFINE
+		 * clause, don't remove it.  The DEFINE expression needs these columns
+		 * in the tuplestore slot for pattern matching evaluation, even if the
+		 * outer query doesn't reference them.
+		 */

Before this, there's a check in the function:

		if (tle->ressortgroupref || tle->resjunk)
			continue;

Below is the query in the regression test that is suppoed to trigger
the error.  In this case column "i" in the target list should have
resjunk flag to be set to true. So I thought the if statenment above
becomes true and the column i is not removed from subquery's target
list. Am I missing something?

The test case in the patch:

-- Subquery wrapping: RPR window inside outer aggregate.
-- Tests that WindowAgg is not removed by remove_unused_subquery_outputs()
-- when DEFINE clause contains PREV/NEXT.
--
-- PREV in DEFINE + outer aggregate

--EXPLAIN
SELECT count(*) FROM (
  SELECT count(*) OVER w FROM generate_series(1,10) i
  WINDOW w AS (
    ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
    PATTERN (A+)
    DEFINE A AS i > PREV(i)
  )
) t;

Also I think removal of the WindowAgg node is fine here because it's
not necessary to calculate the result of the sub query at
all. Actually the query above runs fine on v46. I guess some of the
incremental patches caused this to stop working. Can you elaborate?

Regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp