Re: [HACKERS] PATCH: Don't downcase filepath/filename while loading libraries
QL Zhuo writes: > And, the attached new patch fixes the memory leaks. Pushed with minor adjustments --- mostly, getting rid of the now-redundant canonicalize_path() call. I also updated the documentation. I notice the documentation formerly said "All library names are converted to lower case unless double-quoted", which means that formally this isn't a bug at all, but operating-as-designed-and-documented. Still, I agree it's an improvement. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PATCH: Don't downcase filepath/filename while loading libraries
"That's fourteen years without complains", so maybe not to back-patch is good choice, until someone complains this :) And, the attached new patch fixes the memory leaks. -- This email address (zhuo.devgmail.com) is only for development affairs, e.g. mail list, please mail to zhuohexoasis.com or zhuoqlzoho.com for other purpose. ZHUO QL (KDr2), http://kdr2.com On Fri, Jun 16, 2017 at 12:27 PM, Tom Lane wrote: > Michael Paquier writes: > > On Fri, Jun 16, 2017 at 1:01 PM, Tom Lane wrote: > >> (2) My inclination would be not to back-patch. This change could break > >> configurations that worked before, and the lack of prior complaints > >> says that not many people are having a problem with it. > > > That's fourteen years without complains, still I cannot imagine any > > cases where it would be a problem as people who would have faced this > > problem but not reported it have likely just enforced the FS to handle > > case-insensitivity for paths. > > Well, it's not just about case sensitivity. The comment for > SplitDirectoriesString points out that it deals with embedded spaces > differently, and it also applies canonicalize_path(). I'm too tired > to think hard about what that part might mean for compatibility, but > it probably isn't nothing. > > Anyway, I agree that this is an appropriate change for HEAD. Just > not convinced that we should shove it into minor releases. > > regards, tom lane > 0001-Don-t-downcase-filepath-in-load_libraries.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
Re: [HACKERS] PATCH: Don't downcase filepath/filename while loading libraries
Michael Paquier writes: > On Fri, Jun 16, 2017 at 1:01 PM, Tom Lane wrote: >> (2) My inclination would be not to back-patch. This change could break >> configurations that worked before, and the lack of prior complaints >> says that not many people are having a problem with it. > That's fourteen years without complains, still I cannot imagine any > cases where it would be a problem as people who would have faced this > problem but not reported it have likely just enforced the FS to handle > case-insensitivity for paths. Well, it's not just about case sensitivity. The comment for SplitDirectoriesString points out that it deals with embedded spaces differently, and it also applies canonicalize_path(). I'm too tired to think hard about what that part might mean for compatibility, but it probably isn't nothing. Anyway, I agree that this is an appropriate change for HEAD. Just not convinced that we should shove it into minor releases. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PATCH: Don't downcase filepath/filename while loading libraries
On Fri, Jun 16, 2017 at 1:01 PM, Tom Lane wrote: > (2) My inclination would be not to back-patch. This change could break > configurations that worked before, and the lack of prior complaints > says that not many people are having a problem with it. That's fourteen years without complains, still I cannot imagine any cases where it would be a problem as people who would have faced this problem but not reported it have likely just enforced the FS to handle case-insensitivity for paths. Or they just relied on the default: on Windows the default is to be case-insensitive, as much as OSX. So the proposed patch handles things better with case-sensitive paths, without really impacting users with case-insensive FS. I see the point of not back-patching the change though. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PATCH: Don't downcase filepath/filename while loading libraries
Michael Paquier writes: > On Fri, Jun 16, 2017 at 11:04 AM, QL Zhuo wrote: >> After few digging, I found there's a wrong use of `SplitIdentifierString` in >> function `load_libraries` in /src/backend/utils/init/miscinit.c, and the >> attached patch fixes it. > That's a good catch. All the other callers of SplitIdentifierString() > don't handle a list of directories. This requires a back-patch. (1) As is, I think the patch leaks memory. SplitDirectoriesString's API is not identical to SplitIdentifierString's. (2) My inclination would be not to back-patch. This change could break configurations that worked before, and the lack of prior complaints says that not many people are having a problem with it. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PATCH: Don't downcase filepath/filename while loading libraries
On Fri, Jun 16, 2017 at 11:04 AM, QL Zhuo wrote: > I just put this line in my postgresql.conf: > > ``` > shared_preload_libraries = '/Path/Contains/UpCaseWords/an_ext.so' > ``` > > Then the server couldn't start. It tried to load the file > "/path/contains/upcasewords/an_ext.so" and failed. > > After few digging, I found there's a wrong use of `SplitIdentifierString` in > function `load_libraries` in /src/backend/utils/init/miscinit.c, and the > attached patch fixes it. That's a good catch. All the other callers of SplitIdentifierString() don't handle a list of directories. This requires a back-patch. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers