From 278cb4341103e967189997985b09981a73e23a34 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 24 Jul 2017 11:23:27 -0400 Subject: [PATCH] Be more consistent about errors for opfamily member lookup failures. Add error checks in some places that were calling get_opfamily_member or get_opfamily_proc and just assuming that the call could never fail. Also, standardize the wording for such errors in some other places. None of these errors are expected in normal use, hence they're just elog not ereport. But they may be handy for diagnosing omissions in custom opclasses. Rushabh Lathia found the oversight in RelationBuildPartitionKey(); I found the others by grepping for all callers of these functions. Discussion: https://postgr.es/m/CAGPqQf2R9Nk8htpv0FFi+FP776EwMyGuORpc9zYkZKC8sFQE3g@mail.gmail.com --- src/backend/catalog/index.c | 4 ++++ src/backend/catalog/partition.c | 7 ++++--- src/backend/executor/execExpr.c | 3 +++ src/backend/executor/execReplication.c | 3 +-- src/backend/executor/nodeIndexscan.c | 3 +++ src/backend/optimizer/path/indxpath.c | 4 ++-- src/backend/optimizer/path/pathkeys.c | 4 ++-- src/backend/optimizer/plan/createplan.c | 7 ++++--- src/backend/utils/cache/relcache.c | 4 ++++ 9 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 027abd56b0..d25b39bb54 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -1738,6 +1738,10 @@ BuildSpeculativeIndexInfo(Relation index, IndexInfo *ii) index->rd_opcintype[i], index->rd_opcintype[i], ii->ii_UniqueStrats[i]); + if (!OidIsValid(ii->ii_UniqueOps[i])) + elog(ERROR, "missing operator %d(%u,%u) in opfamily %u", + ii->ii_UniqueStrats[i], index->rd_opcintype[i], + index->rd_opcintype[i], index->rd_opfamily[i]); ii->ii_UniqueProcs[i] = get_opcode(ii->ii_UniqueOps[i]); } } diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c index 74736e0ea1..e20ddce2db 100644 --- a/src/backend/catalog/partition.c +++ b/src/backend/catalog/partition.c @@ -1187,14 +1187,15 @@ get_partition_operator(PartitionKey key, int col, StrategyNumber strategy, key->partopcintype[col], key->partopcintype[col], strategy); + if (!OidIsValid(operoid)) + elog(ERROR, "missing operator %d(%u,%u) in opfamily %u", + strategy, key->partopcintype[col], key->partopcintype[col], + key->partopfamily[col]); *need_relabel = true; } else *need_relabel = false; - if (!OidIsValid(operoid)) - elog(ERROR, "could not find operator for partitioning"); - return operoid; } diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index 5267a011bb..7496189fab 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -1640,6 +1640,9 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, lefttype, righttype, BTORDER_PROC); + if (!OidIsValid(proc)) + elog(ERROR, "missing support function %d(%u,%u) in opfamily %u", + BTORDER_PROC, lefttype, righttype, opfamily); /* Set up the primary fmgr lookup information */ finfo = palloc0(sizeof(FmgrInfo)); diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index bc53d07c7d..3819de28ad 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -81,9 +81,8 @@ build_replindex_scan_key(ScanKey skey, Relation rel, Relation idxrel, operator = get_opfamily_member(opfamily, optype, optype, BTEqualStrategyNumber); - if (!OidIsValid(operator)) - elog(ERROR, "could not find member %d(%u,%u) of opfamily %u", + elog(ERROR, "missing operator %d(%u,%u) in opfamily %u", BTEqualStrategyNumber, optype, optype, opfamily); regop = get_opcode(operator); diff --git a/src/backend/executor/nodeIndexscan.c b/src/backend/executor/nodeIndexscan.c index d8aceb1f2c..75b10115f5 100644 --- a/src/backend/executor/nodeIndexscan.c +++ b/src/backend/executor/nodeIndexscan.c @@ -1367,6 +1367,9 @@ ExecIndexBuildScanKeys(PlanState *planstate, Relation index, op_lefttype, op_righttype, BTORDER_PROC); + if (!RegProcedureIsValid(opfuncid)) + elog(ERROR, "missing support function %d(%u,%u) in opfamily %u", + BTORDER_PROC, op_lefttype, op_righttype, opfamily); inputcollation = lfirst_oid(collids_cell); collids_cell = lnext(collids_cell); diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index dedb9f521d..f35380391a 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -3978,13 +3978,13 @@ adjust_rowcompare_for_index(RowCompareExpr *clause, expr_op = get_opfamily_member(opfam, lefttype, righttype, op_strategy); if (!OidIsValid(expr_op)) /* should not happen */ - elog(ERROR, "could not find member %d(%u,%u) of opfamily %u", + elog(ERROR, "missing operator %d(%u,%u) in opfamily %u", op_strategy, lefttype, righttype, opfam); if (!var_on_left) { expr_op = get_commutator(expr_op); if (!OidIsValid(expr_op)) /* should not happen */ - elog(ERROR, "could not find commutator of member %d(%u,%u) of opfamily %u", + elog(ERROR, "could not find commutator of operator %d(%u,%u) of opfamily %u", op_strategy, lefttype, righttype, opfam); } new_ops = lappend_oid(new_ops, expr_op); diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c index 37cfa098d4..9d83a5ca62 100644 --- a/src/backend/optimizer/path/pathkeys.c +++ b/src/backend/optimizer/path/pathkeys.c @@ -197,8 +197,8 @@ make_pathkey_from_sortinfo(PlannerInfo *root, opcintype, BTEqualStrategyNumber); if (!OidIsValid(equality_op)) /* shouldn't happen */ - elog(ERROR, "could not find equality operator for opfamily %u", - opfamily); + elog(ERROR, "missing operator %d(%u,%u) in opfamily %u", + BTEqualStrategyNumber, opcintype, opcintype, opfamily); opfamilies = get_mergejoin_opfamilies(equality_op); if (!opfamilies) /* certainly should find some */ elog(ERROR, "could not find opfamilies for equality operator %u", diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index e589d92c80..9d838e4f39 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -2634,7 +2634,8 @@ create_indexscan_plan(PlannerInfo *root, exprtype, pathkey->pk_strategy); if (!OidIsValid(sortop)) - elog(ERROR, "failed to find sort operator for ORDER BY expression"); + elog(ERROR, "missing operator %d(%u,%u) in opfamily %u", + pathkey->pk_strategy, exprtype, exprtype, pathkey->pk_opfamily); indexorderbyops = lappend_oid(indexorderbyops, sortop); } } @@ -5738,7 +5739,7 @@ prepare_sort_from_pathkeys(Plan *lefttree, List *pathkeys, pk_datatype, pathkey->pk_strategy); if (!OidIsValid(sortop)) /* should not happen */ - elog(ERROR, "could not find member %d(%u,%u) of opfamily %u", + elog(ERROR, "missing operator %d(%u,%u) in opfamily %u", pathkey->pk_strategy, pk_datatype, pk_datatype, pathkey->pk_opfamily); @@ -6216,7 +6217,7 @@ make_unique_from_pathkeys(Plan *lefttree, List *pathkeys, int numCols) pk_datatype, BTEqualStrategyNumber); if (!OidIsValid(eqop)) /* should not happen */ - elog(ERROR, "could not find member %d(%u,%u) of opfamily %u", + elog(ERROR, "missing operator %d(%u,%u) in opfamily %u", BTEqualStrategyNumber, pk_datatype, pk_datatype, pathkey->pk_opfamily); diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 43238dd8cb..6bd93b03d0 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -945,6 +945,10 @@ RelationBuildPartitionKey(Relation relation) opclassform->opcintype, opclassform->opcintype, BTORDER_PROC); + if (!OidIsValid(funcid)) /* should not happen */ + elog(ERROR, "missing support function %d(%u,%u) in opfamily %u", + BTORDER_PROC, opclassform->opcintype, opclassform->opcintype, + opclassform->opcfamily); fmgr_info(funcid, &key->partsupfunc[i]);