Refactor code a bit.

QueryDocument() adds the filters to the MongoDB query.  However, for
MongoAcquireSampleRows(), there are no quals and thus NO need to call
QueryDocument().  Fix that. Also, the list_concat() function has
changed in v13, so add a mongo_list_concat() wrapper which calls the
correct form depending on the version.

Along the way, improve some comments and indentation.

FDW-138, Vaibhav Dalvi, reviewed by Suraj Kharage and Jeevan Chalke,
with additional improvements by me.

mongo_fdw.c

@@ -413,9 +413,9 @@ MongoGetForeignPlan(PlannerInfo *root,
 					Plan *outer_plan)
 {
 	MongoFdwRelationInfo *fpinfo = (MongoFdwRelationInfo *) foreignrel->fdw_private;
-	Index		scanRangeTableIndex = foreignrel->relid;
+	Index		scan_relid = foreignrel->relid;
 	ForeignScan *foreignScan;
-	List	   *foreignPrivateList;
+	List	   *fdw_private;
 	List	   *columnList;
 	List	   *scan_var_list;
 	ListCell   *lc;
@@ -492,13 +492,13 @@ MongoGetForeignPlan(PlannerInfo *root,
 	columnList = mongo_get_column_list(root, foreignrel, scan_var_list);
 
 	/* Construct foreign plan with query document and column list */
-	foreignPrivateList = list_make2(columnList, remote_exprs);
+	fdw_private = list_make2(columnList, remote_exprs);
 
 	/* Create the foreign scan node */
 	foreignScan = make_foreignscan(targetList, local_exprs,
-								   scanRangeTableIndex,
+								   scan_relid,
 								   NIL, /* No expressions to evaluate */
-								   foreignPrivateList
+								   fdw_private
 #if PG_VERSION_NUM >= 90500
 								   ,NIL
 								   ,NIL
@@ -680,7 +680,7 @@ MongoIterateForeignScan(ForeignScanState *node)
 	}
 
 	/*
-	 * We execute the protocol to load a virtual tuple into a slot. We first
+	 * We execute the protocol to load a virtual tuple into a slot.  We first
 	 * call ExecClearTuple, then fill in values / isnull arrays, and last call
 	 * ExecStoreVirtualTuple.  If we are done fetching documents from Mongo,
 	 * we just return an empty slot as required.
@@ -1293,8 +1293,6 @@ ColumnMappingHash(Oid foreignTableId, List *columnList)
 	ListCell   *columnCell;
 	const long	hashTableSize = 2048;
 	HTAB	   *columnMappingHash;
-
-	/* Create hash table */
 	HASHCTL		hashInfo;
 
 	memset(&hashInfo, 0, sizeof(hashInfo));
@@ -1313,7 +1311,7 @@ ColumnMappingHash(Oid foreignTableId, List *columnList)
 		Var		   *column = (Var *) lfirst(columnCell);
 		AttrNumber	columnId = column->varattno;
 		ColumnMapping *columnMapping;
-		char	   *columnName = NULL;
+		char	   *columnName;
 		bool		handleFound = false;
 		void	   *hashKey;
 
@@ -1581,7 +1579,7 @@ ColumnTypesCompatible(BSON_TYPE bsonType, Oid columnTypeId)
 			break;
 		default:
 			/*
-			 * We currently error out on other data types. Some types such as
+			 * We currently error out on other data types.  Some types such as
 			 * byte arrays are easy to add, but they need testing.
 			 *
 			 * Other types such as money or inet, do not have equivalents in
@@ -2152,7 +2150,7 @@ MongoAcquireSampleRows(Relation relation,
 	AttrNumber	columnId;
 	HTAB	   *columnMappingHash;
 	MONGO_CURSOR *mongoCursor;
-	BSON	   *queryDocument;
+	BSON	   *queryDocument = BsonCreate();
 	List	   *columnList = NIL;
 	char	   *relationName;
 	MemoryContext oldContext = CurrentMemoryContext;
@@ -2197,7 +2195,19 @@ MongoAcquireSampleRows(Relation relation,
 	 */
 	mongoConnection = mongo_get_connection(server, user, options);
 
-	queryDocument = QueryDocument(foreignTableId, NIL, NULL);
+	if (!BsonFinish(queryDocument))
+	{
+#ifdef META_DRIVER
+		ereport(ERROR,
+				(errmsg("could not create document for query"),
+				 errhint("BSON flags: %d", queryDocument->flags)));
+#else
+		ereport(ERROR,
+				(errmsg("could not create document for query"),
+				 errhint("BSON error: %d", queryDocument->err)));
+#endif
+	}
+
 	/* Create cursor for collection name and set query */
 	mongoCursor = MongoCursorCreate(mongoConnection, options->svr_database,
 									options->collectionName, queryDocument);

mongo_fdw.h

@@ -170,6 +170,13 @@
 #define POSTGRES_TO_UNIX_EPOCH_DAYS 		(POSTGRES_EPOCH_JDATE - UNIX_EPOCH_JDATE)
 #define POSTGRES_TO_UNIX_EPOCH_USECS 		(POSTGRES_TO_UNIX_EPOCH_DAYS * USECS_PER_DAY)
 
+/* Macro for list API backporting. */
+#if PG_VERSION_NUM < 130000
+	#define mongo_list_concat(l1, l2) list_concat(l1, list_copy(l2))
+#else
+	#define mongo_list_concat(l1, l2) list_concat((l1), (l2))
+#endif
+
 /*
  * MongoValidOption keeps an option name and a context.  When an option is
  * passed into mongo_fdw objects (server and foreign table), we compare this
@@ -268,10 +275,10 @@ typedef struct MongoFdwModifyState
 } MongoFdwModifyState;
 
 /*
- * ColumnMapping reprents a hash table entry that maps a column name to column
- * related information.  We construct these hash table entries to speed up the
- * conversion from BSON documents to PostgreSQL tuples; and each hash entry
- * maps the column name to the column's tuple index and its type-related
+ * ColumnMapping represents a hash table entry that maps a column name to
+ * column-related information.  We construct these hash table entries to speed
+ * up the conversion from BSON documents to PostgreSQL tuples, and each hash
+ * entry maps the column name to the column's tuple index and its type-related
  * information.
  */
 typedef struct ColumnMapping

mongo_query.c

@@ -80,6 +80,7 @@ static void AppendConstantValue(BSON *queryDocument, const char *keyName,
 static void AppendParamValue(BSON *queryDocument, const char *keyName,
 							 Param *paramNode,
 							 ForeignScanState *scanStateNode);
+static char *MongoOperatorName(const char *operatorName);
 static bool foreign_expr_walker(Node *node,
 								foreign_glob_cxt *glob_cxt,
 								foreign_loc_cxt *outer_cxt);
@@ -249,7 +250,7 @@ QueryDocument(Oid relationId, List *opExpressionList,
  * 		Takes in the given PostgreSQL comparison operator name, and returns its
  * 		equivalent in MongoDB.
  */
-char *
+static char *
 MongoOperatorName(const char *operatorName)
 {
 	const char *mongoOperatorName = NULL;
@@ -710,7 +711,7 @@ mongo_get_column_list(PlannerInfo *root, RelOptInfo *foreignrel,
 			 * relation.
 			 */
 			attrs_used = bms_make_singleton(0 -
-										 FirstLowInvalidHeapAttributeNumber);
+											FirstLowInvalidHeapAttributeNumber);
 
 			wr_var_list = prepare_var_list_for_baserel(rte->relid, var->varno,
 													   attrs_used);
@@ -1033,7 +1034,7 @@ mongo_is_foreign_expr(PlannerInfo *root, RelOptInfo *baserel, Expr *expression)
 	if (!foreign_expr_walker((Node *) expression, &glob_cxt, &loc_cxt))
 		return false;
 
-	/* Expressions examined here should be boolean, ie noncollatable */
+	/* Expressions examined here should be boolean, i.e. noncollatable */
 	Assert(loc_cxt.collation == InvalidOid);
 	Assert(loc_cxt.state == FDW_COLLATE_NONE);
 

mongo_query.h

@@ -17,12 +17,8 @@
 
 #define NUMERICARRAY_OID 1231
 
-bool AppendMongoValue(BSON *queryDocument,
-					  const char *keyName,
-					  Datum value,
-					  bool isnull,
-					  Oid id);
-
-char *MongoOperatorName(const char *operatorName);
+/* Function to be used in mongo_fdw.c */
+extern bool AppendMongoValue(BSON *queryDocument, const char *keyName,
+							 Datum value, bool isnull, Oid id);
 
 #endif							/* MONGO_QUERY_H */

option.c

@@ -156,9 +156,9 @@ mongo_get_options(Oid foreignTableId)
 	foreignServer = GetForeignServer(foreignTable->serverid);
 	mapping = GetUserMapping(GetUserId(), foreignTable->serverid);
 
-	optionList = list_concat(optionList, foreignTable->options);
-	optionList = list_concat(optionList, foreignServer->options);
-	optionList = list_concat(optionList, mapping->options);
+	optionList = mongo_list_concat(optionList, foreignTable->options);
+	optionList = mongo_list_concat(optionList, foreignServer->options);
+	optionList = mongo_list_concat(optionList, mapping->options);
 
 	options = (MongoFdwOptions *) palloc0(sizeof(MongoFdwOptions));