Re: BUG #16583: merge join on tables with different DB collation behind postgres_fdw fails
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Cc: jurafejfar@gmail.com, Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Date: 2020-08-18T20:09:44Z
Lists: pgsql-bugs, pgsql-hackers
Attachments
- fix-postgres-fdw-collation-handling-1.patch (text/x-diff) patch
I wrote:
>> So I think what is happening here is that postgres_fdw's version of
>> IMPORT FOREIGN SCHEMA translates "COLLATE default" on the remote
>> server to "COLLATE default" on the local one, which of course is
>> a big fail if the defaults don't match. That allows the local
>> planner to believe that remote ORDER BYs on the two foreign tables
>> will give compatible results, causing the merge join to not work
>> very well at all.
Here's a full patch addressing this issue. I decided that the best
way to address the test-instability problem is to explicitly give
collations to all the foreign-table columns for which it matters
in the postgres_fdw test. (For portability's sake, that has to be
"C" or "POSIX"; I mostly used "C".) Aside from ensuring that the
test still passes with some other prevailing locale, this seems like
a good idea since we'll then be testing the case we are encouraging
users to use.
And indeed, it immediately turned up a new problem: if we explicitly
assign a collation to a foreign-table column c, the system won't
ship WHERE clauses as simple as "c = 'foo'" to the remote. This
surprised me, but the reason turned out to be that what postgres_fdw
is actually seeing is something like
{OPEXPR
:opno 98
:opfuncid 67
:opresulttype 16
:opretset false
:opcollid 0
:inputcollid 950
:args (
{VAR
:varno 6
:varattno 4
:vartype 25
:vartypmod -1
:varcollid 950
:varlevelsup 0
:varnosyn 6
:varattnosyn 4
:location 171
}
{RELABELTYPE
:arg
{CONST
:consttype 25
:consttypmod -1
:constcollid 100
:constlen -1
:constbyval false
:constisnull false
:location 341
:constvalue 9 [ 36 0 0 0 48 48 48 48 49 ]
}
:resulttype 25
:resulttypmod -1
:resultcollid 950
:relabelformat 2
:location -1
}
)
:location -1
}
that is, the constant is being explicitly relabeled with the correct
collation, and thus is_foreign_expr() thinks the collation shown by
the RelabelType node is an unsafely-introduced collation.
What I did about this was to change the recursion rule in
foreign_expr_walker() so that merging a safely-derived collation with
the same collation unsafely derived is considered safe. I think this
is all right, and it allows us to accept some cases that previously
were rejected as unsafe. But I might be missing something.
(BTW, there's an independent bug here, which is that we're getting
a tree of the above shape rather than a simple Const with the
appropriate collation; that is, this tree isn't fully const-folded.
This is a bug in canonicalize_ec_expression, which I'll go fix
separately. But it won't affect the problem at hand.)
This seems like a sufficiently large change in postgres_fdw's
behavior to require review, so I'll go add this to the next CF.
regards, tom lane
Commits
-
Doc: extend warnings about collation-mismatch hazards in postgres_fdw.
- 2aa255588756 9.6.24 landed
- 1d7e8e772453 10.19 landed
- b274c982db67 11.14 landed
- 97c5651cea0f 12.9 landed
- 7b0be9fb2ddd 15.0 landed
- 739b872f6c98 13.5 landed
- 02c4e3533926 14.0 landed
-
Add support for asynchronous execution.
- 27e1f14563cf 14.0 cited
-
Suppress unnecessary RelabelType nodes in yet more cases.
- 814a57065ec9 12.5 landed
- 69ffc2f83899 13.0 landed
- 207293240780 14.0 landed