So I'm testing a patch to make the planner use measured all-visible-page
counts for index-only scans. I was expecting to possibly see some plan
changes in the regression tests, but this diff scared me:
***************
*** 906,921 **** FROM tenk1 WHERE unique1 < 10; sum | unique1 -----+---------
! 4 | 4
! 6 | 2
! 3 | 1
! 7 | 6
! 15 | 9
! 17 | 8
! 13 | 5
! 8 | 3
! 10 | 7
! 7 | 0 (10 rows) CREATE TEMP VIEW v_window AS
--- 906,921 ---- FROM tenk1 WHERE unique1 < 10; sum | unique1 -----+---------
! 0 | 0
! 1 | 1
! 3 | 2
! 5 | 3
! 7 | 4
! 9 | 5
! 11 | 6
! 13 | 7
! 15 | 8
! 17 | 9 (10 rows) CREATE TEMP VIEW v_window AS
On inspection, though, there's no bug. The plan did change, and that
affected the order in which the rows are fetched, and that changed the
window function outputs because this test case is effectively using
SUM(x) OVER (ROWS 1 PRECEDING) without any ordering specifier.
There are several adjacent tests that are underspecified in the same
way, but their results didn't change because they aren't candidates for
index-only scans.
We could hack around this by adding more columns to the result so that
an index-only scan doesn't work. But I wonder whether it wouldn't be
smarter to add ORDER BY clauses to the window function calls. I've been
known to argue against adding just-in-case ORDER BYs to the regression
tests in the past; but these cases bother me more because a plan change
will not just rearrange the result rows but change their contents,
making it really difficult to verify that nothing's seriously wrong.
I can't recall whether there was some good reason for underspecifying
these test queries. It looks like all the problematic ones were added in
commit ec4be2ee6827b6bd85e0813c7a8993cfbb0e6fa7 "Extend the set of frame
options supported for window functions", which means it was either me or
Hitoshi-san who wrote them that way, but memory is not serving this
afternoon.
Opinions?
regards, tom lane