Re: pg_get_constraintdef: Schema qualify foreign tables unless pretty printing is enabled

On Tue, Aug 9, 2022 at 5:33 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> I would say that pg_get_indexdef is the one that's out of step.
> I count 11 calls of generate_relation_name in ruleutils.c,
> of which only three have this business of being overridden
> when not-pretty. What is the rationale for that, and why
> would we move pg_get_constraintdef from one category to the
> other?
>

The overall motivation here is to make it easy to recreate the schema
without having to match the search_path on the importing side to be
identical to the exporting side. There is a workaround, which is to do a
SET search_path before calling these functions that excludes the referenced
schemas (which I guess is what pg_dump does?).

But I wonder, why do we have an explicit pretty printing flag on these
functions, and PRETTYFLAG_SCHEMA in the code to represent this behavior. If
we don't want pretty printing to affect schema qualification, why does that
flag exist?

Of the other call sites, in terms of using generate_relation_name vs
generate_qualified_relation_name:

* pg_get_triggerdef_worker makes it conditional on pretty=true, but only
for ON, not the FROM (not clear why that difference exists?)
* pg_get_indexdef_worker makes it conditional on prettyFlags &
PRETTYFLAG_SCHEMA for the ON
* pg_get_statisticsobj_worker does not handle pretty printing (always uses
generate_relation_name)
* make_ruledef makes it conditional on prettyFlags & PRETTYFLAG_SCHEMA for
the TO
* get_insert_query_def does not handle pretty printing (always uses
generate_relation_name)
* get_update_query_def does not handle pretty printing (always uses
generate_relation_name)
* get_delete_query_def does not handle pretty printing (always uses
generate_relation_name)
* get_rule_expr does not handle pretty printing (always uses
generate_relation_name)
* get_from_clause_item does not handle pretty printing (always uses
generate_relation_name)

Looking at that, it seems we didn't make the effort for the view related
code with all its complexity, and didn't do it for pg_get_statisticsobjdef
since it doesn't have a pretty flag. Why we didn't do it in
pg_get_triggerdef_worker for FROM isn't clear to me.

If we want to be entirely consistent (and keep supporting
PRETTYFLAG_SCHEMA), that probably means:

* Adding a pretty flag to pg_get_statisticsobjdef
* Teaching get_query_def to pass down prettyFlags to get_*_query_def
functions
* Update pg_get_triggerdef_worker to handle pretty for FROM as well

If that seems like a sensible direction I'd be happy to work on a patch.

Thanks,
Lukas

--
Lukas Fittl