Re: dblink get_connect_string() passes FDW option "updatable" to the connect string, connection fails.

From: Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Joe Conway <mail(at)joeconway(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: dblink get_connect_string() passes FDW option "updatable" to the connect string, connection fails.
Date: 2016-11-22 19:43:05
Message-ID: CADkLM=f4TkPAiPp1=9Z43kV35O9a1WS7=PTWc8SzyX923ZMkbg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

>
>
>
>> It looks like this might be fairly easy to fix by having
>> get_connect_string() use is_valid_dblink_option() to check each
>> option name, and silently ignore options that are inappropriate.
>>
>
> From what I can tell, it is very straightforward, the context oids are set
> up just a few lines above where the new is_valid_dblink_option() calls
> would be.
>
> I'm happy to write the patch, for both v10 and any back-patches we feel
> are necessary. However, I suspect with a patch this trivial that reviewing
> another person's patch might be more work for a committer than just doing
> it themselves. If that's not the case, let me know and I'll get started.
>

Joe indicated that he wouldn't be able to get to the patch until this
weekend at the earliest, so I went ahead and made the patches on my own.

Nothing unusual to report for master, 9.6, 9.5, or 9.3. The patch is
basically the same for all of them and I was able to re-run the test script
at the beginning of the thread to ensure that the fix worked.

In 9.4, I encountered a complaint about flex 2.6.0. After a little research
it seems that a fix for that made it into versions 9.3+, but not 9.4. That
mini-patch is attached as well (0001.configure.94.diff). The dblink patch
for 9.4 was basically the same as the others.

The issue (no validation of connection string elements pulled from an FDW)
exists in 9.2, however, the only possible source of such options I know of
(postgres_fdw) does not. So I doubt we need to patch 9.2, but it's trivial
to do so if we want to.

Attachment Content-Type Size
0001.configure.94.diff text/plain 678 bytes
0001.dblink_check_connect_options.93.diff text/plain 2.4 KB
0001.dblink_check_connect_options.94.diff text/plain 2.4 KB
0001.dblink_check_connect_options.95.diff text/plain 2.4 KB
0001.dblink_check_connect_options.96.diff text/plain 2.4 KB
0001.dblink_check_connect_options.master.diff text/plain 2.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2016-11-22 19:50:17 Re: Declarative partitioning - another take
Previous Message Robert Haas 2016-11-22 19:35:38 Re: [RFC] Should we fix postmaster to avoid slow shutdown?