File globbing with a PathFilter is too restrictive

• Key: HADOOP-3497
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Tom White
• Reporter: Tom White
• Component/s: fs
• Affects Version/s: 0.17.0
• Fix Version/s: 0.20.0

Description

Consider the file hierarchy

/a
/a/b

Calling the globStatus method on FileSystem with a path of

/*/*

and a PathFilter that only accepts /a/b returns no matches. It should return a single match: /a/b.

Comments

[Tom White]

Here's a test that exposes the issue. The problem is that the testing is done per path component, so since the parent directory /a doesn't match the filter the whole path /a/b is rejected.

[Tom White]

Patch for review. I've removed the user filtering at each level in a path, instead the filtering is done on the full path name after the globbing step.

[Hairong Kuang]

Tom, nice change! Two comments:
1. I think the checking if paths are accepted by the filter or not (lines 932-936 in Filesystem.java) should be performed no matter last component of the path pattern has a pattern or not. Your patch does the checking only when last component does not have a pattern.
2. This patch probably should be marked as an incompatible change. The trunk implements a wrong semantics.

[Tom White]

New patch to address Hairong's comments.

[Hairong Kuang]

+1. The patch looks good to me.

[Tom White]

Resubmitting to hudson.

[Hadoop QA]

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12388141/hadoop-3497-v2.patch
against trunk revision 686420.

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 3 new or modified tests.

-1 javadoc. The javadoc tool appears to have generated 1 warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 findbugs. The patch does not introduce any new Findbugs warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

-1 core tests. The patch failed core unit tests.

-1 contrib tests. The patch failed contrib unit tests.

Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3064/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3064/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3064/artifact/trunk/build/test/checkstyle-errors.html
Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3064/console

This message is automatically generated.

[Tom White]

Retrying on hudson (it didn't run properly last time).

[Hadoop QA]

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12388141/hadoop-3497-v2.patch
against trunk revision 689380.

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 3 new or modified tests.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 findbugs. The patch does not introduce any new Findbugs warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

-1 core tests. The patch failed core unit tests.

-1 contrib tests. The patch failed contrib unit tests.

Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3122/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3122/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3122/artifact/trunk/build/test/checkstyle-errors.html
Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3122/console

This message is automatically generated.

[Tom White]

Cancelling while test failures are investigated.

[Tom White]

The test that is failing is TestFileInputFormatPathFilter#testWithPathFilterWithoutGlob. This creates files named a, b, aa, bb in a directory, then uses an input format with a filter that only accepts files whose last component is 1 character long. Only files a and b should match. The input path is the directory, not a glob path, and to work it relies on the following following behaviour of FileSystem#globStatus.

If you call FileSystem#globStatus(Path pathPattern, PathFilter filter) with a pathPattern that has a fixed (non-globbing) final component, then the status for that path will always be returned, regardless of the filter.

So, for a path /a which exists

fs.globStatus(new Path("/a"), new PathFilter() {
  @Override
  public boolean accept(Path path) {
    return false;
  }})

will return the status for /a, even though the filter rejects every path!

This seems wrong, and should really be changed. It has a potential impact on applications however, since a filter is now being applied that previously wasn't. Does this seem the right thing to do?

I've attached a patch which fixes the test.

[Hairong Kuang]

+1 on the change. Tom, could you please put the above semantic change in the release note?

[Tom White]

Successfully ran the unit tests and test-patch

     [exec] +1 overall.  
     [exec] 
     [exec]     +1 @author.  The patch does not contain any @author tags.
     [exec] 
     [exec]     +1 tests included.  The patch appears to include 6 new or modified tests.
     [exec] 
     [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
     [exec] 
     [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
     [exec] 
     [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
     [exec] 
     [exec]     +1 Eclipse classpath. The patch retains Eclipse classpath integrity.

[Tom White]

I've just committed this.

[Hudson]

Integrated in Hadoop-trunk #680 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/680/)