PATCH: Expose generate_qualified_relation_name functionality

From: Craig Ringer <craig(at)2ndquadrant(dot)com>
To: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: PATCH: Expose generate_qualified_relation_name functionality
Date: 2017-10-06 03:08:37
Message-ID: CAMsr+YFJYGvFEP_E38i7jCAN=tcFZ40D2yonDLrP+JTPu4rH7Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I'm regularly surprised that the only non-static function we seem to
have for getting a relation name by oid is get_rel_name. It doesn't
handle schema qualification at all, and it returns NULL rather than
ERROR'ing.

Doing it correctly involves interacting with the syscache, calling
RelationIsVisible and calling get_namespace_name if needed, then
passing the result to quote_qualified_identifier.

so here's what I propose to do:

Add get_qualified_relation_name(bool force_qualified, bool missing_ok)
to ruleutils.c and builtins.h.

(Unless there's somewhere better? I wanted to put it in lsyscache.c
alongside get_rel_name, but it uses quote_qualified_identifier,
StringInfo, etc, so it doesn't fit well in lsyscache.c.)

Replace generate_qualified_relation_name in ruleutils.c with a call to
get_qualified_relation_name(relid, true, false) .

Add comments to RelationGetRelationName, get_rel_name and regclassout
pointing to get_qualified_relation_name.

The only part I don't like here is the two boolean arguments, since
they're ugly to read. But inventing a new flags field seemed a bit
heavy for such a trivial task.

I had a quick look through the codebase for places where this pattern
is repeated and found astonishingly few. In most places we just use
get_rel_name and hope the user can figure out any ambiguity.

I did notice that in heap_copy_data and AlterTableMoveAll we manually
schema-qualify a relation name and fail to call
quote_qualified_identifier, so it's unclear if we mean a relation
named "my.relation" or the relation "relation" in schema "my". But in
diagnostic output, meh, whatever.

Arguably regclassout does the same thing as
get_qualified_relation_name, but I didn't replace it because it cares
about IsBootstrapProcessingMode().

(Prompted by https://dba.stackexchange.com/q/187788/7788)

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachment Content-Type Size
v1-0001-Add-get_qualified_relation_name.patch text/x-patch 6.5 KB

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2017-10-06 03:19:39 Re: Parallel Append implementation
Previous Message Craig Ringer 2017-10-06 03:04:38 Re: [PATCH] A hook for session start