Commit 77ba6108 authored by Tom Lane's avatar Tom Lane

Revert "Use Foreign Key relationships to infer multi-column join selectivity".

This commit reverts 137805f8 as well as the associated commits 015e8894,
5306df28, and 68d704ed.  We found multiple bugs in this feature, and
there was concern about possible planner slowdown (though to be fair,
exhibiting a very large slowdown proved difficult).  The way forward
requires a considerable rewrite, which may or may not be possible to
accomplish in time for beta2.  In my judgment reviewing the rewrite will
be easier to accomplish starting from a clean slate, so let's temporarily
revert what's there now.  This also leaves us in a safe state if it turns
out to be necessary to postpone the rewrite to the next development cycle.

Discussion: <20160429102531.GA13701@huehner.biz>
parent 5c6d2a5e
......@@ -2137,16 +2137,6 @@ _outIndexOptInfo(StringInfo str, const IndexOptInfo *node)
/* we don't bother with fields copied from the index AM's API struct */
}
static void
_outForeignKeyOptInfo(StringInfo str, const ForeignKeyOptInfo *node)
{
WRITE_NODE_TYPE("FOREIGNKEYOPTINFO");
WRITE_OID_FIELD(conrelid);
WRITE_OID_FIELD(confrelid);
WRITE_INT_FIELD(nkeys);
}
static void
_outEquivalenceClass(StringInfo str, const EquivalenceClass *node)
{
......@@ -3617,9 +3607,6 @@ outNode(StringInfo str, const void *obj)
case T_IndexOptInfo:
_outIndexOptInfo(str, obj);
break;
case T_ForeignKeyOptInfo:
_outForeignKeyOptInfo(str, obj);
break;
case T_EquivalenceClass:
_outEquivalenceClass(str, obj);
break;
......
This diff is collapsed.
......@@ -28,7 +28,6 @@
#include "catalog/dependency.h"
#include "catalog/heap.h"
#include "catalog/pg_am.h"
#include "catalog/pg_constraint.h"
#include "foreign/fdwapi.h"
#include "miscadmin.h"
#include "nodes/makefuncs.h"
......@@ -42,7 +41,6 @@
#include "rewrite/rewriteManip.h"
#include "storage/bufmgr.h"
#include "utils/lsyscache.h"
#include "utils/syscache.h"
#include "utils/rel.h"
#include "utils/snapmgr.h"
......@@ -96,9 +94,6 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
Relation relation;
bool hasindex;
List *indexinfos = NIL;
List *fkinfos = NIL;
List *fkoidlist;
ListCell *l;
/*
* We need not lock the relation since it was already locked, either by
......@@ -149,6 +144,7 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
if (hasindex)
{
List *indexoidlist;
ListCell *l;
LOCKMODE lmode;
indexoidlist = RelationGetIndexList(relation);
......@@ -395,85 +391,6 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
rel->indexlist = indexinfos;
/*
* Load foreign key data. Note this is the definitional data from the
* catalog only and does not lock the referenced tables here. The
* precise definition of the FK is important and may affect the usage
* elsewhere in the planner, e.g. if the constraint is deferred or
* if the constraint is not valid then relying upon this in the executor
* may not be accurate, though might be considered a useful estimate for
* planning purposes.
*/
fkoidlist = RelationGetFKeyList(relation);
foreach(l, fkoidlist)
{
Oid fkoid = lfirst_oid(l);
HeapTuple htup;
Form_pg_constraint constraint;
ForeignKeyOptInfo *info;
Datum adatum;
bool isnull;
ArrayType *arr;
int numkeys;
int i;
htup = SearchSysCache1(CONSTROID, ObjectIdGetDatum(fkoid));
if (!HeapTupleIsValid(htup)) /* should not happen */
elog(ERROR, "cache lookup failed for constraint %u", fkoid);
constraint = (Form_pg_constraint) GETSTRUCT(htup);
Assert(constraint->contype == CONSTRAINT_FOREIGN);
info = makeNode(ForeignKeyOptInfo);
info->conrelid = constraint->conrelid;
info->confrelid = constraint->confrelid;
/* conkey */
adatum = SysCacheGetAttr(CONSTROID, htup,
Anum_pg_constraint_conkey, &isnull);
Assert(!isnull);
arr = DatumGetArrayTypeP(adatum);
numkeys = ARR_DIMS(arr)[0];
info->conkeys = (int*)palloc(numkeys * sizeof(int));
for (i = 0; i < numkeys; i++)
info->conkeys[i] = ((int16 *) ARR_DATA_PTR(arr))[i];
/* confkey */
adatum = SysCacheGetAttr(CONSTROID, htup,
Anum_pg_constraint_confkey, &isnull);
Assert(!isnull);
arr = DatumGetArrayTypeP(adatum);
Assert(numkeys == ARR_DIMS(arr)[0]);
info->confkeys = (int*)palloc(numkeys * sizeof(int));
for (i = 0; i < numkeys; i++)
info->confkeys[i] = ((int16 *) ARR_DATA_PTR(arr))[i];
/* conpfeqop */
adatum = SysCacheGetAttr(CONSTROID, htup,
Anum_pg_constraint_conpfeqop, &isnull);
Assert(!isnull);
arr = DatumGetArrayTypeP(adatum);
Assert(numkeys == ARR_DIMS(arr)[0]);
info->conpfeqop = (Oid*)palloc(numkeys * sizeof(Oid));
for (i = 0; i < numkeys; i++)
info->conpfeqop[i] = ((Oid *) ARR_DATA_PTR(arr))[i];
info->nkeys = numkeys;
ReleaseSysCache(htup);
fkinfos = lappend(fkinfos, info);
}
list_free(fkoidlist);
rel->fkeylist = fkinfos;
/* Grab foreign-table info using the relcache, while we have it */
if (relation->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
{
......
......@@ -2031,7 +2031,6 @@ RelationDestroyRelation(Relation relation, bool remember_tupdesc)
FreeTupleDesc(relation->rd_att);
}
list_free(relation->rd_indexlist);
list_free(relation->rd_fkeylist);
bms_free(relation->rd_indexattr);
bms_free(relation->rd_keyattr);
bms_free(relation->rd_idattr);
......@@ -3957,79 +3956,6 @@ RelationGetIndexList(Relation relation)
return result;
}
/*
* RelationGetFKeyList -- get a list of foreign key oids
*
* Use an index scan on pg_constraint to load in FK definitions,
* intended for use within the planner, not for enforcing FKs.
*
* Data is ordered by Oid, though this is not critical at this point
* since we do not lock the referenced relations.
*/
List *
RelationGetFKeyList(Relation relation)
{
Relation conrel;
SysScanDesc conscan;
ScanKeyData skey;
HeapTuple htup;
List *result;
List *oldlist;
MemoryContext oldcxt;
/* Quick exit if we already computed the list. */
if (relation->rd_fkeylist)
return list_copy(relation->rd_fkeylist);
/* Fast path if no FKs... if it doesn't have a trigger, it can't have a FK */
if (!relation->rd_rel->relhastriggers)
return NIL;
/*
* We build the list we intend to return (in the caller's context) while
* doing the scan. After successfully completing the scan, we copy that
* list into the relcache entry. This avoids cache-context memory leakage
* if we get some sort of error partway through.
*/
result = NIL;
/* Prepare to scan pg_constraint for entries having conrelid = this rel. */
ScanKeyInit(&skey,
Anum_pg_constraint_conrelid,
BTEqualStrategyNumber, F_OIDEQ,
ObjectIdGetDatum(RelationGetRelid(relation)));
conrel = heap_open(ConstraintRelationId, AccessShareLock);
conscan = systable_beginscan(conrel, ConstraintRelidIndexId, true,
NULL, 1, &skey);
while (HeapTupleIsValid(htup = systable_getnext(conscan)))
{
Form_pg_constraint constraint = (Form_pg_constraint) GETSTRUCT(htup);
/* return only foreign keys */
if (constraint->contype != CONSTRAINT_FOREIGN)
continue;
/* Add FK's OID to result list in the proper order */
result = insert_ordered_oid(result, HeapTupleGetOid(htup));
}
systable_endscan(conscan);
heap_close(conrel, AccessShareLock);
/* Now save a copy of the completed list in the relcache entry. */
oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
oldlist = relation->rd_fkeylist;
relation->rd_fkeylist = list_copy(result);
MemoryContextSwitchTo(oldcxt);
/* Don't leak the old list, if there is one */
list_free(oldlist);
return result;
}
/*
* insert_ordered_oid
* Insert a new Oid into a sorted list of Oids, preserving ordering
......@@ -4994,7 +4920,6 @@ load_relcache_init_file(bool shared)
rel->rd_indexattr = NULL;
rel->rd_keyattr = NULL;
rel->rd_idattr = NULL;
rel->rd_fkeylist = NIL;
rel->rd_createSubid = InvalidSubTransactionId;
rel->rd_newRelfilenodeSubid = InvalidSubTransactionId;
rel->rd_amcache = NULL;
......
......@@ -877,15 +877,6 @@ static struct config_bool ConfigureNamesBool[] =
true,
NULL, NULL, NULL
},
{
{"enable_fkey_estimates", PGC_USERSET, QUERY_TUNING_METHOD,
gettext_noop("Enables use of foreign keys for estimating joins."),
NULL
},
&enable_fkey_estimates,
true,
NULL, NULL, NULL
},
{
{"geqo", PGC_USERSET, QUERY_TUNING_GEQO,
......
......@@ -223,7 +223,6 @@ typedef enum NodeTag
T_PlannerGlobal,
T_RelOptInfo,
T_IndexOptInfo,
T_ForeignKeyOptInfo,
T_ParamPathInfo,
T_Path,
T_IndexPath,
......
......@@ -516,7 +516,6 @@ typedef struct RelOptInfo
List *lateral_vars; /* LATERAL Vars and PHVs referenced by rel */
Relids lateral_referencers; /* rels that reference me laterally */
List *indexlist; /* list of IndexOptInfo */
List *fkeylist; /* list of ForeignKeyOptInfo */
BlockNumber pages; /* size estimates derived from pg_class */
double tuples;
double allvisfrac;
......@@ -623,27 +622,6 @@ typedef struct IndexOptInfo
void (*amcostestimate) (); /* AM's cost estimator */
} IndexOptInfo;
/*
* ForeignKeyOptInfo
* Per-foreign-key information for planning/optimization
*
* Only includes columns from pg_constraint related to foreign keys.
*
* conkeys[], confkeys[] and conpfeqop[] each have nkeys entries.
*/
typedef struct ForeignKeyOptInfo
{
NodeTag type;
Oid conrelid; /* relation constrained by the foreign key */
Oid confrelid; /* relation referenced by the foreign key */
int nkeys; /* number of columns in the foreign key */
int *conkeys; /* attnums of columns in the constrained table */
int *confkeys; /* attnums of columns in the referenced table */
Oid *conpfeqop; /* OIDs of equality operators used by the FK */
} ForeignKeyOptInfo;
/*
* EquivalenceClasses
......
......@@ -66,7 +66,6 @@ extern bool enable_nestloop;
extern bool enable_material;
extern bool enable_mergejoin;
extern bool enable_hashjoin;
extern bool enable_fkey_estimates;
extern int constraint_exclusion;
extern double clamp_row_est(double nrows);
......
......@@ -76,8 +76,6 @@ extern Expr *adjust_rowcompare_for_index(RowCompareExpr *clause,
int indexcol,
List **indexcolnos,
bool *var_on_left_p);
extern bool has_matching_fkey(RelOptInfo *rel, RelOptInfo *frel, List *clauses,
bool reverse);
/*
* tidpath.h
......
......@@ -95,9 +95,6 @@ typedef struct RelationData
Oid rd_oidindex; /* OID of unique index on OID, if any */
Oid rd_replidindex; /* OID of replica identity index, if any */
/* data managed by RelationGetFKList: */
List *rd_fkeylist; /* OIDs of foreign keys */
/* data managed by RelationGetIndexAttrBitmap: */
Bitmapset *rd_indexattr; /* identifies columns used in indexes */
Bitmapset *rd_keyattr; /* cols that can be ref'd by foreign keys */
......
......@@ -38,7 +38,6 @@ extern void RelationClose(Relation relation);
* Routines to compute/retrieve additional cached information
*/
extern List *RelationGetIndexList(Relation relation);
extern List *RelationGetFKeyList(Relation relation);
extern Oid RelationGetOidIndex(Relation relation);
extern Oid RelationGetReplicaIndex(Relation relation);
extern List *RelationGetIndexExpressions(Relation relation);
......
SELECT name, setting FROM pg_settings WHERE name LIKE 'enable%';
name | setting
-----------------------+---------
enable_bitmapscan | on
enable_fkey_estimates | on
enable_hashagg | on
enable_hashjoin | on
enable_indexonlyscan | on
enable_indexscan | on
enable_material | on
enable_mergejoin | on
enable_nestloop | on
enable_seqscan | on
enable_sort | on
enable_tidscan | on
(12 rows)
name | setting
----------------------+---------
enable_bitmapscan | on
enable_hashagg | on
enable_hashjoin | on
enable_indexonlyscan | on
enable_indexscan | on
enable_material | on
enable_mergejoin | on
enable_nestloop | on
enable_seqscan | on
enable_sort | on
enable_tidscan | on
(11 rows)
CREATE TABLE foo2(fooid int, f2 int);
INSERT INTO foo2 VALUES(1, 11);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment