Re: add more frame types in window functions (ROWS)

Andrew Gierth wrote:
> But what you have in the regression tests _now_ is, as far as I can
> tell, only working by chance; with "rules" and "window" being in the
> same parallel group, and window.sql creating a view, you have an
> obvious race condition, unless I'm missing something.
>   
You're right.  rules.sql does this, among other things that might get 
impacted:
 SELECT viewname, definition FROM pg_views WHERE schemaname <> 
'information_schema' ORDER BY viewname;

Both rules and window are part of the same parallel group:
 test: select_views portals_p2 rules foreign_key cluster dependency guc 
bitmapops combocid tsearch tsdicts foreign_data window xmlmap

Which means that views created in the window test could absolutely cause 
the rules test to fail given a bad race condition.  Either rules or 
window needs to be moved to another section of the test schedule.  (I 
guess you could cut down the scope of "rules" to avoid this particular 
problem, but hacking other people's regression tests to work around 
issues caused by yours is never good practice).  I also agree with 
Andrew's sentiment that including a view on top of the new window 
implementations is good practice, just for general robustness.

Also, while not a strict requirement, I personally hate seeing things 
with simple names used in the regression tests, like how "v" is used in 
this patch.  The better written regression tests use a preface for all 
their names to decrease the chance of a conflict here; for example, the 
rules test names all of its views with names like rtest_v1 so they're 
very clearly not going to conflict with views created by other tests.  
No cleverness there can eliminate the conflict with rules though, when 
it's looking at every view in the system.

It looks like a lot of progress has been made on this patch through its 
review.  But there's enough open issues still that I think it could use 
a bit more time to mature before we try to get it committed--the fact 
that it's been getting bounced around for weeks now and the regression 
tests aren't even completely settled down yet is telling.  The feature 
seems complete, useful, and functionally solid, but still in need of 
some general cleanup and re-testing afterwards.  I'm going to mark this 
one "Returned with Feedback" for now.  Please continue to work on 
knocking all these issues out, this should be a lot easier to get 
committed in our next CF.

-- 
Greg Smith    2ndQuadrant   Baltimore, MD
PostgreSQL Training, Services and Support
greg@2ndQuadrant.com  www.2ndQuadrant.com