HBase
  1. HBase
  2. HBASE-4361

Certain filter expressions fail in the shell

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 0.92.0
    • Fix Version/s: None
    • Component/s: Filters, shell
    • Labels:
      None

      Description

      Running the following in the shell hangs and then fails:

      scan 't1', { FILTER => "SingleColumnValueFilter(">", '1', 'f1', 'col_a')" }
      

      The error seems to be: org.jruby.exceptions.RaiseException: (NoMethodError) undefined method `write' for true:TrueClass

      1. small-improvements.txt
        2 kB
        Todd Lipcon
      2. Filter Language.docx
        134 kB
        Anirudh Todi

        Activity

        Hide
        Todd Lipcon added a comment -

        After hacking HBase to show a full stack trace:

        org.jruby.exceptions.RaiseException: (NoMethodError) undefined method `write' for true:TrueClassorg.jruby.exceptions.RaiseException: (NoMethodError) undefined method `write' for true:TrueClass
                at Hbase::Table.scan(/home/todd/git/hbase/bin/../bin/../src/main/ruby/hbase/table.rb:255)
                at Shell::Commands::Scan.command(/home/todd/git/hbase/bin/../bin/../src/main/ruby/shell/commands/scan.rb:61)
                at Shell::Commands::Scan.command_safe(/home/todd/git/hbase/bin/../bin/../src/main/ruby/shell/commands.rb:31)
                at Shell::Commands::Command.translate_hbase_exceptions(/home/todd/git/hbase/bin/../bin/../src/main/ruby/shell/commands.rb:70)
                at Shell::Commands::Command.command_safe(/home/todd/git/hbase/bin/../bin/../src/main/ruby/shell/commands.rb:31)
                at Shell::Shell.command(/home/todd/git/hbase/bin/../bin/../src/main/ruby/shell.rb:106)
        
        Show
        Todd Lipcon added a comment - After hacking HBase to show a full stack trace: org.jruby.exceptions.RaiseException: (NoMethodError) undefined method `write' for true:TrueClassorg.jruby.exceptions.RaiseException: (NoMethodError) undefined method `write' for true:TrueClass at Hbase::Table.scan(/home/todd/git/hbase/bin/../bin/../src/main/ruby/hbase/table.rb:255) at Shell::Commands::Scan.command(/home/todd/git/hbase/bin/../bin/../src/main/ruby/shell/commands/scan.rb:61) at Shell::Commands::Scan.command_safe(/home/todd/git/hbase/bin/../bin/../src/main/ruby/shell/commands.rb:31) at Shell::Commands::Command.translate_hbase_exceptions(/home/todd/git/hbase/bin/../bin/../src/main/ruby/shell/commands.rb:70) at Shell::Commands::Command.command_safe(/home/todd/git/hbase/bin/../bin/../src/main/ruby/shell/commands.rb:31) at Shell::Shell.command(/home/todd/git/hbase/bin/../bin/../src/main/ruby/shell.rb:106)
        Hide
        Todd Lipcon added a comment -

        Several problems here:
        1) I was using double-quotes twice, so it was passing "true" as the filter value. JRuby and its lovely lack of type checking then passed that through to the point where it tried to write true to the wire as a Writable, and failed.
        2) The documentation for SingleColumnValueFilter has the incorrect order of arguments.
        3) The errors given back by the filter parsing code are inscrutable.

        Show
        Todd Lipcon added a comment - Several problems here: 1) I was using double-quotes twice, so it was passing "true" as the filter value. JRuby and its lovely lack of type checking then passed that through to the point where it tried to write true to the wire as a Writable, and failed. 2) The documentation for SingleColumnValueFilter has the incorrect order of arguments. 3) The errors given back by the filter parsing code are inscrutable.
        Hide
        Todd Lipcon added a comment -

        For reference, the correct way to specify this is:
        scan 't1',

        { FILTER => "SingleColumnValueFilter('f1', 'col_a', >, 'binary:1')" }

        But I had to read the code for 30 minutes to figure it out. We need lots of docs updates on the filter language.

        Show
        Todd Lipcon added a comment - For reference, the correct way to specify this is: scan 't1', { FILTER => "SingleColumnValueFilter('f1', 'col_a', >, 'binary:1')" } But I had to read the code for 30 minutes to figure it out. We need lots of docs updates on the filter language.
        Hide
        Todd Lipcon added a comment -

        here are a few improvements. Still need to fix the docs, etc, to be correct. Ideally IMO the filter parsing would be done by javacc or antlr so we'd have a real grammar.

        Show
        Todd Lipcon added a comment - here are a few improvements. Still need to fix the docs, etc, to be correct. Ideally IMO the filter parsing would be done by javacc or antlr so we'd have a real grammar.
        Hide
        Anirudh Todi added a comment -

        Thanks Stack for pinging me about the Filter Language!

        1. Todd, I think your test failed because the way I wrote my code, the arguments needed to be enclosed in single quotes. I know it is in the documentation I sent out, however, it may not have been highlighted. I don't see how I would parse the filterString if the entire filterString is enclosed in double quotes and some of the arguments are enclosed in double quotes too?

        2. Regarding the incorrect order of arguments specified in the documentation for SingleColumnValueFilter, I apologize about that - the filter language went through several iterations and I may have overlooked updating the documentation in one place. I can take a look at it over the weekend and check the documentation again

        3. Better error messages? - I can try and improve this. Are there any specific points I can address to improve what is already there?

        4. Writing the filter parsing using antlr or javacc, I doubt I will have time to do that during the school semester. It's something I could potentially look into during the winter break. However, I leave this to your discretion.

        Hope the Filter Language is benefiting the community!

        Show
        Anirudh Todi added a comment - Thanks Stack for pinging me about the Filter Language! 1. Todd, I think your test failed because the way I wrote my code, the arguments needed to be enclosed in single quotes. I know it is in the documentation I sent out, however, it may not have been highlighted. I don't see how I would parse the filterString if the entire filterString is enclosed in double quotes and some of the arguments are enclosed in double quotes too? 2. Regarding the incorrect order of arguments specified in the documentation for SingleColumnValueFilter, I apologize about that - the filter language went through several iterations and I may have overlooked updating the documentation in one place. I can take a look at it over the weekend and check the documentation again 3. Better error messages? - I can try and improve this. Are there any specific points I can address to improve what is already there? 4. Writing the filter parsing using antlr or javacc, I doubt I will have time to do that during the school semester. It's something I could potentially look into during the winter break. However, I leave this to your discretion. Hope the Filter Language is benefiting the community!
        Hide
        Anirudh Todi added a comment -

        Updated the Filter Language document to include the error Todd caught.

        Also updated some of the examples.

        However, I didn't notice any other error.

        Show
        Anirudh Todi added a comment - Updated the Filter Language document to include the error Todd caught. Also updated some of the examples. However, I didn't notice any other error.
        Hide
        Todd Lipcon added a comment -

        I don't see how I would parse the filterString if the entire filterString is enclosed in double quotes and some of the arguments are enclosed in double quotes too?

        Yep, that was my mistake But the shell didn't do a good job of showing me my error. Dynamic typing FTL My patch here at least changes the shell to say: "Invalid filter: false" instead of the strange exception I got before.

        Better error messages? - I can try and improve this. Are there any specific points I can address to improve what is already there

        In general, all of the exceptions thrown should tell you which text caused them to throw. For example, "Invalid quoted string" is useless, but "Invalid quoted string: foo" is useful. "Mismatched parenthesis" should include the offset in the string at which it was found. "Incorrect Filter String" if the stack is non-empty at the end of parseFilterString should probably say something like "Incorrect filter string: unmatched LPAREN at offset <n>". Pretty much all of the places where IllegalArgumentException are thrown in ParseFilter could be improved to have (a) the offset at which the error occurred, and (b) details about what was incorrect.

        All of the above aside, it's a nice feature once you get it to work Thanks for the hard work implementing it.

        Show
        Todd Lipcon added a comment - I don't see how I would parse the filterString if the entire filterString is enclosed in double quotes and some of the arguments are enclosed in double quotes too? Yep, that was my mistake But the shell didn't do a good job of showing me my error. Dynamic typing FTL My patch here at least changes the shell to say: "Invalid filter: false" instead of the strange exception I got before. Better error messages? - I can try and improve this. Are there any specific points I can address to improve what is already there In general, all of the exceptions thrown should tell you which text caused them to throw. For example, "Invalid quoted string" is useless, but "Invalid quoted string: foo" is useful. "Mismatched parenthesis" should include the offset in the string at which it was found. "Incorrect Filter String" if the stack is non-empty at the end of parseFilterString should probably say something like "Incorrect filter string: unmatched LPAREN at offset <n>". Pretty much all of the places where IllegalArgumentException are thrown in ParseFilter could be improved to have (a) the offset at which the error occurred, and (b) details about what was incorrect. All of the above aside, it's a nice feature once you get it to work Thanks for the hard work implementing it.
        Hide
        Ted Yu added a comment -

        Moving the remaining work to 0.94

        Show
        Ted Yu added a comment - Moving the remaining work to 0.94
        Hide
        Lars Hofhansl added a comment -

        Shuffling it on to 0.96.

        Show
        Lars Hofhansl added a comment - Shuffling it on to 0.96.
        Hide
        Ted Yu added a comment -

        No assignee.

        Show
        Ted Yu added a comment - No assignee.
        Hide
        stack added a comment -

        Not being worked on. Moving out.

        Show
        stack added a comment - Not being worked on. Moving out.

          People

          • Assignee:
            Unassigned
            Reporter:
            Todd Lipcon
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:

              Development