Re: Skip temporary table schema name from explain-verbose output. - Mailing list pgsql-hackers

I wrote:
> Simon Riggs <simon.riggs@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');