Re: Import Statistics in postgres_fdw before resorting to sampling.

Hi, thanks for working on this. Generally I think that this is a good
idea to avoid fetching rows from a foreign table to compute statistics
that may already be available on the foreign server.

I started reviewing the v7 patch and here are my initial comments. I
still want to do another round of review and run some more tests.

+		if (fdwroutine->ImportStatistics != NULL &&
+			fdwroutine->StatisticsAreImportable != NULL &&
+			fdwroutine->StatisticsAreImportable(onerel))
+			import_stats = true;
+		else
+		{
+			if (fdwroutine->AnalyzeForeignTable != NULL)
+				ok = fdwroutine->AnalyzeForeignTable(onerel,
+													 &acquirefunc,
+													 &relpages);
+
+			if (!ok)
+			{
+				ereport(WARNING,
+						errmsg("skipping \"%s\" -- cannot analyze this foreign table.",
+							   RelationGetRelationName(onerel)));
+				relation_close(onerel, ShareUpdateExclusiveLock);
+				return;
+			}
+		}
+
 		if (fdwroutine->AnalyzeForeignTable != NULL)
 			ok = fdwroutine->AnalyzeForeignTable(onerel,
 												 &acquirefunc,
												 &relpages);

		if (!ok)
		{
			ereport(WARNING,
					(errmsg("skipping \"%s\" --- cannot analyze this foreign table",
							RelationGetRelationName(onerel))));
			relation_close(onerel, ShareUpdateExclusiveLock);
			return;
		}

It seems that we have the same code within the else branch after the if/else
check, is this correct?

---

+	 * Every row of the result should be an attribute that we specificially

s/specificially/specifically

---

+		if (TupleDescAttr(tupdesc, i)->attisdropped)
+			continue;
+
+		/* Ignore generated columns. */
+		if (TupleDescAttr(tupdesc, i)->attgenerated)
+			continue;
+
+		attname = NameStr(TupleDescAttr(tupdesc, i)->attname);

Wouldn't be better to call TupleDescAttr a single time and save the value?
		Form_pg_attribute attr = TupleDescAttr(tupdesc, i - 1);

		/* Ignore dropped columns. */
		if (attr->attisdropped)
			continue;
		...

--
Matheus Alcantara
EDB: https://www.enterprisedb.com