Ensure that a StreamNode is always owned by only one StreamBitmap

In this commit, we change the ownership of a StreamNode, so that when a
StreamNode A is added as an input to a StreamNode B, the bitmap that was
pointing to A abandons the ownership and the pointer is set to NULL. In the
above example, when GPDB frees BitmapOR, it will not attempt to free OR's
StreamNode.

In addition, if a StreamNode is an OpStream, then opstream_free function is
used to free the memory used by the StreamNode, which actually uses pfree. On
the other hand, if a StreamNode is an IndexStream, then stream_free function is
invoked, which does not pfree the StreamNode. For consistency, we add function
indexstream_free to always pfree a StreamNode of IndexStream type.
Signed-off-by: NShreedhar Hardikar <shardikar@pivotal.io>

src/backend/access/bitmap/bitmap.c

@@ -35,6 +35,7 @@ static bool words_get_match(BMBatchWords *words, BMIterateResult *result,
 							bool newentry);
 static IndexScanDesc copy_scan_desc(IndexScanDesc scan);
 static void stream_free(StreamNode *self);
+static void indexstream_free(StreamNode *self);
 static bool pull_stream(StreamNode *self, PagetableEntry *e);
 static void cleanup_pos(BMScanPosition pos);
 
@@ -210,7 +211,7 @@ bmgetmulti(PG_FUNCTION_ARGS)
 		is->type = BMS_INDEX;
 		is->pull = pull_stream;
 		is->nextblock = 0;
-		is->free = stream_free;
+		is->free = indexstream_free;
 		is->set_instrument = NULL;
 		is->upd_instrument = NULL;
 
@@ -643,6 +644,16 @@ stream_free(StreamNode *self)
 	}
 }
 
+/*
+ * free the memory associated with an IndexStream
+ */
+static void
+indexstream_free(StreamNode *self) {
+	stream_free(self);
+
+	pfree(self);
+}
+
 static void
 cleanup_pos(BMScanPosition pos) 
 {

src/backend/executor/nodeBitmapAnd.c

@@ -109,6 +109,14 @@ ExecCountSlotsBitmapAnd(BitmapAnd *node)
 
 /* ----------------------------------------------------------------
  *	   MultiExecBitmapAnd
+ *
+ *	   BitmapAnd node gets the bitmaps generated from BitmapIndexScan
+ *	   nodes and outputs a bitmap that ANDs all input bitmaps.
+ *
+ *	   The first input bitmap is utilized to store the result of the
+ *	   AND and returned to the caller. In addition, the output points
+ *	   to a newly created OpStream node of type BMS_AND, where all
+ *	   StreamNodes of input bitmaps are added as input streams.
  * ----------------------------------------------------------------
  */
 Node *
@@ -203,6 +211,8 @@ MultiExecBitmapAnd(BitmapAndState *node)
 	   				StreamBitmap *s = (StreamBitmap *)subresult;
 		   			stream_add_node((StreamBitmap *)node->bitmap,
 									s->streamNode, BMS_AND);
+		   			/* node->bitmap should be the only owner of the newly created AND StreamNode */
+		   			s->streamNode = NULL;
 
 					/*
 					 * Don't free subresult here, as we are still using the StreamNode inside it.

src/backend/executor/nodeBitmapIndexscan.c

@@ -39,6 +39,19 @@
 
 /* ----------------------------------------------------------------
  *		MultiExecBitmapIndexScan(node)
+ *
+ *		If IndexScanState's iss_NumArrayKeys = 0, then BitmapIndexScan
+ *		node returns a StreamBitmap that stores all the interesting rows.
+ *		The returning StreamBitmap will point to an IndexStream that contains
+ *		pointers to functions for handling the output.
+ *
+ *		If IndexScanState's iss_NumArrayKeys > 0 (e.g., WHERE clause contains
+ *		`d in (0,1)` condition and a bitmap index has been created on column d),
+ *		then iss_NumArrayKeys StreamBitmaps will be created, where every bitmap
+ *		points to an individual IndexStream. BitmapIndexScan node
+ *		returns a StreamBitmap that ORs all the above StreamBitmaps. Also, the
+ *		returning StreamBitmap will point to an OpStream of type BMS_OR whose input
+ *		streams are the IndexStreams created for every array key.
  * ----------------------------------------------------------------
  */
 Node *

src/backend/executor/nodeBitmapOr.c

@@ -109,6 +109,14 @@ ExecCountSlotsBitmapOr(BitmapOr *node)
 
 /* ----------------------------------------------------------------
  *	   MultiExecBitmapOr
+ *
+ *	   BitmapOr node gets the bitmaps generated from BitmapIndexScan
+ *	   nodes and outputs a bitmap that ORs all input bitmaps.
+ *
+ *	   The first input bitmap is utilized to store the result of the
+ *	   OR and returned to the caller. In addition, the output points
+ *	   to a newly created OpStream node of type BMS_OR, where all
+ *	   StreamNodes of input bitmaps are added as input streams.
  * ----------------------------------------------------------------
  */
 Node *
@@ -181,6 +189,8 @@ MultiExecBitmapOr(BitmapOrState *node)
 					StreamBitmap *s = (StreamBitmap *)subresult;
 					stream_add_node((StreamBitmap *)node->bitmap, 
 									s->streamNode, BMS_OR);
+					/* node->bitmap should be the only owner of the newly created OR StreamNode */
+					s->streamNode = NULL;
 
 					/*
 					 * Don't free subresult here, as we are still using the StreamNode inside it.

src/test/regress/expected/bitmapscan.out

@@ -1005,3 +1005,38 @@ SELECT COUNT(DISTINCT(highCardinalityHighDomain)) AS distinct_hchd FROM card_hea
 (1 row)
 
 drop table if exists bm_test;
+-- Test if StreamNodes and StreamBitmaps are freed correctly.
+create table foo (c int, d int) distributed by (c);
+create index ie_foo on foo using bitmap(d);
+insert into foo values (1, 1);
+insert into foo values (2, 2);
+-- Next queries will create additional StreamNodes. In particular,
+-- d in (1, 2) will be transformed into an OR with two BMS_INDEX input streams.
+select * from foo where d in (1, 2) and (d = 1 or d = 2);
+ c | d 
+---+---
+ 1 | 1
+ 2 | 2
+(2 rows)
+
+select * from foo where (d = 1 or d = 2) and d in (1, 2);
+ c | d 
+---+---
+ 1 | 1
+ 2 | 2
+(2 rows)
+
+-- This query will eliminate StreamNodes since there is no tuple where d =3.
+select * from foo where d = 3 and (d = 1 or d = 2);
+ c | d 
+---+---
+(0 rows)
+
+-- If a segment contains tuples with d in (1, 2), then it will create the whole StreamNode tree, 
+-- otherwise segments will eliminate nodes.
+select * from foo where d = 2 and (d = 1 or d = 2);
+ c | d 
+---+---
+ 2 | 2
+(1 row)
+

src/test/regress/expected/bitmapscan_ao.out

@@ -193,6 +193,48 @@ select count(1) from bmcrash where bitmap_col = '999' AND btree_col1 = 'abcdefg9
     23
 (1 row)
 
+select count(1) from bmcrash where btree_col1 = 'abcdefg999' AND bitmap_col = '999';
+ count 
+-------
+    10
+(1 row)
+
+select count(1) from bmcrash where btree_col1 = 'abcdefg999' OR bitmap_col = '999' AND btree_col2 = '2015-01-01';
+ count 
+-------
+    10
+(1 row)
+
+select count(1) from bmcrash where btree_col1 = 'abcdefg999' AND bitmap_col = '999' OR btree_col2 = '2015-01-01';
+ count 
+-------
+    23
+(1 row)
+
+select count(1) from bmcrash where btree_col1 = 'abcdefg999' AND btree_col2 = '2015-01-01' OR bitmap_col = '999';
+ count 
+-------
+    10
+(1 row)
+
+select count(1) from bmcrash where btree_col1 = 'abcdefg999' AND btree_col2 = '2015-01-01' AND bitmap_col = '999';
+ count 
+-------
+     0
+(1 row)
+
+select count(1) from bmcrash where btree_col1 = 'abcdefg999' OR btree_col2 = '2015-01-01' AND bitmap_col = '999';
+ count 
+-------
+    10
+(1 row)
+
+select count(1) from bmcrash where btree_col1 = 'abcdefg999' OR btree_col2 = '2015-01-01' OR bitmap_col = '999';
+ count 
+-------
+    23
+(1 row)
+
 -- start_ignore
 drop schema bm_ao cascade;
 NOTICE:  drop cascades to append only table bmcrash

src/test/regress/sql/bitmapscan.sql

@@ -281,3 +281,22 @@ INSERT INTO card_heap_table_w_bitmap (id, v, highCardinalityHighDomain)
 SELECT COUNT(DISTINCT(highCardinalityHighDomain)) AS distinct_hchd FROM card_heap_table_w_bitmap ORDER BY distinct_hchd;
 
 drop table if exists bm_test;
+
+-- Test if StreamNodes and StreamBitmaps are freed correctly.
+create table foo (c int, d int) distributed by (c);
+create index ie_foo on foo using bitmap(d);
+insert into foo values (1, 1);
+insert into foo values (2, 2);
+
+-- Next queries will create additional StreamNodes. In particular,
+-- d in (1, 2) will be transformed into an OR with two BMS_INDEX input streams.
+select * from foo where d in (1, 2) and (d = 1 or d = 2);
+select * from foo where (d = 1 or d = 2) and d in (1, 2);
+
+-- This query will eliminate StreamNodes since there is no tuple where d =3.
+select * from foo where d = 3 and (d = 1 or d = 2);
+
+-- If a segment contains tuples with d in (1, 2), then it will create the whole StreamNode tree, 
+-- otherwise segments will eliminate nodes.
+select * from foo where d = 2 and (d = 1 or d = 2);
+

src/test/regress/sql/bitmapscan_ao.sql

@@ -173,6 +173,13 @@ select count(1) from bmcrash where bitmap_col = '999' OR btree_col1 = 'abcdefg99
 select count(1) from bmcrash where bitmap_col = '999' OR btree_col1 = 'abcdefg999' AND btree_col2 = '2015-01-01';
 select count(1) from bmcrash where bitmap_col = '999' AND btree_col1 = 'abcdefg999' OR btree_col2 = '2015-01-01';
 
+select count(1) from bmcrash where btree_col1 = 'abcdefg999' AND bitmap_col = '999';
+select count(1) from bmcrash where btree_col1 = 'abcdefg999' OR bitmap_col = '999' AND btree_col2 = '2015-01-01';
+select count(1) from bmcrash where btree_col1 = 'abcdefg999' AND bitmap_col = '999' OR btree_col2 = '2015-01-01';
+select count(1) from bmcrash where btree_col1 = 'abcdefg999' AND btree_col2 = '2015-01-01' OR bitmap_col = '999';
+select count(1) from bmcrash where btree_col1 = 'abcdefg999' AND btree_col2 = '2015-01-01' AND bitmap_col = '999';
+select count(1) from bmcrash where btree_col1 = 'abcdefg999' OR btree_col2 = '2015-01-01' AND bitmap_col = '999';
+select count(1) from bmcrash where btree_col1 = 'abcdefg999' OR btree_col2 = '2015-01-01' OR bitmap_col = '999';
 
 -- start_ignore
 drop schema bm_ao cascade;