pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: steven.winfield@cantabcapital.com
Cc: pgsql-bugs@lists.postgresql.org, Michael Paquier <michael@paquier.xyz>
Subject: Re: BUG #15248: pg_upgrade fails when a function with an empty search_path is encountered
Date: Tue, 19 Jun 2018 15:17:52 -0400
Message-ID: <7585.1529435872@sss.pgh.pa.us> (raw)
In-Reply-To: <152940707697.1452.4922154343836581163@wrigleys.postgresql.org>
References: <152940707697.1452.4922154343836581163@wrigleys.postgresql.org>

=?utf-8?q?PG_Bug_reporting_form?= <noreply@postgresql.org> writes:
> If a function is defined like this:

> CREATE FUNCTION public.foo(int) RETURNS int
>     LANGUAGE "sql"
>     SET search_path TO ''
>     AS $_$ SELECT 1; $_$;

> ...then, during pg_upgrade, pg_restore renders this SQL to recreate the
> function using double-quotes around the empty search_path:

>     Command was: CREATE FUNCTION "public"."foo"("int") RETURNS int
>     LANGUAGE "sql" 
>     SET "search_path" TO ""
>     AS $_$ SELECT 1 $_$

> [ which isn't legal syntax ]

Hm.  So, basically, this is due to insufficient thought about quoting
rules in commits 742869946 et al.  We made pg_dump print out exactly
what it sees in pg_proc.proconfig for this case --- but that string
has been constructed by flatten_set_variable_args, and while its
quoting rules are a lot like those for double-quoted SQL identifiers,
they aren't identical.  Notably we have this issue, that "" is a legal
empty string as per flatten_set_variable_args, but it's not a legal
SQL identifier.  Another problem, now that I think about it, is that
on reload a double-quoted identifier is going to get truncated to
NAMEDATALEN, but that's not necessarily appropriate --- some
GUC_LIST_QUOTE variables contain file path names that could legally
be longer than that.

If we were working in a green field I'd think about revising
flatten_set_variable_args, but touching those rules is pretty scary;
even if we found all the relevant places in the backend and pg_dump,
there might be third-party code that is looking at proconfig or
related catalog columns.  Also, that'd do nothing for existing
database objects that have config strings in the existing style.

So it seems like what we have to do here is to teach pg_dump and ruleutils
to parse a GUC_LIST_QUOTE value the same way SplitIdentifierString does,
and then quote each extracted list element as a string literal.  Bleah.
It's not *that* much code, but it's annoying, especially because of the
duplicated logic.

			regards, tom lane




view thread (4+ messages)  latest in thread

Message-ID: <7585.1529435872@sss.pgh.pa.us>
Permalink:  ../7585.1529435872@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/7585.1529435872@sss.pgh.pa.us

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-bugs@postgresql.org
  Cc: tgl@sss.pgh.pa.us, steven.winfield@cantabcapital.com, michael@paquier.xyz
  Subject: Re: BUG #15248: pg_upgrade fails when a function with an empty search_path is encountered
  In-Reply-To: <7585.1529435872@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox