Re: postgres_fdw vs data formatting GUCs (was Re: [HACKERS] [v9.3] writable foreign tables)

[Daniel Farina]

On Wed, Mar 20, 2013 at 7:43 AM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Daniel Farina <dan...@heroku.com> writes:
>> Okay, one more of those fridge-logic bugs.  Sorry for the noise. v5.
>
>> A missing PG_RETHROW to get the properly finally-esque semantics:
>
>> --- a/contrib/dblink/dblink.c
>> +++ b/contrib/dblink/dblink.c
>> @@ -642,7 +642,10 @@ dblink_fetch(PG_FUNCTION_ARGS)
>>   }
>>   PG_CATCH();
>>   {
>> + /* Pop any set GUCs, if necessary */
>>   restoreLocalGucs(&rgs);
>> +
>> + PG_RE_THROW();
>>   }
>>   PG_END_TRY();
>
> Um ... you shouldn't need a PG_TRY for that at all.  guc.c will take
> care of popping the values on transaction abort --- that's really rather
> the whole point of having that mechanism.

Hmm, well, merely raising the error doesn't reset the GUCs, so I was
rather thinking that this was a good idea to compose more neatly in
the case of nested exception processing, e.g.:

    PG_TRY();
    {
        PG_TRY();
        {
            elog(NOTICE, "pre: %s",
                 GetConfigOption("DateStyle", false, true));
            materializeResult(fcinfo, res);
        }
        PG_CATCH();
        {
            elog(NOTICE, "inner catch: %s",
                 GetConfigOption("DateStyle", false, true));
            PG_RE_THROW();
        }
        PG_END_TRY();
    }
    PG_CATCH();
    {
        elog(NOTICE, "outer catch: %s",
             GetConfigOption("DateStyle", false, true));
        restoreLocalGucs(&rgs);
        elog(NOTICE, "restored: %s",
             GetConfigOption("DateStyle", false, true));
        PG_RE_THROW();
    }
    PG_END_TRY();

I don't think this paranoia is made in other call sites for AtEOXact,
so included is a version that takes the same stance. This one shows up
 best with the whitespace-insensitive option for git:

--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -634,21 +634,8 @@ dblink_fetch(PG_FUNCTION_ARGS)
         * affect parsing and then un-set them afterwards.
         */
        initRemoteGucs(&rgs, conn);
-
-       PG_TRY();
-       {
        applyRemoteGucs(&rgs);
        materializeResult(fcinfo, res);
-       }
-       PG_CATCH();
-       {
-               /* Pop any set GUCs, if necessary */
-               restoreLocalGucs(&rgs);
-
-               PG_RE_THROW();
-       }
-       PG_END_TRY();
-
        restoreLocalGucs(&rgs);

        return (Datum) 0;
@@ -823,9 +810,6 @@ dblink_record_internal(FunctionCallInfo fcinfo,
bool is_async)
                if (freeconn)
                        PQfinish(conn);

-               /* Pop any set GUCs, if necessary */
-               restoreLocalGucs(&rgs);
-
                PG_RE_THROW();
        }
        PG_END_TRY();

--
fdr

dblink-guc-sensitive-types-v6.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers