Fix double initialization of TsFdwRelInfo

This doesn't cause any problems except degrading the performance.

The TsFdwRelInfo is initialized twice for hypertable on access node:

1) when using it for chunk size estimation in `estimate_chunk_size`,
2) when expanding the distributed hypertable in `tsl_set_rel_pathlist`
callback.

The initialization has to happen inside the TSL module, this is why we
can't easily move it up the call graph to, say,
`import/ts_set_append_rel_size`, which is FDW-agnostic. As a fix, just
accept that TimescaleDBPrivate structure can be allocated already, and
don't discard the previous one.

src/import/allpaths.c

@@ -497,7 +497,7 @@ set_rel_consider_parallel(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte
 
 /* copied from allpaths.c */
 static void
-set_append_rel_size(PlannerInfo *root, RelOptInfo *rel, Index rti, RangeTblEntry *rte)
+ts_set_append_rel_size(PlannerInfo *root, RelOptInfo *rel, Index rti, RangeTblEntry *rte)
 {
 	int parentRTindex = rti;
 	bool has_live_children;
@@ -853,7 +853,7 @@ ts_set_rel_size(PlannerInfo *root, RelOptInfo *rel, Index rti, RangeTblEntry *rt
 	else if (rte->inh)
 	{
 		/* It's an "append relation", process accordingly */
-		set_append_rel_size(root, rel, rti, rte);
+		ts_set_append_rel_size(root, rel, rti, rte);
 	}
 	else
 	{

src/planner.c

@@ -709,7 +709,10 @@ reenable_inheritance(PlannerInfo *root, RelOptInfo *rel, Index rti, RangeTblEntr
 			 */
 			if (in_rel->reloptkind == RELOPT_BASEREL ||
 				in_rel->reloptkind == RELOPT_OTHER_MEMBER_REL)
+			{
+				Assert(in_rte->relkind == RELKIND_RELATION);
 				ts_set_rel_size(root, in_rel, i, in_rte);
+			}
 
 			/* if we're activating inheritance during a hypertable's pathlist
 			 * creation then we're past the point at which postgres will add

src/planner.h

@@ -14,7 +14,7 @@
 #include "export.h"
 #include "guc.h"
 
-typedef struct TsFdwRelationInfo TsFdwRelationInfo;
+typedef struct TsFdwRelInfo TsFdwRelInfo;
 typedef struct TimescaleDBPrivate
 {
 	bool appends_ordered;
@@ -25,7 +25,7 @@ typedef struct TimescaleDBPrivate
 	List *chunk_oids;
 	List *serverids;
 	Relids server_relids;
-	TsFdwRelationInfo *fdw_relation_info;
+	TsFdwRelInfo *fdw_relation_info;
 } TimescaleDBPrivate;
 
 extern TSDLLEXPORT bool ts_rte_is_hypertable(const RangeTblEntry *rte, bool *isdistributed);

tsl/src/fdw/relinfo.c

@@ -85,35 +85,33 @@ apply_fdw_and_server_options(TsFdwRelInfo *fpinfo)
 TsFdwRelInfo *
 fdw_relinfo_get(RelOptInfo *rel)
 {
-	TimescaleDBPrivate *rel_private;
+	TimescaleDBPrivate *rel_private = rel->fdw_private;
+	TsFdwRelInfo *fdw_relation_info = rel_private->fdw_relation_info;
 
-	if (!rel->fdw_private)
+	/*
-		ts_create_private_reloptinfo(rel);
+	 * This function is expected to return either null or a fully initialized
+	 * fdw_relation_info struct.
+	 */
+	Assert(!fdw_relation_info || fdw_relation_info->type != TS_FDW_RELINFO_UNINITIALIZED);
 
-	rel_private = rel->fdw_private;
+	return fdw_relation_info;
-
-	if (!rel_private->fdw_relation_info)
-		rel_private->fdw_relation_info = palloc0(sizeof(TsFdwRelInfo));
-
-	return (TsFdwRelInfo *) rel_private->fdw_relation_info;
 }
 
 TsFdwRelInfo *
-fdw_relinfo_alloc(RelOptInfo *rel, TsFdwRelInfoType reltype)
+fdw_relinfo_alloc_or_get(RelOptInfo *rel)
 {
-	TimescaleDBPrivate *rel_private;
+	TimescaleDBPrivate *rel_private = rel->fdw_private;
-	TsFdwRelInfo *fpinfo;
+	if (rel_private == NULL)
+	{
+		rel_private = ts_create_private_reloptinfo(rel);
+	}
 
-	if (NULL == rel->fdw_private)
+	if (rel_private->fdw_relation_info == NULL)
-		ts_create_private_reloptinfo(rel);
+	{
+		rel_private->fdw_relation_info = (TsFdwRelInfo *) palloc0(sizeof(TsFdwRelInfo));
+	}
 
-	rel_private = rel->fdw_private;
+	return rel_private->fdw_relation_info;
-
-	fpinfo = (TsFdwRelInfo *) palloc0(sizeof(*fpinfo));
-	rel_private->fdw_relation_info = (void *) fpinfo;
-	fpinfo->type = reltype;
-
-	return fpinfo;
 }
 
 static const double FILL_FACTOR_CURRENT_CHUNK = 0.5;
@@ -297,7 +295,12 @@ estimate_chunk_size(PlannerInfo *root, RelOptInfo *chunk_rel)
 	}
 
 	RelOptInfo *parent_info = root->simple_rel_array[parent_relid];
-	TsFdwRelInfo *parent_private = fdw_relinfo_get(parent_info);
+	/*
+	 * The parent FdwRelInfo might not be allocated and initialized here, because
+	 * it happens later in tsl_set_pathlist callback. We don't care about this
+	 * because we only need it for chunk size estimates, so allocate it ourselves.
+	 */
+	TsFdwRelInfo *parent_private = fdw_relinfo_alloc_or_get(parent_info);
 	RangeTblEntry *parent_rte = planner_rt_fetch(parent_relid, root);
 	Cache *hcache = ts_hypertable_cache_pin();
 	Hypertable *ht = ts_hypertable_cache_get_entry(hcache, parent_rte->relid, CACHE_FLAG_NONE);
@@ -378,9 +381,13 @@ fdw_relinfo_create(PlannerInfo *root, RelOptInfo *rel, Oid server_oid, Oid local
 
 	/*
 	 * We use TsFdwRelInfo to pass various information to subsequent
-	 * functions.
+	 * functions. It might be already partially initialized for a data node
+	 * hypertable, because we use it to maintain the chunk size estimates when
+	 * planning.
 	 */
-	fpinfo = fdw_relinfo_alloc(rel, type);
+	fpinfo = fdw_relinfo_alloc_or_get(rel);
+	Assert(fpinfo->type == TS_FDW_RELINFO_UNINITIALIZED || fpinfo->type == type);
+	fpinfo->type = type;
 
 	/*
 	 * Set the name of relation in fpinfo, while we are constructing it here.

tsl/src/fdw/relinfo.h

@@ -25,7 +25,7 @@
 
 typedef enum
 {
-	TS_FDW_RELINFO_UNKNOWN = 0,
+	TS_FDW_RELINFO_UNINITIALIZED = 0,
 	TS_FDW_RELINFO_HYPERTABLE_DATA_NODE,
 	TS_FDW_RELINFO_HYPERTABLE,
 	TS_FDW_RELINFO_FOREIGN_TABLE,
@@ -147,7 +147,7 @@ typedef struct TsFdwRelInfo
 
 extern TsFdwRelInfo *fdw_relinfo_create(PlannerInfo *root, RelOptInfo *rel, Oid server_oid,
 										Oid local_table_id, TsFdwRelInfoType type);
-extern TsFdwRelInfo *fdw_relinfo_alloc(RelOptInfo *rel, TsFdwRelInfoType reltype);
+extern TsFdwRelInfo *fdw_relinfo_alloc_or_get(RelOptInfo *rel);
 extern TsFdwRelInfo *fdw_relinfo_get(RelOptInfo *rel);
 
 #endif /* TIMESCALEDB_TSL_FDW_RELINFO_H */

tsl/src/fdw/scan_plan.c

@@ -949,6 +949,8 @@ fdw_create_upper_paths(TsFdwRelInfo *input_fpinfo, PlannerInfo *root, UpperRelat
 {
 	Assert(input_fpinfo != NULL);
 
+	TsFdwRelInfo *output_fpinfo = NULL;
+
 	/*
 	 * If input rel is not safe to pushdown, then simply return as we cannot
 	 * perform any post-join operations on the data node.
@@ -965,8 +967,9 @@ fdw_create_upper_paths(TsFdwRelInfo *input_fpinfo, PlannerInfo *root, UpperRelat
 	{
 		case UPPERREL_GROUP_AGG:
 		case UPPERREL_PARTIAL_GROUP_AGG:
-			input_fpinfo = fdw_relinfo_alloc(output_rel, input_fpinfo->type);
+			output_fpinfo = fdw_relinfo_alloc_or_get(output_rel);
-			input_fpinfo->pushdown_safe = false;
+			output_fpinfo->type = input_fpinfo->type;
+			output_fpinfo->pushdown_safe = false;
 			add_foreign_grouping_paths(root,
 									   input_rel,
 									   output_rel,

tsl/src/planner.c

@@ -133,9 +133,10 @@ tsl_set_rel_pathlist_dml(PlannerInfo *root, RelOptInfo *rel, Index rti, RangeTbl
 	}
 }
 
-/* The fdw needs to expand a distributed hypertable inside the `GetForeignPath` callback. But, since
+/*
- * the hypertable base table is not a foreign table, that callback would not normally be called.
+ * The fdw needs to expand a distributed hypertable inside the `GetForeignPath`
- * Thus, we call it manually in this hook.
+ * callback. But, since the hypertable base table is not a foreign table, that
+ * callback would not normally be called. Thus, we call it manually in this hook.
  */
 void
 tsl_set_rel_pathlist(PlannerInfo *root, RelOptInfo *rel, Index rti, RangeTblEntry *rte)