Hadoop Common
  1. Hadoop Common
  2. HADOOP-7945

Document that Path objects do not support ":" in them.

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Critical Critical
    • Resolution: Unresolved
    • Affects Version/s: 0.20.0
    • Fix Version/s: None
    • Component/s: None
    • Labels:

      Description

      Until HADOOP-3257 is fixed, this particular exclusion should be documented. This is a major upsetter to many beginners.

        Issue Links

          Activity

          Hide
          Harsh J added a comment -

          Am thinking it might just be better to document this and add a test case (that'd fail the day its fixed).

          Maybe this approach could cause incompatibility, as am not sure if there are ways to 'tweak' the Path into accepting ":" (escaped/encoded).

          Another character that causes issue is a slash in the filename itself, even if escaped/encoded.

          Show
          Harsh J added a comment - Am thinking it might just be better to document this and add a test case (that'd fail the day its fixed). Maybe this approach could cause incompatibility, as am not sure if there are ways to 'tweak' the Path into accepting ":" (escaped/encoded). Another character that causes issue is a slash in the filename itself, even if escaped/encoded.
          Hide
          Niels Basjes added a comment -

          I added a unit test to force to trigger the error. It passes this and all existing unit tests.
          So I think I found the right spot.
          Harsh, can you check it?

          Show
          Niels Basjes added a comment - I added a unit test to force to trigger the error. It passes this and all existing unit tests. So I think I found the right spot. Harsh, can you check it?
          Hide
          Harsh J added a comment -

          Nope, given checkPathArg's positioning, that wouldn't work for path that are fully qualified (i.e. need a ":" as part of the file:///path/to/some/file/:/with/colon).

          Show
          Harsh J added a comment - Nope, given checkPathArg's positioning, that wouldn't work for path that are fully qualified (i.e. need a ":" as part of the file:///path/to/some/file/:/with/colon ).
          Hide
          Niels Basjes added a comment -

          Sorry, that was unreadable.

          diff --git hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java
          index 81c79db..e135efb 100644
          --- hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java
          +++ hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java
          @@ -90,7 +90,13 @@ public class Path implements Comparable {
               if( path.length() == 0 ) {
                  throw new IllegalArgumentException(
                      "Can not create a Path from an empty string");
          -    }   
          +    }
          +    if( path.contains(":") ) {
          +      throw new IllegalArgumentException(
          +          "Can not create Path because Path is limited by URI semantics "+
          +          "in the sense that one cannot create files whose names include "+
          +          "characters such as \":\" etc.");
          +    }
             }
             
             /** Construct a path from a String.  Path strings are URIs, but with
          
          
          Show
          Niels Basjes added a comment - Sorry, that was unreadable. diff --git hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java index 81c79db..e135efb 100644 --- hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java +++ hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java @@ -90,7 +90,13 @@ public class Path implements Comparable { if ( path.length() == 0 ) { throw new IllegalArgumentException( "Can not create a Path from an empty string" ); - } + } + if ( path.contains( ":" ) ) { + throw new IllegalArgumentException( + "Can not create Path because Path is limited by URI semantics " + + "in the sense that one cannot create files whose names include " + + "characters such as \" :\ " etc." ); + } } /** Construct a path from a String . Path strings are URIs, but with
          Hide
          Niels Basjes added a comment -

          Quick shot: Simply add another check to the already existing checkPathArg method in the Path class.
          I.e. something like this (I never tested this code):

          diff --git hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java
          index 81c79db..e135efb 100644
          — hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java
          +++ hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java
          @@ -90,7 +90,13 @@ public class Path implements Comparable {
          if( path.length() == 0 )

          { throw new IllegalArgumentException( "Can not create a Path from an empty string"); - }


          + }
          + if( path.contains(":") )

          { + throw new IllegalArgumentException( + "Can not create Path because Path is limited by URI semantics "+ + "in the sense that one cannot create files whose names include "+ + "characters such as \":\" etc."); + }

          }

          /** Construct a path from a String. Path strings are URIs, but with

          Show
          Niels Basjes added a comment - Quick shot: Simply add another check to the already existing checkPathArg method in the Path class. I.e. something like this (I never tested this code): diff --git hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java index 81c79db..e135efb 100644 — hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java +++ hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java @@ -90,7 +90,13 @@ public class Path implements Comparable { if( path.length() == 0 ) { throw new IllegalArgumentException( "Can not create a Path from an empty string"); - } + } + if( path.contains(":") ) { + throw new IllegalArgumentException( + "Can not create Path because Path is limited by URI semantics "+ + "in the sense that one cannot create files whose names include "+ + "characters such as \":\" etc."); + } } /** Construct a path from a String. Path strings are URIs, but with
          Hide
          Harsh J added a comment -

          I guess that could bring in compatibility issues. (People begin expecting that to fail, and at some point in future a bug fix like HADOOP-3257 regresses against that thought.) But otherwise, that idea is sound.

          Do we emit no "Invalid char in path" form of messages today? If we do, then this is not a worry, we could inject looking for colon in that somehow.

          Want to take a shot at it Niels?

          Show
          Harsh J added a comment - I guess that could bring in compatibility issues. (People begin expecting that to fail, and at some point in future a bug fix like HADOOP-3257 regresses against that thought.) But otherwise, that idea is sound. Do we emit no "Invalid char in path" form of messages today? If we do, then this is not a worry, we could inject looking for colon in that somehow. Want to take a shot at it Niels?
          Hide
          Niels Basjes added a comment -

          How about adding simple input check in Path that throws an exception with a descriptive message. That way the log files of the job/task contain the reason for the failure.

          Show
          Niels Basjes added a comment - How about adding simple input check in Path that throws an exception with a descriptive message. That way the log files of the job/task contain the reason for the failure.
          Hide
          Harsh J added a comment -

          We can document this in Path's javadocs, I think that should suffice. But brownie points if you can think of somewhere more visible and relevant

          Show
          Harsh J added a comment - We can document this in Path's javadocs, I think that should suffice. But brownie points if you can think of somewhere more visible and relevant

            People

            • Assignee:
              Unassigned
              Reporter:
              Harsh J
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:

                Development