From 29c4ad98293e3c5cb3fcdd413a3f4904efff8762 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 1 Jan 2010 21:53:49 +0000
Subject: [PATCH] Support "x IS NOT NULL" clauses as indexscan conditions. 
 This turns out to be just a minor extension of the previous patch that made
 "x IS NULL" indexable, because we can treat the IS NOT NULL condition as if
 it were "x < NULL" or "x > NULL" (depending on the index's NULLS FIRST/LAST
 option), just like IS NULL is treated like "x = NULL".  Aside from any
 possible usefulness in its own right, this is an important improvement for
 index-optimized MAX/MIN aggregates: it is now reliably possible to get a
 column's min or max value cheaply, even when there are a lot of nulls
 cluttering the interesting end of the index.

---
 doc/src/sgml/catalogs.sgml                 |   4 +-
 doc/src/sgml/indexam.sgml                  |   5 +-
 doc/src/sgml/indices.sgml                  |  11 +-
 src/backend/access/common/scankey.c        |   6 +-
 src/backend/access/gist/gistget.c          |  22 ++--
 src/backend/access/gist/gistscan.c         |  13 +-
 src/backend/access/nbtree/nbtutils.c       | 144 ++++++++++++++++-----
 src/backend/executor/nodeIndexscan.c       |  29 ++++-
 src/backend/optimizer/path/indxpath.c      |   7 +-
 src/backend/optimizer/plan/createplan.c    |  12 +-
 src/backend/optimizer/plan/planagg.c       |  38 ++++--
 src/backend/utils/adt/selfuncs.c           |  28 ++--
 src/include/access/skey.h                  |  25 ++--
 src/include/catalog/pg_am.h                |   4 +-
 src/include/nodes/relation.h               |   4 +-
 src/include/optimizer/planmain.h           |   3 +-
 src/test/regress/expected/create_index.out |  50 ++++++-
 src/test/regress/sql/create_index.sql      |  10 +-
 18 files changed, 295 insertions(+), 120 deletions(-)

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 0acd79361bd..9f81dc209ff 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -1,4 +1,4 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/catalogs.sgml,v 2.215 2009/12/29 20:11:42 tgl Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/catalogs.sgml,v 2.216 2010/01/01 21:53:48 tgl Exp $ -->
 <!--
  Documentation of the system catalogs, directed toward PostgreSQL developers
  -->
@@ -466,7 +466,7 @@
       <entry><structfield>amsearchnulls</structfield></entry>
       <entry><type>bool</type></entry>
       <entry></entry>
-      <entry>Does the access method support IS NULL searches?</entry>
+      <entry>Does the access method support IS NULL/NOT NULL searches?</entry>
      </row>
 
      <row>
diff --git a/doc/src/sgml/indexam.sgml b/doc/src/sgml/indexam.sgml
index b81cd27d313..97af5464456 100644
--- a/doc/src/sgml/indexam.sgml
+++ b/doc/src/sgml/indexam.sgml
@@ -1,4 +1,4 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/indexam.sgml,v 2.31 2009/07/29 20:56:17 tgl Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/indexam.sgml,v 2.32 2010/01/01 21:53:49 tgl Exp $ -->
 
 <chapter id="indexam">
  <title>Index Access Method Interface Definition</title>
@@ -134,7 +134,8 @@
    null values.  An index access method that sets
    <structfield>amindexnulls</structfield> may also set
    <structfield>amsearchnulls</structfield>, indicating that it supports
-   <literal>IS NULL</> clauses as search conditions.
+   <literal>IS NULL</> and <literal>IS NOT NULL</> clauses as search
+   conditions.
   </para>
 
  </sect1>
diff --git a/doc/src/sgml/indices.sgml b/doc/src/sgml/indices.sgml
index 7801b099f22..0ab3e25e80b 100644
--- a/doc/src/sgml/indices.sgml
+++ b/doc/src/sgml/indices.sgml
@@ -1,4 +1,4 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/indices.sgml,v 1.79 2009/08/07 20:54:31 alvherre Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/indices.sgml,v 1.80 2010/01/01 21:53:49 tgl Exp $ -->
 
 <chapter id="indexes">
  <title id="indexes-title">Indexes</title>
@@ -147,8 +147,8 @@ CREATE INDEX test1_id_index ON test1 (id);
 
    Constructs equivalent to combinations of these operators, such as
    <literal>BETWEEN</> and <literal>IN</>, can also be implemented with
-   a B-tree index search.  Also, an <literal>IS NULL</> condition on
-   an index column can be used with a B-tree index.
+   a B-tree index search.  Also, an <literal>IS NULL</> or <literal>IS NOT
+   NULL</> condition on an index column can be used with a B-tree index.
   </para>
 
   <para>
@@ -180,8 +180,7 @@ CREATE INDEX test1_id_index ON test1 (id);
    Hash indexes can only handle simple equality comparisons.
    The query planner will consider using a hash index whenever an
    indexed column is involved in a comparison using the
-   <literal>=</literal> operator.  (Hash indexes do not support
-   <literal>IS NULL</> searches.)
+   <literal>=</literal> operator.
    The following command is used to create a hash index:
 <synopsis>
 CREATE INDEX <replaceable>name</replaceable> ON <replaceable>table</replaceable> USING hash (<replaceable>column</replaceable>);
@@ -1025,7 +1024,7 @@ SELECT am.amname AS index_method,
      real statistics, some default values are assumed, which are
      almost certain to be inaccurate.  Examining an application's
      index usage without having run <command>ANALYZE</command> is
-     therefore a lost cause. 
+     therefore a lost cause.
      See <xref linkend="vacuum-for-statistics" endterm="vacuum-for-statistics-title">
      and <xref linkend="autovacuum" endterm="autovacuum-title"> for more information.
     </para>
diff --git a/src/backend/access/common/scankey.c b/src/backend/access/common/scankey.c
index 2524369abe4..f066becf012 100644
--- a/src/backend/access/common/scankey.c
+++ b/src/backend/access/common/scankey.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/common/scankey.c,v 1.32 2009/01/01 17:23:34 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/common/scankey.c,v 1.33 2010/01/01 21:53:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -21,7 +21,7 @@
  * ScanKeyEntryInitialize
  *		Initializes a scan key entry given all the field values.
  *		The target procedure is specified by OID (but can be invalid
- *		if SK_SEARCHNULL is set).
+ *		if SK_SEARCHNULL or SK_SEARCHNOTNULL is set).
  *
  * Note: CurrentMemoryContext at call should be as long-lived as the ScanKey
  * itself, because that's what will be used for any subsidiary info attached
@@ -45,7 +45,7 @@ ScanKeyEntryInitialize(ScanKey entry,
 		fmgr_info(procedure, &entry->sk_func);
 	else
 	{
-		Assert(flags & SK_SEARCHNULL);
+		Assert(flags & (SK_SEARCHNULL | SK_SEARCHNOTNULL));
 		MemSet(&entry->sk_func, 0, sizeof(entry->sk_func));
 	}
 }
diff --git a/src/backend/access/gist/gistget.c b/src/backend/access/gist/gistget.c
index a094495b8a9..98104a7a9f4 100644
--- a/src/backend/access/gist/gistget.c
+++ b/src/backend/access/gist/gistget.c
@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/gist/gistget.c,v 1.82 2009/10/08 22:34:57 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/gist/gistget.c,v 1.83 2010/01/01 21:53:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -413,14 +413,20 @@ gistindex_keytest(IndexTuple tuple,
 		{
 			/*
 			 * On non-leaf page we can't conclude that child hasn't NULL
-			 * values because of assumption in GiST: uinon (VAL, NULL) is VAL
-			 * But if on non-leaf page key IS  NULL then all childs has NULL.
+			 * values because of assumption in GiST: union (VAL, NULL) is VAL.
+			 * But if on non-leaf page key IS NULL, then all children are NULL.
 			 */
-
-			Assert(key->sk_flags & SK_SEARCHNULL);
-
-			if (GistPageIsLeaf(p) && !isNull)
-				return false;
+			if (key->sk_flags & SK_SEARCHNULL)
+			{
+				if (GistPageIsLeaf(p) && !isNull)
+					return false;
+			}
+			else
+			{
+				Assert(key->sk_flags & SK_SEARCHNOTNULL);
+				if (isNull)
+					return false;
+			}
 		}
 		else if (isNull)
 		{
diff --git a/src/backend/access/gist/gistscan.c b/src/backend/access/gist/gistscan.c
index aed3e95b4e3..dd43036524a 100644
--- a/src/backend/access/gist/gistscan.c
+++ b/src/backend/access/gist/gistscan.c
@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/gist/gistscan.c,v 1.76 2009/06/11 14:48:53 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/gist/gistscan.c,v 1.77 2010/01/01 21:53:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -92,15 +92,18 @@ gistrescan(PG_FUNCTION_ARGS)
 		 * field.
 		 *
 		 * Next, if any of keys is a NULL and that key is not marked with
-		 * SK_SEARCHNULL then nothing can be found.
+		 * SK_SEARCHNULL/SK_SEARCHNOTNULL then nothing can be found (ie,
+		 * we assume all indexable operators are strict).
 		 */
 		for (i = 0; i < scan->numberOfKeys; i++)
 		{
-			scan->keyData[i].sk_func = so->giststate->consistentFn[scan->keyData[i].sk_attno - 1];
+			ScanKey		skey = &(scan->keyData[i]);
 
-			if (scan->keyData[i].sk_flags & SK_ISNULL)
+			skey->sk_func = so->giststate->consistentFn[skey->sk_attno - 1];
+
+			if (skey->sk_flags & SK_ISNULL)
 			{
-				if ((scan->keyData[i].sk_flags & SK_SEARCHNULL) == 0)
+				if (!(skey->sk_flags & (SK_SEARCHNULL | SK_SEARCHNOTNULL)))
 					so->qual_ok = false;
 			}
 		}
diff --git a/src/backend/access/nbtree/nbtutils.c b/src/backend/access/nbtree/nbtutils.c
index b715f60d24f..33d3c9e8e77 100644
--- a/src/backend/access/nbtree/nbtutils.c
+++ b/src/backend/access/nbtree/nbtutils.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/nbtree/nbtutils.c,v 1.94 2009/10/08 22:34:57 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/nbtree/nbtutils.c,v 1.95 2010/01/01 21:53:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -276,6 +276,11 @@ _bt_preprocess_keys(IndexScanDesc scan)
 		 * in any particular strategy in this case, so set it to
 		 * BTEqualStrategyNumber --- we can treat IS NULL as an equality
 		 * operator for purposes of search strategy.
+		 *
+		 * Likewise, "x IS NOT NULL" is supported.  We treat that as either
+		 * "less than NULL" in a NULLS LAST index, or "greater than NULL"
+		 * in a NULLS FIRST index.  However, we have to flip those around in
+		 * a DESC index, to allow for the re-flipping that occurs elsewhere.
 		 */
 		if (cur->sk_flags & SK_ISNULL)
 		{
@@ -284,6 +289,21 @@ _bt_preprocess_keys(IndexScanDesc scan)
 				cur->sk_strategy = BTEqualStrategyNumber;
 				cur->sk_subtype = InvalidOid;
 			}
+			else if (cur->sk_flags & SK_SEARCHNOTNULL)
+			{
+				switch (indoption[cur->sk_attno - 1] &
+						(INDOPTION_DESC | INDOPTION_NULLS_FIRST))
+				{
+					case 0:		/* ASC / NULLS LAST */
+					case INDOPTION_DESC | INDOPTION_NULLS_FIRST:
+						cur->sk_strategy = BTLessStrategyNumber;
+						break;
+					default:
+						cur->sk_strategy = BTGreaterStrategyNumber;
+						break;
+				}
+				cur->sk_subtype = InvalidOid;
+			}
 			else
 				so->qual_ok = false;
 		}
@@ -320,7 +340,7 @@ _bt_preprocess_keys(IndexScanDesc scan)
 	{
 		if (i < numberOfKeys)
 		{
-			/* See comments above about NULLs and IS NULL handling. */
+			/* See comments above about NULLs and IS NULL/NOT NULL handling */
 			/* Note: we assume SK_ISNULL is never set in a row header key */
 			if (cur->sk_flags & SK_ISNULL)
 			{
@@ -329,6 +349,21 @@ _bt_preprocess_keys(IndexScanDesc scan)
 					cur->sk_strategy = BTEqualStrategyNumber;
 					cur->sk_subtype = InvalidOid;
 				}
+				else if (cur->sk_flags & SK_SEARCHNOTNULL)
+				{
+					switch (indoption[cur->sk_attno - 1] &
+							(INDOPTION_DESC | INDOPTION_NULLS_FIRST))
+					{
+						case 0:		/* ASC / NULLS LAST */
+						case INDOPTION_DESC | INDOPTION_NULLS_FIRST:
+							cur->sk_strategy = BTLessStrategyNumber;
+							break;
+						default:
+							cur->sk_strategy = BTGreaterStrategyNumber;
+							break;
+					}
+					cur->sk_subtype = InvalidOid;
+				}
 				else
 				{
 					so->qual_ok = false;
@@ -365,13 +400,6 @@ _bt_preprocess_keys(IndexScanDesc scan)
 					if (!chk || j == (BTEqualStrategyNumber - 1))
 						continue;
 
-					/* IS NULL together with any other predicate must fail */
-					if (eq->sk_flags & SK_SEARCHNULL)
-					{
-						so->qual_ok = false;
-						return;
-					}
-
 					if (_bt_compare_scankey_args(scan, chk, eq, chk,
 												 &test_result))
 					{
@@ -484,23 +512,6 @@ _bt_preprocess_keys(IndexScanDesc scan)
 		else
 		{
 			/* yup, keep only the more restrictive key */
-
-			/* if either arg is NULL, don't try to compare */
-			if ((cur->sk_flags | xform[j]->sk_flags) & SK_ISNULL)
-			{
-				/* at least one of them must be an IS NULL clause */
-				Assert(j == (BTEqualStrategyNumber - 1));
-				Assert((cur->sk_flags | xform[j]->sk_flags) & SK_SEARCHNULL);
-				/* if one is and one isn't, the search must fail */
-				if ((cur->sk_flags ^ xform[j]->sk_flags) & SK_SEARCHNULL)
-				{
-					so->qual_ok = false;
-					return;
-				}
-				/* we have duplicate IS NULL clauses, ignore the newer one */
-				continue;
-			}
-
 			if (_bt_compare_scankey_args(scan, cur, cur, xform[j],
 										 &test_result))
 			{
@@ -534,8 +545,7 @@ _bt_preprocess_keys(IndexScanDesc scan)
 }
 
 /*
- * Compare two scankey values using a specified operator.  Both values
- * must be already known non-NULL.
+ * Compare two scankey values using a specified operator.
  *
  * The test we want to perform is logically "leftarg op rightarg", where
  * leftarg and rightarg are the sk_argument values in those ScanKeys, and
@@ -555,8 +565,7 @@ _bt_preprocess_keys(IndexScanDesc scan)
  *
  * Note: this routine needs to be insensitive to any DESC option applied
  * to the index column.  For example, "x < 4" is a tighter constraint than
- * "x < 5" regardless of which way the index is sorted.  We don't worry about
- * NULLS FIRST/LAST either, since the given values are never nulls.
+ * "x < 5" regardless of which way the index is sorted.
  */
 static bool
 _bt_compare_scankey_args(IndexScanDesc scan, ScanKey op,
@@ -571,6 +580,64 @@ _bt_compare_scankey_args(IndexScanDesc scan, ScanKey op,
 				cmp_op;
 	StrategyNumber strat;
 
+	/*
+	 * First, deal with cases where one or both args are NULL.  This should
+	 * only happen when the scankeys represent IS NULL/NOT NULL conditions.
+	 */
+	if ((leftarg->sk_flags | rightarg->sk_flags) & SK_ISNULL)
+	{
+		bool		leftnull,
+					rightnull;
+
+		if (leftarg->sk_flags & SK_ISNULL)
+		{
+			Assert(leftarg->sk_flags & (SK_SEARCHNULL | SK_SEARCHNOTNULL));
+			leftnull = true;
+		}
+		else
+			leftnull = false;
+		if (rightarg->sk_flags & SK_ISNULL)
+		{
+			Assert(rightarg->sk_flags & (SK_SEARCHNULL | SK_SEARCHNOTNULL));
+			rightnull = true;
+		}
+		else
+			rightnull = false;
+
+		/*
+		 * We treat NULL as either greater than or less than all other values.
+		 * Since true > false, the tests below work correctly for NULLS LAST
+		 * logic.  If the index is NULLS FIRST, we need to flip the strategy.
+		 */
+		strat = op->sk_strategy;
+		if (op->sk_flags & SK_BT_NULLS_FIRST)
+			strat = BTCommuteStrategyNumber(strat);
+
+		switch (strat)
+		{
+			case BTLessStrategyNumber:
+				*result = (leftnull < rightnull);
+				break;
+			case BTLessEqualStrategyNumber:
+				*result = (leftnull <= rightnull);
+				break;
+			case BTEqualStrategyNumber:
+				*result = (leftnull == rightnull);
+				break;
+			case BTGreaterEqualStrategyNumber:
+				*result = (leftnull >= rightnull);
+				break;
+			case BTGreaterStrategyNumber:
+				*result = (leftnull > rightnull);
+				break;
+			default:
+				elog(ERROR, "unrecognized StrategyNumber: %d", (int) strat);
+				*result = false;		/* keep compiler quiet */
+				break;
+		}
+		return true;
+	}
+
 	/*
 	 * The opfamily we need to worry about is identified by the index column.
 	 */
@@ -844,11 +911,18 @@ _bt_checkkeys(IndexScanDesc scan,
 
 		if (key->sk_flags & SK_ISNULL)
 		{
-			/* Handle IS NULL tests */
-			Assert(key->sk_flags & SK_SEARCHNULL);
-
-			if (isNull)
-				continue;		/* tuple satisfies this qual */
+			/* Handle IS NULL/NOT NULL tests */
+			if (key->sk_flags & SK_SEARCHNULL)
+			{
+				if (isNull)
+					continue;		/* tuple satisfies this qual */
+			}
+			else
+			{
+				Assert(key->sk_flags & SK_SEARCHNOTNULL);
+				if (!isNull)
+					continue;		/* tuple satisfies this qual */
+			}
 
 			/*
 			 * Tuple fails this qual.  If it's a required qual for the current
diff --git a/src/backend/executor/nodeIndexscan.c b/src/backend/executor/nodeIndexscan.c
index b136825dc8f..df877ae191e 100644
--- a/src/backend/executor/nodeIndexscan.c
+++ b/src/backend/executor/nodeIndexscan.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/nodeIndexscan.c,v 1.136 2009/10/26 02:26:31 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/nodeIndexscan.c,v 1.137 2010/01/01 21:53:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -640,7 +640,8 @@ ExecInitIndexScan(IndexScan *node, EState *estate, int eflags)
  * (Note that we treat all array-expressions as requiring runtime evaluation,
  * even if they happen to be constants.)
  *
- * 5. NullTest ("indexkey IS NULL").  We just fill in the ScanKey properly.
+ * 5. NullTest ("indexkey IS NULL/IS NOT NULL").  We just fill in the
+ * ScanKey properly.
  *
  * Input params are:
  *
@@ -987,13 +988,14 @@ ExecIndexBuildScanKeys(PlanState *planstate, Relation index, Index scanrelid,
 		}
 		else if (IsA(clause, NullTest))
 		{
-			/* indexkey IS NULL */
-			Assert(((NullTest *) clause)->nulltesttype == IS_NULL);
+			/* indexkey IS NULL or indexkey IS NOT NULL */
+			NullTest   *ntest = (NullTest *) clause;
+			int			flags;
 
 			/*
 			 * argument should be the index key Var, possibly relabeled
 			 */
-			leftop = ((NullTest *) clause)->arg;
+			leftop = ntest->arg;
 
 			if (leftop && IsA(leftop, RelabelType))
 				leftop = ((RelabelType *) leftop)->arg;
@@ -1009,8 +1011,23 @@ ExecIndexBuildScanKeys(PlanState *planstate, Relation index, Index scanrelid,
 			/*
 			 * initialize the scan key's fields appropriately
 			 */
+			switch (ntest->nulltesttype)
+			{
+				case IS_NULL:
+					flags = SK_ISNULL | SK_SEARCHNULL;
+					break;
+				case IS_NOT_NULL:
+					flags = SK_ISNULL | SK_SEARCHNOTNULL;
+					break;
+				default:
+					elog(ERROR, "unrecognized nulltesttype: %d",
+						 (int) ntest->nulltesttype);
+					flags = 0;	/* keep compiler quiet */
+					break;
+			}
+
 			ScanKeyEntryInitialize(this_scan_key,
-								   SK_ISNULL | SK_SEARCHNULL,
+								   flags,
 								   varattno,	/* attribute number to scan */
 								   InvalidStrategy,		/* no strategy */
 								   InvalidOid,	/* no strategy subtype */
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index 7bb5d8993cc..456a5b9be48 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/path/indxpath.c,v 1.242 2009/09/17 20:49:28 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/path/indxpath.c,v 1.243 2010/01/01 21:53:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1218,7 +1218,7 @@ match_clause_to_indexcol(IndexOptInfo *index,
 	 * Clause must be a binary opclause, or possibly a ScalarArrayOpExpr
 	 * (which is always binary, by definition).  Or it could be a
 	 * RowCompareExpr, which we pass off to match_rowcompare_to_indexcol().
-	 * Or, if the index supports it, we can handle IS NULL clauses.
+	 * Or, if the index supports it, we can handle IS NULL/NOT NULL clauses.
 	 */
 	if (is_opclause(clause))
 	{
@@ -1256,8 +1256,7 @@ match_clause_to_indexcol(IndexOptInfo *index,
 	{
 		NullTest   *nt = (NullTest *) clause;
 
-		if (nt->nulltesttype == IS_NULL &&
-			match_index_to_operand((Node *) nt->arg, indexcol, index))
+		if (match_index_to_operand((Node *) nt->arg, indexcol, index))
 			return true;
 		return false;
 	}
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 43b0c123ca2..c8d7a28d622 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/createplan.c,v 1.268 2009/12/29 20:11:45 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/createplan.c,v 1.269 2010/01/01 21:53:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -73,7 +73,6 @@ static MergeJoin *create_mergejoin_plan(PlannerInfo *root, MergePath *best_path,
 static HashJoin *create_hashjoin_plan(PlannerInfo *root, HashPath *best_path,
 					 Plan *outer_plan, Plan *inner_plan);
 static List *fix_indexqual_references(List *indexquals, IndexPath *index_path);
-static Node *fix_indexqual_operand(Node *node, IndexOptInfo *index);
 static List *get_switched_clauses(List *clauses, Relids outerrelids);
 static List *order_qual_clauses(PlannerInfo *root, List *clauses);
 static void copy_path_costsize(Plan *dest, Path *src);
@@ -2117,7 +2116,6 @@ fix_indexqual_references(List *indexquals, IndexPath *index_path)
 		{
 			NullTest   *nt = (NullTest *) clause;
 
-			Assert(nt->nulltesttype == IS_NULL);
 			nt->arg = (Expr *) fix_indexqual_operand((Node *) nt->arg,
 													 index);
 		}
@@ -2131,7 +2129,13 @@ fix_indexqual_references(List *indexquals, IndexPath *index_path)
 	return fixed_indexquals;
 }
 
-static Node *
+/*
+ * fix_indexqual_operand
+ *	  Convert an indexqual expression to a Var referencing the index column.
+ *
+ * This is exported because planagg.c needs it.
+ */
+Node *
 fix_indexqual_operand(Node *node, IndexOptInfo *index)
 {
 	/*
diff --git a/src/backend/optimizer/plan/planagg.c b/src/backend/optimizer/plan/planagg.c
index 2b7934b686f..a32a06fc28d 100644
--- a/src/backend/optimizer/plan/planagg.c
+++ b/src/backend/optimizer/plan/planagg.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/planagg.c,v 1.47 2009/12/15 17:57:46 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/planagg.c,v 1.48 2010/01/01 21:53:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -37,7 +37,7 @@ typedef struct
 	Oid			aggfnoid;		/* pg_proc Oid of the aggregate */
 	Oid			aggsortop;		/* Oid of its sort operator */
 	Expr	   *target;			/* expression we are aggregating on */
-	Expr	   *notnulltest;	/* expression for "target IS NOT NULL" */
+	NullTest   *notnulltest;	/* expression for "target IS NOT NULL" */
 	IndexPath  *path;			/* access path for index scan */
 	Cost		pathcost;		/* estimated cost to fetch first row */
 	bool		nulls_first;	/* null ordering direction matching index */
@@ -308,7 +308,7 @@ build_minmax_path(PlannerInfo *root, RelOptInfo *rel, MinMaxAggInfo *info)
 	ntest = makeNode(NullTest);
 	ntest->nulltesttype = IS_NOT_NULL;
 	ntest->arg = copyObject(info->target);
-	info->notnulltest = (Expr *) ntest;
+	info->notnulltest = ntest;
 
 	/*
 	 * Build list of existing restriction clauses plus the notnull test. We
@@ -475,7 +475,7 @@ make_agg_subplan(PlannerInfo *root, MinMaxAggInfo *info)
 	PlannerInfo subroot;
 	Query	   *subparse;
 	Plan	   *plan;
-	Plan	   *iplan;
+	IndexScan  *iplan;
 	TargetEntry *tle;
 	SortGroupClause *sortcl;
 
@@ -529,16 +529,13 @@ make_agg_subplan(PlannerInfo *root, MinMaxAggInfo *info)
 	 *
 	 * Also we must add a "WHERE target IS NOT NULL" restriction to the
 	 * indexscan, to be sure we don't return a NULL, which'd be contrary to
-	 * the standard behavior of MIN/MAX.  XXX ideally this should be done
-	 * earlier, so that the selectivity of the restriction could be included
-	 * in our cost estimates.  But that looks painful, and in most cases the
-	 * fraction of NULLs isn't high enough to change the decision.
+	 * the standard behavior of MIN/MAX.
 	 *
 	 * The NOT NULL qual has to go on the actual indexscan; create_plan might
 	 * have stuck a gating Result atop that, if there were any pseudoconstant
 	 * quals.
 	 *
-	 * We can skip adding the NOT NULL qual if it's redundant with either an
+	 * We can skip adding the NOT NULL qual if it duplicates either an
 	 * already-given WHERE condition, or a clause of the index predicate.
 	 */
 	plan = create_plan(&subroot, (Path *) info->path);
@@ -546,14 +543,27 @@ make_agg_subplan(PlannerInfo *root, MinMaxAggInfo *info)
 	plan->targetlist = copyObject(subparse->targetList);
 
 	if (IsA(plan, Result))
-		iplan = plan->lefttree;
+		iplan = (IndexScan *) plan->lefttree;
 	else
-		iplan = plan;
-	Assert(IsA(iplan, IndexScan));
+		iplan = (IndexScan *) plan;
+	if (!IsA(iplan, IndexScan))
+		elog(ERROR, "result of create_plan(IndexPath) isn't an IndexScan");
 
-	if (!list_member(iplan->qual, info->notnulltest) &&
+	if (!list_member(iplan->indexqualorig, info->notnulltest) &&
 		!list_member(info->path->indexinfo->indpred, info->notnulltest))
-		iplan->qual = lcons(info->notnulltest, iplan->qual);
+	{
+		NullTest   *ntest;
+
+		/* Need a "fixed" copy as well as the original */
+		ntest = copyObject(info->notnulltest);
+		ntest->arg = (Expr *) fix_indexqual_operand((Node *) ntest->arg,
+													info->path->indexinfo);
+
+		iplan->indexqual = lappend(iplan->indexqual,
+								   ntest);
+		iplan->indexqualorig = lappend(iplan->indexqualorig,
+									   info->notnulltest);
+	}
 
 	plan = (Plan *) make_limit(plan,
 							   subparse->limitOffset,
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 8bc26cb6e97..56e3a085974 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -15,7 +15,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/adt/selfuncs.c,v 1.264 2009/12/29 20:11:45 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/adt/selfuncs.c,v 1.265 2010/01/01 21:53:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -5614,7 +5614,7 @@ btcostestimate(PG_FUNCTION_ARGS)
 	int			indexcol;
 	bool		eqQualHere;
 	bool		found_saop;
-	bool		found_null_op;
+	bool		found_is_null_op;
 	double		num_sa_scans;
 	ListCell   *l;
 
@@ -5639,7 +5639,7 @@ btcostestimate(PG_FUNCTION_ARGS)
 	indexcol = 0;
 	eqQualHere = false;
 	found_saop = false;
-	found_null_op = false;
+	found_is_null_op = false;
 	num_sa_scans = 1;
 	foreach(l, indexQuals)
 	{
@@ -5680,12 +5680,14 @@ btcostestimate(PG_FUNCTION_ARGS)
 		{
 			NullTest   *nt = (NullTest *) clause;
 
-			Assert(nt->nulltesttype == IS_NULL);
 			leftop = (Node *) nt->arg;
 			rightop = NULL;
 			clause_op = InvalidOid;
-			found_null_op = true;
-			is_null_op = true;
+			if (nt->nulltesttype == IS_NULL)
+			{
+				found_is_null_op = true;
+				is_null_op = true;
+			}
 		}
 		else
 		{
@@ -5725,12 +5727,7 @@ btcostestimate(PG_FUNCTION_ARGS)
 			}
 		}
 		/* check for equality operator */
-		if (is_null_op)
-		{
-			/* IS NULL is like = for purposes of selectivity determination */
-			eqQualHere = true;
-		}
-		else
+		if (OidIsValid(clause_op))
 		{
 			op_strategy = get_op_opfamily_strategy(clause_op,
 												   index->opfamily[indexcol]);
@@ -5738,6 +5735,11 @@ btcostestimate(PG_FUNCTION_ARGS)
 			if (op_strategy == BTEqualStrategyNumber)
 				eqQualHere = true;
 		}
+		else if (is_null_op)
+		{
+			/* IS NULL is like = for purposes of selectivity determination */
+			eqQualHere = true;
+		}
 		/* count up number of SA scans induced by indexBoundQuals only */
 		if (IsA(clause, ScalarArrayOpExpr))
 		{
@@ -5760,7 +5762,7 @@ btcostestimate(PG_FUNCTION_ARGS)
 		indexcol == index->ncolumns - 1 &&
 		eqQualHere &&
 		!found_saop &&
-		!found_null_op)
+		!found_is_null_op)
 		numIndexTuples = 1.0;
 	else
 	{
diff --git a/src/include/access/skey.h b/src/include/access/skey.h
index d295d3d12d5..509e1d08ddb 100644
--- a/src/include/access/skey.h
+++ b/src/include/access/skey.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/access/skey.h,v 1.37 2009/01/01 17:23:56 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/access/skey.h,v 1.38 2010/01/01 21:53:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -52,11 +52,13 @@ typedef uint16 StrategyNumber;
  * the operator.  When using a ScanKey in a heap scan, these fields are not
  * used and may be set to InvalidStrategy/InvalidOid.
  *
- * A ScanKey can also represent a condition "column IS NULL"; this is signaled
- * by the SK_SEARCHNULL flag bit.  In this case the argument is always NULL,
+ * A ScanKey can also represent a condition "column IS NULL" or "column
+ * IS NOT NULL"; these cases are signaled by the SK_SEARCHNULL and
+ * SK_SEARCHNOTNULL flag bits respectively.  The argument is always NULL,
  * and the sk_strategy, sk_subtype, and sk_func fields are not used (unless
- * set by the index AM).  Currently, SK_SEARCHNULL is supported only for
- * index scans, not heap scans; and not all index AMs support it.
+ * set by the index AM).  Currently, SK_SEARCHNULL and SK_SEARCHNOTNULL are
+ * supported only for index scans, not heap scans; and not all index AMs
+ * support them.
  *
  * Note: in some places, ScanKeys are used as a convenient representation
  * for the invocation of an access method support procedure.  In this case
@@ -112,12 +114,13 @@ typedef ScanKeyData *ScanKey;
  * bits should be defined here).  Bits 16-31 are reserved for use within
  * individual index access methods.
  */
-#define SK_ISNULL		0x0001	/* sk_argument is NULL */
-#define SK_UNARY		0x0002	/* unary operator (currently unsupported) */
-#define SK_ROW_HEADER	0x0004	/* row comparison header (see above) */
-#define SK_ROW_MEMBER	0x0008	/* row comparison member (see above) */
-#define SK_ROW_END		0x0010	/* last row comparison member (see above) */
-#define SK_SEARCHNULL	0x0020	/* scankey represents a "col IS NULL" qual */
+#define SK_ISNULL			0x0001	/* sk_argument is NULL */
+#define SK_UNARY			0x0002	/* unary operator (not supported!) */
+#define SK_ROW_HEADER		0x0004	/* row comparison header (see above) */
+#define SK_ROW_MEMBER		0x0008	/* row comparison member (see above) */
+#define SK_ROW_END			0x0010	/* last row comparison member */
+#define SK_SEARCHNULL		0x0020	/* scankey represents "col IS NULL" */
+#define SK_SEARCHNOTNULL	0x0040	/* scankey represents "col IS NOT NULL" */
 
 
 /*
diff --git a/src/include/catalog/pg_am.h b/src/include/catalog/pg_am.h
index a92c1f49971..1b3769fdbf5 100644
--- a/src/include/catalog/pg_am.h
+++ b/src/include/catalog/pg_am.h
@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/catalog/pg_am.h,v 1.62 2009/03/24 20:17:15 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/catalog/pg_am.h,v 1.63 2010/01/01 21:53:49 tgl Exp $
  *
  * NOTES
  *		the genbki.sh script reads this file and generates .bki
@@ -46,7 +46,7 @@ CATALOG(pg_am,2601)
 	bool		amcanmulticol;	/* does AM support multi-column indexes? */
 	bool		amoptionalkey;	/* can query omit key for the first column? */
 	bool		amindexnulls;	/* does AM support NULL index entries? */
-	bool		amsearchnulls;	/* can AM search for NULL index entries? */
+	bool		amsearchnulls;	/* can AM search for NULL/NOT NULL entries? */
 	bool		amstorage;		/* can storage type differ from column type? */
 	bool		amclusterable;	/* does AM support cluster command? */
 	Oid			amkeytype;		/* type of data in index, or InvalidOid */
diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index 8738a9924d5..fbbec6d5744 100644
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/relation.h,v 1.180 2009/11/28 00:46:19 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/relation.h,v 1.181 2010/01/01 21:53:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -459,7 +459,7 @@ typedef struct IndexOptInfo
 	bool		predOK;			/* true if predicate matches query */
 	bool		unique;			/* true if a unique index */
 	bool		amoptionalkey;	/* can query omit key for the first column? */
-	bool		amsearchnulls;	/* can AM search for NULL index entries? */
+	bool		amsearchnulls;	/* can AM search for NULL/NOT NULL entries? */
 	bool		amhasgettuple;	/* does AM have amgettuple interface? */
 	bool		amhasgetbitmap; /* does AM have amgetbitmap interface? */
 } IndexOptInfo;
diff --git a/src/include/optimizer/planmain.h b/src/include/optimizer/planmain.h
index 9a661a5ee8c..2e9f0659f26 100644
--- a/src/include/optimizer/planmain.h
+++ b/src/include/optimizer/planmain.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/optimizer/planmain.h,v 1.121 2009/10/26 02:26:45 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/optimizer/planmain.h,v 1.122 2010/01/01 21:53:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -39,6 +39,7 @@ extern Plan *optimize_minmax_aggregates(PlannerInfo *root, List *tlist,
  * prototypes for plan/createplan.c
  */
 extern Plan *create_plan(PlannerInfo *root, Path *best_path);
+extern Node *fix_indexqual_operand(Node *node, IndexOptInfo *index);
 extern SubqueryScan *make_subqueryscan(List *qptlist, List *qpqual,
 				  Index scanrelid, Plan *subplan,
 				  List *subrtable, List *subrowmark);
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 9dd54b39f09..b67f4e06f39 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -720,7 +720,7 @@ Indexes:
 
 DROP TABLE concur_heap;
 --
--- Tests for IS NULL with b-tree indexes
+-- Tests for IS NULL/IS NOT NULL with b-tree indexes
 --
 SELECT unique1, unique2 INTO onek_with_null FROM onek;
 INSERT INTO onek_with_null (unique1,unique2) VALUES (NULL, -1), (NULL, NULL);
@@ -740,6 +740,18 @@ SELECT count(*) FROM onek_with_null WHERE unique1 IS NULL AND unique2 IS NULL;
      1
 (1 row)
 
+SELECT count(*) FROM onek_with_null WHERE unique1 IS NOT NULL;
+ count 
+-------
+  1000
+(1 row)
+
+SELECT count(*) FROM onek_with_null WHERE unique1 IS NULL AND unique2 IS NOT NULL;
+ count 
+-------
+     1
+(1 row)
+
 DROP INDEX onek_nulltest;
 CREATE UNIQUE INDEX onek_nulltest ON onek_with_null (unique2 desc,unique1);
 SELECT count(*) FROM onek_with_null WHERE unique1 IS NULL;
@@ -754,6 +766,18 @@ SELECT count(*) FROM onek_with_null WHERE unique1 IS NULL AND unique2 IS NULL;
      1
 (1 row)
 
+SELECT count(*) FROM onek_with_null WHERE unique1 IS NOT NULL;
+ count 
+-------
+  1000
+(1 row)
+
+SELECT count(*) FROM onek_with_null WHERE unique1 IS NULL AND unique2 IS NOT NULL;
+ count 
+-------
+     1
+(1 row)
+
 DROP INDEX onek_nulltest;
 CREATE UNIQUE INDEX onek_nulltest ON onek_with_null (unique2 desc nulls last,unique1);
 SELECT count(*) FROM onek_with_null WHERE unique1 IS NULL;
@@ -768,6 +792,18 @@ SELECT count(*) FROM onek_with_null WHERE unique1 IS NULL AND unique2 IS NULL;
      1
 (1 row)
 
+SELECT count(*) FROM onek_with_null WHERE unique1 IS NOT NULL;
+ count 
+-------
+  1000
+(1 row)
+
+SELECT count(*) FROM onek_with_null WHERE unique1 IS NULL AND unique2 IS NOT NULL;
+ count 
+-------
+     1
+(1 row)
+
 DROP INDEX onek_nulltest;
 CREATE UNIQUE INDEX onek_nulltest ON onek_with_null (unique2  nulls first,unique1);
 SELECT count(*) FROM onek_with_null WHERE unique1 IS NULL;
@@ -782,6 +818,18 @@ SELECT count(*) FROM onek_with_null WHERE unique1 IS NULL AND unique2 IS NULL;
      1
 (1 row)
 
+SELECT count(*) FROM onek_with_null WHERE unique1 IS NOT NULL;
+ count 
+-------
+  1000
+(1 row)
+
+SELECT count(*) FROM onek_with_null WHERE unique1 IS NULL AND unique2 IS NOT NULL;
+ count 
+-------
+     1
+(1 row)
+
 RESET enable_seqscan;
 RESET enable_indexscan;
 RESET enable_bitmapscan;
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index b205afa34f7..519c3181a84 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -322,7 +322,7 @@ COMMIT;
 DROP TABLE concur_heap;
 
 --
--- Tests for IS NULL with b-tree indexes
+-- Tests for IS NULL/IS NOT NULL with b-tree indexes
 --
 
 SELECT unique1, unique2 INTO onek_with_null FROM onek;
@@ -335,6 +335,8 @@ SET enable_bitmapscan = ON;
 
 SELECT count(*) FROM onek_with_null WHERE unique1 IS NULL;
 SELECT count(*) FROM onek_with_null WHERE unique1 IS NULL AND unique2 IS NULL;
+SELECT count(*) FROM onek_with_null WHERE unique1 IS NOT NULL;
+SELECT count(*) FROM onek_with_null WHERE unique1 IS NULL AND unique2 IS NOT NULL;
 
 DROP INDEX onek_nulltest;
 
@@ -342,6 +344,8 @@ CREATE UNIQUE INDEX onek_nulltest ON onek_with_null (unique2 desc,unique1);
 
 SELECT count(*) FROM onek_with_null WHERE unique1 IS NULL;
 SELECT count(*) FROM onek_with_null WHERE unique1 IS NULL AND unique2 IS NULL;
+SELECT count(*) FROM onek_with_null WHERE unique1 IS NOT NULL;
+SELECT count(*) FROM onek_with_null WHERE unique1 IS NULL AND unique2 IS NOT NULL;
 
 DROP INDEX onek_nulltest;
 
@@ -349,6 +353,8 @@ CREATE UNIQUE INDEX onek_nulltest ON onek_with_null (unique2 desc nulls last,uni
 
 SELECT count(*) FROM onek_with_null WHERE unique1 IS NULL;
 SELECT count(*) FROM onek_with_null WHERE unique1 IS NULL AND unique2 IS NULL;
+SELECT count(*) FROM onek_with_null WHERE unique1 IS NOT NULL;
+SELECT count(*) FROM onek_with_null WHERE unique1 IS NULL AND unique2 IS NOT NULL;
 
 DROP INDEX onek_nulltest;
 
@@ -356,6 +362,8 @@ CREATE UNIQUE INDEX onek_nulltest ON onek_with_null (unique2  nulls first,unique
 
 SELECT count(*) FROM onek_with_null WHERE unique1 IS NULL;
 SELECT count(*) FROM onek_with_null WHERE unique1 IS NULL AND unique2 IS NULL;
+SELECT count(*) FROM onek_with_null WHERE unique1 IS NOT NULL;
+SELECT count(*) FROM onek_with_null WHERE unique1 IS NULL AND unique2 IS NOT NULL;
 
 RESET enable_seqscan;
 RESET enable_indexscan;
-- 
GitLab