diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c index 6290ce9b43..4665f0b2b7 100644 --- a/src/backend/parser/parse_relation.c +++ b/src/backend/parser/parse_relation.c @@ -41,17 +41,35 @@ /* * Support for fuzzily matching columns. * - * This is for building diagnostic messages, where non-exact matching - * attributes are suggested to the user. The struct's fields may be facets of - * a particular RTE, or of an entire range table, depending on context. + * This is for building diagnostic messages, where multiple or non-exact + * matching attributes are of interest. + * + * "distance" is the current best fuzzy-match distance if rfirst isn't NULL, + * otherwise it is the maximum acceptable distance plus 1. + * + * rfirst/first record the closest non-exact match so far, and distance + * is its distance from the target name. If we have found a second non-exact + * match of exactly the same distance, rsecond/second record that. (If + * we find three of the same distance, we conclude that "distance" is not + * a tight enough bound for a useful hint and clear rfirst/rsecond again. + * Only if we later find something closer will we re-populate rfirst.) + * + * rexact1/exact1 record the location of the first exactly-matching column, + * if any. If we find multiple exact matches then rexact2/exact2 record + * another one (we don't especially care which). Currently, these get + * populated independently of the fuzzy-match fields. */ typedef struct { - int distance; /* Weighted distance (lowest so far) */ - RangeTblEntry *rfirst; /* RTE of first */ - AttrNumber first; /* Closest attribute so far */ - RangeTblEntry *rsecond; /* RTE of second */ - AttrNumber second; /* Second closest attribute so far */ + int distance; /* Current or limit distance */ + RangeTblEntry *rfirst; /* RTE of closest non-exact match, or NULL */ + AttrNumber first; /* Col index in rfirst */ + RangeTblEntry *rsecond; /* RTE of another non-exact match w/same dist */ + AttrNumber second; /* Col index in rsecond */ + RangeTblEntry *rexact1; /* RTE of first exact match, or NULL */ + AttrNumber exact1; /* Col index in rexact1 */ + RangeTblEntry *rexact2; /* RTE of second exact match, or NULL */ + AttrNumber exact2; /* Col index in rexact2 */ } FuzzyAttrMatchState; #define MAX_FUZZY_DISTANCE 3 @@ -612,47 +630,39 @@ updateFuzzyAttrMatchState(int fuzzy_rte_penalty, */ if (columndistance < fuzzystate->distance) { - /* Store new lowest observed distance for RTE */ + /* Store new lowest observed distance as first/only match */ fuzzystate->distance = columndistance; fuzzystate->rfirst = rte; fuzzystate->first = attnum; fuzzystate->rsecond = NULL; - fuzzystate->second = InvalidAttrNumber; } else if (columndistance == fuzzystate->distance) { - /* - * This match distance may equal a prior match within this same range - * table. When that happens, the prior match may also be given, but - * only if there is no more than two equally distant matches from the - * RTE (in turn, our caller will only accept two equally distant - * matches overall). - */ - if (AttributeNumberIsValid(fuzzystate->second)) + /* If we already have a match of this distance, update state */ + if (fuzzystate->rsecond != NULL) { - /* Too many RTE-level matches */ + /* + * Too many matches at same distance. Clearly, this value of + * distance is too low a bar, so drop these entries while keeping + * the current distance value, so that only smaller distances will + * be considered interesting. Only if we find something of lower + * distance will we re-populate rfirst (via the stanza above). + */ fuzzystate->rfirst = NULL; - fuzzystate->first = InvalidAttrNumber; fuzzystate->rsecond = NULL; - fuzzystate->second = InvalidAttrNumber; - /* Clearly, distance is too low a bar (for *any* RTE) */ - fuzzystate->distance = columndistance - 1; } - else if (AttributeNumberIsValid(fuzzystate->first)) + else if (fuzzystate->rfirst != NULL) { - /* Record as provisional second match for RTE */ + /* Record as provisional second match */ fuzzystate->rsecond = rte; fuzzystate->second = attnum; } - else if (fuzzystate->distance <= MAX_FUZZY_DISTANCE) + else { /* - * Record as provisional first match (this can occasionally occur - * because previous lowest distance was "too low a bar", rather - * than being associated with a real match) + * Do nothing. When rfirst is NULL, distance is more than what we + * want to consider acceptable, so we should ignore this match. */ - fuzzystate->rfirst = rte; - fuzzystate->first = attnum; } } } @@ -925,21 +935,15 @@ colNameToVar(ParseState *pstate, const char *colname, bool localonly, * This is different from colNameToVar in that it considers every entry in * the ParseState's rangetable(s), not only those that are currently visible * in the p_namespace list(s). This behavior is invalid per the SQL spec, - * and it may give ambiguous results (there might be multiple equally valid - * matches, but only one will be returned). This must be used ONLY as a - * heuristic in giving suitable error messages. See errorMissingColumn. + * and it may give ambiguous results (since there might be multiple equally + * valid matches). This must be used ONLY as a heuristic in giving suitable + * error messages. See errorMissingColumn. * * This function is also different in that it will consider approximate * matches -- if the user entered an alias/column pair that is only slightly * different from a valid pair, we may be able to infer what they meant to - * type and provide a reasonable hint. - * - * The FuzzyAttrMatchState will have 'rfirst' pointing to the best RTE - * containing the most promising match for the alias and column name. If - * the alias and column names match exactly, 'first' will be InvalidAttrNumber; - * otherwise, it will be the attribute number for the match. In the latter - * case, 'rsecond' may point to a second, equally close approximate match, - * and 'second' will contain the attribute number for the second match. + * type and provide a reasonable hint. We return a FuzzyAttrMatchState + * struct providing information about both exact and approximate matches. */ static FuzzyAttrMatchState * searchRangeTableForCol(ParseState *pstate, const char *alias, const char *colname, @@ -951,8 +955,8 @@ searchRangeTableForCol(ParseState *pstate, const char *alias, const char *colnam fuzzystate->distance = MAX_FUZZY_DISTANCE + 1; fuzzystate->rfirst = NULL; fuzzystate->rsecond = NULL; - fuzzystate->first = InvalidAttrNumber; - fuzzystate->second = InvalidAttrNumber; + fuzzystate->rexact1 = NULL; + fuzzystate->rexact2 = NULL; while (pstate != NULL) { @@ -962,6 +966,7 @@ searchRangeTableForCol(ParseState *pstate, const char *alias, const char *colnam { RangeTblEntry *rte = (RangeTblEntry *) lfirst(l); int fuzzy_rte_penalty = 0; + int attnum; /* * Typically, it is not useful to look for matches within join @@ -988,18 +993,27 @@ searchRangeTableForCol(ParseState *pstate, const char *alias, const char *colnam true); /* - * Scan for a matching column; if we find an exact match, we're - * done. Otherwise, update fuzzystate. + * Scan for a matching column, and update fuzzystate. Non-exact + * matches are dealt with inside scanRTEForColumn, but exact + * matches are handled here. (There won't be more than one exact + * match in the same RTE, else we'd have thrown error earlier.) */ - if (scanRTEForColumn(orig_pstate, rte, rte->eref, colname, location, - fuzzy_rte_penalty, fuzzystate) - && fuzzy_rte_penalty == 0) + attnum = scanRTEForColumn(orig_pstate, rte, rte->eref, + colname, location, + fuzzy_rte_penalty, fuzzystate); + if (attnum != InvalidAttrNumber && fuzzy_rte_penalty == 0) { - fuzzystate->rfirst = rte; - fuzzystate->first = InvalidAttrNumber; - fuzzystate->rsecond = NULL; - fuzzystate->second = InvalidAttrNumber; - return fuzzystate; + if (fuzzystate->rexact1 == NULL) + { + fuzzystate->rexact1 = rte; + fuzzystate->exact1 = attnum; + } + else + { + /* Needn't worry about overwriting previous rexact2 */ + fuzzystate->rexact2 = rte; + fuzzystate->exact2 = attnum; + } } } @@ -3645,7 +3659,6 @@ errorMissingColumn(ParseState *pstate, const char *relname, const char *colname, int location) { FuzzyAttrMatchState *state; - char *closestfirst = NULL; /* * Search the entire rtable looking for possible matches. If we find one, @@ -3654,69 +3667,78 @@ errorMissingColumn(ParseState *pstate, state = searchRangeTableForCol(pstate, relname, colname, location); /* - * Extract closest col string for best match, if any. - * - * Infer an exact match referenced despite not being visible from the fact - * that an attribute number was not present in state passed back -- this - * is what is reported when !closestfirst. There might also be an exact - * match that was qualified with an incorrect alias, in which case - * closestfirst will be set (so hint is the same as generic fuzzy case). + * If there are exact match(es), they must be inaccessible for some + * reason. */ - if (state->rfirst && AttributeNumberIsValid(state->first)) - closestfirst = strVal(list_nth(state->rfirst->eref->colnames, - state->first - 1)); - - if (!state->rsecond) + if (state->rexact1) { - /* If we found no match at all, we have little to report */ - if (!state->rfirst) - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_COLUMN), - relname ? - errmsg("column %s.%s does not exist", relname, colname) : - errmsg("column \"%s\" does not exist", colname), - parser_errposition(pstate, location))); - /* Handle case where we have a single alternative spelling to offer */ - else if (closestfirst) + /* + * We don't try too hard when there's multiple inaccessible exact + * matches, but at least be sure that we don't misleadingly suggest + * that there's only one. + */ + if (state->rexact2) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_COLUMN), relname ? errmsg("column %s.%s does not exist", relname, colname) : errmsg("column \"%s\" does not exist", colname), - errhint("Perhaps you meant to reference the column \"%s.%s\".", - state->rfirst->eref->aliasname, closestfirst), + errdetail("There are columns named \"%s\", but they are in tables that cannot be referenced from this part of the query.", + colname), + !relname ? errhint("Try using a table-qualified name.") : 0, parser_errposition(pstate, location))); - /* We found an exact match but it's inaccessible for some reason */ - else + /* Single exact match, so try to determine why it's inaccessible. */ + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_COLUMN), + relname ? + errmsg("column %s.%s does not exist", relname, colname) : + errmsg("column \"%s\" does not exist", colname), + errdetail("There is a column named \"%s\" in table \"%s\", but it cannot be referenced from this part of the query.", + colname, state->rexact1->eref->aliasname), + rte_visible_if_lateral(pstate, state->rexact1) ? + errhint("To reference that column, you must mark this subquery with LATERAL.") : + (!relname && rte_visible_if_qualified(pstate, state->rexact1)) ? + errhint("To reference that column, you must use a table-qualified name.") : 0, + parser_errposition(pstate, location))); + } + + if (!state->rsecond) + { + /* If we found no match at all, we have little to report */ + if (!state->rfirst) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_COLUMN), relname ? errmsg("column %s.%s does not exist", relname, colname) : errmsg("column \"%s\" does not exist", colname), - errdetail("There is a column named \"%s\" in table \"%s\", but it cannot be referenced from this part of the query.", - colname, state->rfirst->eref->aliasname), - rte_visible_if_lateral(pstate, state->rfirst) ? - errhint("To reference that column, you must mark this subquery with LATERAL.") : - (!relname && rte_visible_if_qualified(pstate, state->rfirst)) ? - errhint("To reference that column, you must use a table-qualified name.") : 0, parser_errposition(pstate, location))); + /* Handle case where we have a single alternative spelling to offer */ + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_COLUMN), + relname ? + errmsg("column %s.%s does not exist", relname, colname) : + errmsg("column \"%s\" does not exist", colname), + errhint("Perhaps you meant to reference the column \"%s.%s\".", + state->rfirst->eref->aliasname, + strVal(list_nth(state->rfirst->eref->colnames, + state->first - 1))), + parser_errposition(pstate, location))); } else { /* Handle case where there are two equally useful column hints */ - char *closestsecond; - - closestsecond = strVal(list_nth(state->rsecond->eref->colnames, - state->second - 1)); - ereport(ERROR, (errcode(ERRCODE_UNDEFINED_COLUMN), relname ? errmsg("column %s.%s does not exist", relname, colname) : errmsg("column \"%s\" does not exist", colname), errhint("Perhaps you meant to reference the column \"%s.%s\" or the column \"%s.%s\".", - state->rfirst->eref->aliasname, closestfirst, - state->rsecond->eref->aliasname, closestsecond), + state->rfirst->eref->aliasname, + strVal(list_nth(state->rfirst->eref->colnames, + state->first - 1)), + state->rsecond->eref->aliasname, + strVal(list_nth(state->rsecond->eref->colnames, + state->second - 1))), parser_errposition(pstate, location))); } } diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index ebaa289e12..72a3abad9e 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -5069,8 +5069,8 @@ select ctid from ERROR: column "ctid" does not exist LINE 1: select ctid from ^ -DETAIL: There is a column named "ctid" in table "t1", but it cannot be referenced from this part of the query. -HINT: To reference that column, you must use a table-qualified name. +DETAIL: There are columns named "ctid", but they are in tables that cannot be referenced from this part of the query. +HINT: Try using a table-qualified name. -- -- Take care to reference the correct RTE -- diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index acf28ba580..7c7adbc004 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1194,8 +1194,8 @@ do instead insert into rules_foo2 values (f1); ERROR: column "f1" does not exist LINE 2: do instead insert into rules_foo2 values (f1); ^ -DETAIL: There is a column named "f1" in table "old", but it cannot be referenced from this part of the query. -HINT: To reference that column, you must use a table-qualified name. +DETAIL: There are columns named "f1", but they are in tables that cannot be referenced from this part of the query. +HINT: Try using a table-qualified name. -- this is the correct way: create rule rules_foorule as on insert to rules_foo where f1 < 100 do instead insert into rules_foo2 values (new.f1);