Fix btree stop-at-nulls logic properly.

As pointed out by Naoya Anzai, my previous try at this was a few bricks
shy of a load, because I had forgotten that the initial-positioning logic
might not try to skip over nulls at the end of the index the scan will
start from.  We ought to fix that, because it represents an unnecessary
inefficiency, but first let's get the scan-stop logic back to a safe
state.  With this patch, we preserve the performance benefit requested
in bug #6278 for the case of scanning forward into NULLs (in a NULLS
LAST index), but the reverse case of scanning backward across NULLs
when there's no suitable initial-positioning qual is still inefficient.

src/backend/access/nbtree/nbtutils.c

@@ -609,7 +609,7 @@ _bt_advance_array_keys(IndexScanDesc scan, ScanDirection dir)
  * so that the index sorts in the desired direction.
  *
  * One key purpose of this routine is to discover which scan keys must be
- * satisfied to continue the scan. It also attempts to eliminate redundant
+ * satisfied to continue the scan.  It also attempts to eliminate redundant
  * keys and detect contradictory keys.  (If the index opfamily provides
  * incomplete sets of cross-type operators, we may fail to detect redundant
  * or contradictory keys, but we can survive that.)
@@ -647,7 +647,7 @@ _bt_advance_array_keys(IndexScanDesc scan, ScanDirection dir)
  * </<= keys if we can't compare them.  The logic about required keys still
  * works if we don't eliminate redundant keys.
  *
- * Note that the reason we need direction-sensitive required-key flags is
+ * Note that one reason we need direction-sensitive required-key flags is
  * precisely that we may not be able to eliminate redundant keys.  Suppose
  * we have "x > 4::int AND x > 10::bigint", and we are unable to determine
  * which key is more restrictive for lack of a suitable cross-type operator.
@@ -655,7 +655,10 @@ _bt_advance_array_keys(IndexScanDesc scan, ScanDirection dir)
  * positioning with.  If it picks x > 4, then the x > 10 condition will fail
  * until we reach index entries > 10; but we can't stop the scan just because
  * x > 10 is failing.  On the other hand, if we are scanning backwards, then
- * failure of either key is indeed enough to stop the scan.
+ * failure of either key is indeed enough to stop the scan.  (In general, when
+ * inequality keys are present, the initial-positioning code only promises to
+ * position before the first possible match, not exactly at the first match,
+ * for a forward scan; or after the last match for a backward scan.)
  *
  * As a byproduct of this work, we can detect contradictory quals such
  * as "x = 1 AND x > 2".  If we see that, we return so->qual_ok = FALSE,
@@ -1394,16 +1397,15 @@ _bt_checkkeys(IndexScanDesc scan,
 			}
 
 			/*
-			 * Tuple fails this qual.  If it's a required qual, then we can
-			 * conclude no further tuples will pass, either.  We can stop
-			 * regardless of the scan direction, because we know that NULLs
-			 * sort to one end or the other of the range of values.  If this
-			 * tuple doesn't pass, then no future ones will either, until we
-			 * reach the next set of values of the higher-order index attrs
-			 * (if any) ... and those attrs must have equality quals, else
-			 * this one wouldn't be marked required.
+			 * Tuple fails this qual.  If it's a required qual for the current
+			 * scan direction, then we can conclude no further tuples will
+			 * pass, either.
 			 */
-			if (key->sk_flags & (SK_BT_REQFWD | SK_BT_REQBKWD))
+			if ((key->sk_flags & SK_BT_REQFWD) &&
+				ScanDirectionIsForward(dir))
+				*continuescan = false;
+			else if ((key->sk_flags & SK_BT_REQBKWD) &&
+					 ScanDirectionIsBackward(dir))
 				*continuescan = false;
 
 			/*
@@ -1414,15 +1416,38 @@ _bt_checkkeys(IndexScanDesc scan,
 
 		if (isNull)
 		{
-			/*
-			 * The index entry is NULL, so it must fail this qual (we assume
-			 * all btree operators are strict).  Furthermore, we know that
-			 * all remaining entries with the same higher-order index attr
-			 * values must be NULLs too.  So, just as above, we can stop the
-			 * scan regardless of direction, if the qual is required.
-			 */
-			if (key->sk_flags & (SK_BT_REQFWD | SK_BT_REQBKWD))
-				*continuescan = false;
+			if (key->sk_flags & SK_BT_NULLS_FIRST)
+			{
+				/*
+				 * Since NULLs are sorted before non-NULLs, we know we have
+				 * reached the lower limit of the range of values for this
+				 * index attr.  On a backward scan, we can stop if this qual
+				 * is one of the "must match" subset.  We can stop regardless
+				 * of whether the qual is > or <, so long as it's required,
+				 * because it's not possible for any future tuples to pass.
+				 * On a forward scan, however, we must keep going, because we
+				 * may have initially positioned to the start of the index.
+				 */
+				if ((key->sk_flags & (SK_BT_REQFWD | SK_BT_REQBKWD)) &&
+					ScanDirectionIsBackward(dir))
+					*continuescan = false;
+			}
+			else
+			{
+				/*
+				 * Since NULLs are sorted after non-NULLs, we know we have
+				 * reached the upper limit of the range of values for this
+				 * index attr.  On a forward scan, we can stop if this qual is
+				 * one of the "must match" subset.  We can stop regardless of
+				 * whether the qual is > or <, so long as it's required,
+				 * because it's not possible for any future tuples to pass.
+				 * On a backward scan, however, we must keep going, because we
+				 * may have initially positioned to the end of the index.
+				 */
+				if ((key->sk_flags & (SK_BT_REQFWD | SK_BT_REQBKWD)) &&
+					ScanDirectionIsForward(dir))
+					*continuescan = false;
+			}
 
 			/*
 			 * In any case, this indextuple doesn't match the qual.
@@ -1502,15 +1527,38 @@ _bt_check_rowcompare(ScanKey skey, IndexTuple tuple, TupleDesc tupdesc,
 
 		if (isNull)
 		{
-			/*
-			 * The index entry is NULL, so it must fail this qual (we assume
-			 * all btree operators are strict).  Furthermore, we know that
-			 * all remaining entries with the same higher-order index attr
-			 * values must be NULLs too.  So, just as above, we can stop the
-			 * scan regardless of direction, if the qual is required.
-			 */
-			if (subkey->sk_flags & (SK_BT_REQFWD | SK_BT_REQBKWD))
-				*continuescan = false;
+			if (subkey->sk_flags & SK_BT_NULLS_FIRST)
+			{
+				/*
+				 * Since NULLs are sorted before non-NULLs, we know we have
+				 * reached the lower limit of the range of values for this
+				 * index attr.  On a backward scan, we can stop if this qual
+				 * is one of the "must match" subset.  We can stop regardless
+				 * of whether the qual is > or <, so long as it's required,
+				 * because it's not possible for any future tuples to pass.
+				 * On a forward scan, however, we must keep going, because we
+				 * may have initially positioned to the start of the index.
+				 */
+				if ((subkey->sk_flags & (SK_BT_REQFWD | SK_BT_REQBKWD)) &&
+					ScanDirectionIsBackward(dir))
+					*continuescan = false;
+			}
+			else
+			{
+				/*
+				 * Since NULLs are sorted after non-NULLs, we know we have
+				 * reached the upper limit of the range of values for this
+				 * index attr.  On a forward scan, we can stop if this qual is
+				 * one of the "must match" subset.  We can stop regardless of
+				 * whether the qual is > or <, so long as it's required,
+				 * because it's not possible for any future tuples to pass.
+				 * On a backward scan, however, we must keep going, because we
+				 * may have initially positioned to the end of the index.
+				 */
+				if ((subkey->sk_flags & (SK_BT_REQFWD | SK_BT_REQBKWD)) &&
+					ScanDirectionIsForward(dir))
+					*continuescan = false;
+			}
 
 			/*
 			 * In any case, this indextuple doesn't match the qual.

src/test/regress/expected/create_index.out

@@ -1405,6 +1405,60 @@ SELECT count(*) FROM onek_with_null WHERE unique1 IS NULL AND unique1 > 500;
      0
 (1 row)
 
+DROP INDEX onek_nulltest;
+-- Check initial-positioning logic too
+CREATE UNIQUE INDEX onek_nulltest ON onek_with_null (unique2);
+SET enable_seqscan = OFF;
+SET enable_indexscan = ON;
+SET enable_bitmapscan = OFF;
+SELECT unique1, unique2 FROM onek_with_null
+  ORDER BY unique2 LIMIT 2;
+ unique1 | unique2 
+---------+---------
+         |      -1
+     147 |       0
+(2 rows)
+
+SELECT unique1, unique2 FROM onek_with_null WHERE unique2 >= -1
+  ORDER BY unique2 LIMIT 2;
+ unique1 | unique2 
+---------+---------
+         |      -1
+     147 |       0
+(2 rows)
+
+SELECT unique1, unique2 FROM onek_with_null WHERE unique2 >= 0
+  ORDER BY unique2 LIMIT 2;
+ unique1 | unique2 
+---------+---------
+     147 |       0
+     931 |       1
+(2 rows)
+
+SELECT unique1, unique2 FROM onek_with_null
+  ORDER BY unique2 DESC LIMIT 2;
+ unique1 | unique2 
+---------+---------
+         |        
+     278 |     999
+(2 rows)
+
+SELECT unique1, unique2 FROM onek_with_null WHERE unique2 >= -1
+  ORDER BY unique2 DESC LIMIT 2;
+ unique1 | unique2 
+---------+---------
+     278 |     999
+       0 |     998
+(2 rows)
+
+SELECT unique1, unique2 FROM onek_with_null WHERE unique2 < 999
+  ORDER BY unique2 DESC LIMIT 2;
+ unique1 | unique2 
+---------+---------
+       0 |     998
+     744 |     997
+(2 rows)
+
 RESET enable_seqscan;
 RESET enable_indexscan;
 RESET enable_bitmapscan;

src/test/regress/sql/create_index.sql

@@ -487,6 +487,30 @@ SELECT count(*) FROM onek_with_null WHERE unique1 IS NULL AND unique2 IS NOT NUL
 SELECT count(*) FROM onek_with_null WHERE unique1 IS NOT NULL AND unique1 > 500;
 SELECT count(*) FROM onek_with_null WHERE unique1 IS NULL AND unique1 > 500;
 
+DROP INDEX onek_nulltest;
+
+-- Check initial-positioning logic too
+
+CREATE UNIQUE INDEX onek_nulltest ON onek_with_null (unique2);
+
+SET enable_seqscan = OFF;
+SET enable_indexscan = ON;
+SET enable_bitmapscan = OFF;
+
+SELECT unique1, unique2 FROM onek_with_null
+  ORDER BY unique2 LIMIT 2;
+SELECT unique1, unique2 FROM onek_with_null WHERE unique2 >= -1
+  ORDER BY unique2 LIMIT 2;
+SELECT unique1, unique2 FROM onek_with_null WHERE unique2 >= 0
+  ORDER BY unique2 LIMIT 2;
+
+SELECT unique1, unique2 FROM onek_with_null
+  ORDER BY unique2 DESC LIMIT 2;
+SELECT unique1, unique2 FROM onek_with_null WHERE unique2 >= -1
+  ORDER BY unique2 DESC LIMIT 2;
+SELECT unique1, unique2 FROM onek_with_null WHERE unique2 < 999
+  ORDER BY unique2 DESC LIMIT 2;
+
 RESET enable_seqscan;
 RESET enable_indexscan;
 RESET enable_bitmapscan;