Re: drop tablespace failed when location contains .. on win32

2022-01-31 Thread Tom Lane
Andrew Dunstan writes: > On 1/30/22 16:50, Tom Lane wrote: >> Here's a revised patch version that does it like that. I also >> reviewed and simplified the canonicalize_path logic. I think >> this is committable. > LGTM Pushed, thanks for looking. I think I'm also going to have a look at

Re: drop tablespace failed when location contains .. on win32

2022-01-31 Thread Andrew Dunstan
On 1/30/22 16:50, Tom Lane wrote: > Michael Paquier writes: >> In order to make the tests cheap, there is no need to have a separate >> module in src/test/modules/ as your patch 0002 is doing. Instead, you >> should move the C code of your SQL function test_canonicalize_path() >> to

Re: drop tablespace failed when location contains .. on win32

2022-01-31 Thread Tom Lane
Michael Paquier writes: > Should we have tests for WIN32 (aka for driver letters and "//")? > This could be split into its own separate test file to limit the > damage with the alternate outputs, and the original complain was from > there. I thought about it and concluded that the value couldn't

Re: drop tablespace failed when location contains .. on win32

2022-01-31 Thread Michael Paquier
On Sun, Jan 30, 2022 at 04:50:03PM -0500, Tom Lane wrote: > Here's a revised patch version that does it like that. I also > reviewed and simplified the canonicalize_path logic. I think > this is committable. Thanks for the updated version. The range of the tests looks fine enough, and the CF

Re: drop tablespace failed when location contains .. on win32

2022-01-30 Thread Tom Lane
Michael Paquier writes: > In order to make the tests cheap, there is no need to have a separate > module in src/test/modules/ as your patch 0002 is doing. Instead, you > should move the C code of your SQL function test_canonicalize_path() > to src/test/regress/regress.c, then add some tests in >

Re: drop tablespace failed when location contains .. on win32

2022-01-24 Thread Michael Paquier
On Mon, Jan 24, 2022 at 11:21:12AM +, wangsh.f...@fujitsu.com wrote: > Tom Lane wrote: >>> I concur with the upthread comments that there's little chance >>> we'll commit 0003 as-is; the code-to-benefit ratio is too high. >>> Instead, you might consider adding test_canonicalize_path in >>>

RE: drop tablespace failed when location contains .. on win32

2022-01-24 Thread wangsh.f...@fujitsu.com
Hi, The new version is attached. Tom Lane wrote: > I tried to read 0001 but really couldn't make sense of the logic > at all, because it's seriously underdocumented. At minimum you > need an API spec comment for canonicalize_path_sub, explaining > what it's supposed to do and why. I have

Re: drop tablespace failed when location contains .. on win32

2022-01-17 Thread Michael Paquier
On Tue, Jan 18, 2022 at 01:08:01AM +, wangsh.f...@fujitsu.com wrote: > Yes, I will send a new version before next weekend Thanks! -- Michael signature.asc Description: PGP signature

RE: drop tablespace failed when location contains .. on win32

2022-01-17 Thread wangsh.f...@fujitsu.com
Hi > This patch is a wanted bugfix and has been waiting for an update for 2 months. > > Do you plan to send a new version soon? Yes, I will send a new version before next weekend Regards Shenhao Wang

Re: drop tablespace failed when location contains .. on win32

2022-01-16 Thread Julien Rouhaud
Hi, This patch is a wanted bugfix and has been waiting for an update for 2 months. Do you plan to send a new version soon?

Re: drop tablespace failed when location contains .. on win32

2021-11-18 Thread Michael Paquier
On Wed, Nov 10, 2021 at 05:43:31PM -0500, Tom Lane wrote: > Another thing I happened to notice is that join_path_components > is going out of its way to not generate "foo/./bar", but if > we are fixing canonicalize_path to be able to delete the "./", > that seems like a waste of code now. > > I

Re: drop tablespace failed when location contains .. on win32

2021-11-10 Thread Tom Lane
"wangsh.f...@fujitsu.com" writes: > I don't know this. After some test, I think it's better to consider > '/hoge/foo/bar' > as a absolute path. Agreed. I think we are considering "absolute path" here as a syntactic concept; Windows' weird rules about drive letters don't really matter for the

RE: drop tablespace failed when location contains .. on win32

2021-09-26 Thread wangsh.f...@fujitsu.com
Hi, > -Original Message- > From: Kyotaro Horiguchi > Sent: Monday, September 13, 2021 4:36 PM > To: mich...@paquier.xyz > Mmm. I haven't thought that so seriously, but '/hoge/foo/bar' doesn't > seem to be an absolute path on Windows since it lacks > ":" or "//hostname" part. If we're

Re: drop tablespace failed when location contains .. on win32

2021-09-13 Thread Kyotaro Horiguchi
At Mon, 13 Sep 2021 16:06:52 +0900, Michael Paquier wrote in > On Sun, Sep 12, 2021 at 07:33:23AM +, wangsh.f...@fujitsu.com wrote: > > 0001 is a small fix, because I find that is_absolute_path is not > > appropriate, > > see comment in skip_drive: > > > * On Windows, a path may begin

Re: drop tablespace failed when location contains .. on win32

2021-09-13 Thread Michael Paquier
On Sun, Sep 12, 2021 at 07:33:23AM +, wangsh.f...@fujitsu.com wrote: > 0001 is a small fix, because I find that is_absolute_path is not appropriate, > see comment in skip_drive: > > * On Windows, a path may begin with "C:" or "//network/". #define is_absolute_path(filename) \ ( \ -

RE: drop tablespace failed when location contains .. on win32

2021-09-12 Thread wangsh.f...@fujitsu.com
Hi, > -Original Message- > From: Andrew Dunstan > Sent: Thursday, September 9, 2021 8:30 PM > I think I would say that we should remove any "." or ".." element in the > path except at the beginning, and in the case of ".." also remove the > preceding element, unless someone can convince

Re: drop tablespace failed when location contains .. on win32

2021-09-09 Thread Andrew Dunstan
On 9/8/21 11:44 PM, Michael Paquier wrote: > On Thu, Sep 09, 2021 at 02:35:52AM +, wangsh.f...@fujitsu.com wrote: >> Do you mean changing the action of canonicalize_path(), like remove all the >> (..) ? >> >> I'm willing to fix this problem. > Looking at canonicalize_path(), we have already

Re: drop tablespace failed when location contains .. on win32

2021-09-08 Thread Kyotaro Horiguchi
At Thu, 9 Sep 2021 12:44:45 +0900, Michael Paquier wrote in > On Thu, Sep 09, 2021 at 02:35:52AM +, wangsh.f...@fujitsu.com wrote: > > Do you mean changing the action of canonicalize_path(), like remove all the > > (..) ? > > > > I'm willing to fix this problem. > > Looking at

Re: drop tablespace failed when location contains .. on win32

2021-09-08 Thread Michael Paquier
On Thu, Sep 09, 2021 at 02:35:52AM +, wangsh.f...@fujitsu.com wrote: > Do you mean changing the action of canonicalize_path(), like remove all the > (..) ? > > I'm willing to fix this problem. Looking at canonicalize_path(), we have already some logic around pending_strips to remove paths

RE: drop tablespace failed when location contains .. on win32

2021-09-08 Thread wangsh.f...@fujitsu.com
Hi, > -Original Message- > From: Michael Paquier > > On Wed, Sep 08, 2021 at 08:54:25AM -0400, Andrew Dunstan wrote: > > That seems like a bug. It's not very canonical :-) > > Yes, better to fix that. I fear that more places are impacted than > just the tablespace code paths. > -- >

Re: drop tablespace failed when location contains .. on win32

2021-09-08 Thread Michael Paquier
On Wed, Sep 08, 2021 at 08:54:25AM -0400, Andrew Dunstan wrote: > That seems like a bug. It's not very canonical :-) Yes, better to fix that. I fear that more places are impacted than just the tablespace code paths. -- Michael signature.asc Description: PGP signature

Re: drop tablespace failed when location contains .. on win32

2021-09-08 Thread Andrew Dunstan
On 9/8/21 6:16 AM, wangsh.f...@fujitsu.com wrote: > Hi, > > I find a problem related to tablespace on win32(server2019). > >> postgres=# create tablespace tbs location >> 'C:\Users\postgres\postgres_install\aa\..\aa'; >> CREATE TABLESPACE >> postgres=# create table tbl(col int) tablespace tbs;

drop tablespace failed when location contains .. on win32

2021-09-08 Thread wangsh.f...@fujitsu.com
Hi, I find a problem related to tablespace on win32(server2019). > postgres=# create tablespace tbs location > 'C:\Users\postgres\postgres_install\aa\..\aa'; > CREATE TABLESPACE > postgres=# create table tbl(col int) tablespace tbs; > ERROR: could not stat directory