I wrote: > Simon Riggs <simon.ri...@enterprisedb.com> writes: >> Surely we need a test to exercise this works? Otherwise ready...
> There are lots of places in the core regression tests where we'd have > used a temp table, except that we needed to do EXPLAIN and the results > would've been unstable, so we used a short-lived plain table instead. > Find one of those and change it to use a temp table. Hmm ... I looked through the core regression tests' usages of EXPLAIN VERBOSE and didn't really find any that it seemed to make sense to change that way. I guess we've been more effective at programming around that restriction than I thought. Anyway, after looking at the 0001 patch, I think there's a pretty large oversight in that it doesn't touch ruleutils.c, although EXPLAIN relies heavily on that to print expressions and suchlike. We could account for that as in the attached revision of 0001. However, I wonder whether this isn't going in the wrong direction. Instead of piecemeal s/get_namespace_name/get_namespace_name_or_temp/, we should consider just putting this behavior right into get_namespace_name, and dropping the separate get_namespace_name_or_temp function. I can't really see any situation in which it's important to report the exact schema name of our own temp schema. On the other hand, I don't like 0002 one bit, because it's not accounting for whether the temp schema it's mangling is *our own* temp schema or some other session's. I do not think it is wise or even safe to report some other temp schema as being "pg_temp". By the same token, I wonder whether this bit in event_trigger.c is a good idea or a safety hazard: /* XXX not quite get_namespace_name_or_temp */ if (isAnyTempNamespace(schema_oid)) schema = pstrdup("pg_temp"); else schema = get_namespace_name(schema_oid); Alvaro, you seem to be responsible for both the existence of the separate get_namespace_name_or_temp function and the fact that it's being avoided here. I wonder what you think about this. regards, tom lane
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index f15c97ad7a..51fac77f3d 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -2854,7 +2854,7 @@ postgresExplainForeignScan(ForeignScanState *node, ExplainState *es) { char *namespace; - namespace = get_namespace_name(get_rel_namespace(rte->relid)); + namespace = get_namespace_name_or_temp(get_rel_namespace(rte->relid)); appendStringInfo(relations, "%s.%s", quote_identifier(namespace), quote_identifier(relname)); diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 340db2bac4..36fbe129cf 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -3747,7 +3747,7 @@ ExplainTargetRel(Plan *plan, Index rti, ExplainState *es) Assert(rte->rtekind == RTE_RELATION); objectname = get_rel_name(rte->relid); if (es->verbose) - namespace = get_namespace_name(get_rel_namespace(rte->relid)); + namespace = get_namespace_name_or_temp(get_rel_namespace(rte->relid)); objecttag = "Relation Name"; break; case T_FunctionScan: @@ -3774,8 +3774,7 @@ ExplainTargetRel(Plan *plan, Index rti, ExplainState *es) objectname = get_func_name(funcid); if (es->verbose) - namespace = - get_namespace_name(get_func_namespace(funcid)); + namespace = get_namespace_name_or_temp(get_func_namespace(funcid)); } } objecttag = "Function Name"; diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 5e7108f903..4df8cc5abf 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -1617,7 +1617,7 @@ pg_get_statisticsobj_worker(Oid statextid, bool columns_only, bool missing_ok) if (!columns_only) { - nsp = get_namespace_name(statextrec->stxnamespace); + nsp = get_namespace_name_or_temp(statextrec->stxnamespace); appendStringInfo(&buf, "CREATE STATISTICS %s", quote_qualified_identifier(nsp, NameStr(statextrec->stxname))); @@ -2811,7 +2811,7 @@ pg_get_functiondef(PG_FUNCTION_ARGS) * We always qualify the function name, to ensure the right function gets * replaced. */ - nsp = get_namespace_name(proc->pronamespace); + nsp = get_namespace_name_or_temp(proc->pronamespace); appendStringInfo(&buf, "CREATE OR REPLACE %s %s(", isfunction ? "FUNCTION" : "PROCEDURE", quote_qualified_identifier(nsp, name)); @@ -11183,7 +11183,7 @@ get_opclass_name(Oid opclass, Oid actual_datatype, appendStringInfo(buf, " %s", quote_identifier(opcname)); else { - nspname = get_namespace_name(opcrec->opcnamespace); + nspname = get_namespace_name_or_temp(opcrec->opcnamespace); appendStringInfo(buf, " %s.%s", quote_identifier(nspname), quote_identifier(opcname)); @@ -11495,7 +11495,7 @@ generate_relation_name(Oid relid, List *namespaces) need_qual = !RelationIsVisible(relid); if (need_qual) - nspname = get_namespace_name(reltup->relnamespace); + nspname = get_namespace_name_or_temp(reltup->relnamespace); else nspname = NULL; @@ -11527,7 +11527,7 @@ generate_qualified_relation_name(Oid relid) reltup = (Form_pg_class) GETSTRUCT(tp); relname = NameStr(reltup->relname); - nspname = get_namespace_name(reltup->relnamespace); + nspname = get_namespace_name_or_temp(reltup->relnamespace); if (!nspname) elog(ERROR, "cache lookup failed for namespace %u", reltup->relnamespace); @@ -11639,7 +11639,7 @@ generate_function_name(Oid funcid, int nargs, List *argnames, Oid *argtypes, p_funcid == funcid) nspname = NULL; else - nspname = get_namespace_name(procform->pronamespace); + nspname = get_namespace_name_or_temp(procform->pronamespace); result = quote_qualified_identifier(nspname, proname); @@ -11702,7 +11702,7 @@ generate_operator_name(Oid operid, Oid arg1, Oid arg2) nspname = NULL; else { - nspname = get_namespace_name(operform->oprnamespace); + nspname = get_namespace_name_or_temp(operform->oprnamespace); appendStringInfo(&buf, "OPERATOR(%s.", quote_identifier(nspname)); } @@ -11790,7 +11790,7 @@ add_cast_to(StringInfo buf, Oid typid) typform = (Form_pg_type) GETSTRUCT(typetup); typname = NameStr(typform->typname); - nspname = get_namespace_name(typform->typnamespace); + nspname = get_namespace_name_or_temp(typform->typnamespace); appendStringInfo(buf, "::%s.%s", quote_identifier(nspname), quote_identifier(typname)); @@ -11822,7 +11822,7 @@ generate_qualified_type_name(Oid typid) typtup = (Form_pg_type) GETSTRUCT(tp); typname = NameStr(typtup->typname); - nspname = get_namespace_name(typtup->typnamespace); + nspname = get_namespace_name_or_temp(typtup->typnamespace); if (!nspname) elog(ERROR, "cache lookup failed for namespace %u", typtup->typnamespace); @@ -11856,7 +11856,7 @@ generate_collation_name(Oid collid) collname = NameStr(colltup->collname); if (!CollationIsVisible(collid)) - nspname = get_namespace_name(colltup->collnamespace); + nspname = get_namespace_name_or_temp(colltup->collnamespace); else nspname = NULL; diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c index 6bba5f8ec4..4ebaa552a2 100644 --- a/src/backend/utils/cache/lsyscache.c +++ b/src/backend/utils/cache/lsyscache.c @@ -3340,7 +3340,7 @@ char * get_namespace_name_or_temp(Oid nspid) { if (isTempNamespace(nspid)) - return "pg_temp"; + return pstrdup("pg_temp"); else return get_namespace_name(nspid); } diff --git a/src/test/regress/expected/explain.out b/src/test/regress/expected/explain.out index cda28098ba..16e196a7bd 100644 --- a/src/test/regress/expected/explain.out +++ b/src/test/regress/expected/explain.out @@ -477,6 +477,19 @@ select jsonb_pretty( (1 row) rollback; +-- Test display of temporary objects +create temp table t1(f1 float8); +create function pg_temp.mysin(float8) returns float8 language plpgsql +as 'begin return sin($1); end'; +select explain_filter('explain (verbose) select * from t1 where pg_temp.mysin(f1) < 0.5'); + explain_filter +------------------------------------------------------------ + Seq Scan on pg_temp.t1 (cost=N.N..N.N rows=N width=N) + Output: f1 + Filter: (pg_temp.mysin(t1.f1) < 'N.N'::double precision) +(3 rows) + +-- Test compute_query_id set compute_query_id = on; select explain_filter('explain (verbose) select * from int8_tbl i8'); explain_filter diff --git a/src/test/regress/sql/explain.sql b/src/test/regress/sql/explain.sql index 3f9ae9843a..f401d99409 100644 --- a/src/test/regress/sql/explain.sql +++ b/src/test/regress/sql/explain.sql @@ -104,5 +104,14 @@ select jsonb_pretty( rollback; +-- Test display of temporary objects +create temp table t1(f1 float8); + +create function pg_temp.mysin(float8) returns float8 language plpgsql +as 'begin return sin($1); end'; + +select explain_filter('explain (verbose) select * from t1 where pg_temp.mysin(f1) < 0.5'); + +-- Test compute_query_id set compute_query_id = on; select explain_filter('explain (verbose) select * from int8_tbl i8');