Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.3.0, 1.4.0
    • Component/s: Table API & SQL
    • Labels:
      None

      Description

      TableAPI testAllPassingFilter:

       
         val t = util.addTable[(Int, Long, String)]('int, 'long, 'string)
          val resScala = t.filter(Literal(true)).select('int as 'myInt, 'string)
          val resJava = t.filter("TrUe").select("int as myInt, string")
      

      We got error:

      org.apache.flink.table.api.ValidationException: Cannot resolve [TrUe] given input [int, long, string].
      

      The error is caused by :

          lazy val boolLiteral: PackratParser[Expression] = ("true" | "false") ^^ {
          str => Literal(str.toBoolean)
        }
      

      I want improve the method as follow:

       lazy val boolLiteral: PackratParser[Expression] =
          ("(t|T)(r|R)(u|U)(e|E)".r | "(f|F)(a|A)(l|L)(s|S)(e|E)".r) ^^ { str => Literal(str.toBoolean)}
      

      Is there any drawback to this improvement? Welcome anyone feedback ?

        Issue Links

          Activity

          Hide
          twalthr Timo Walther added a comment -

          Fixed in 1.4: 9a9e193a1edf335c89804dc3643fc02681570821
          Fixed in 1.3: 95fce15483fe6b12adc465a0b709362b8ea7fd0c

          Show
          twalthr Timo Walther added a comment - Fixed in 1.4: 9a9e193a1edf335c89804dc3643fc02681570821 Fixed in 1.3: 95fce15483fe6b12adc465a0b709362b8ea7fd0c
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/flink/pull/3944

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/3944
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user twalthr commented on the issue:

          https://github.com/apache/flink/pull/3944

          Thanks @sunjincheng121. Yes, I think a keyword is a nicer approach. I will add it and merge your tests.

          Show
          githubbot ASF GitHub Bot added a comment - Github user twalthr commented on the issue: https://github.com/apache/flink/pull/3944 Thanks @sunjincheng121. Yes, I think a keyword is a nicer approach. I will add it and merge your tests.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user sunjincheng121 commented on the issue:

          https://github.com/apache/flink/pull/3944

          Hi, @twalthr In this PR. I want change the method `boolLiteral` of `ExpressionParser`. @fhueske had a suggestion that we should add "true" and "false" as keywords to the parser. Because Keywords are parsed case-insensitive. And I appreciate If you can give us some suggestions.

          Thanks,
          SunJincheng

          Show
          githubbot ASF GitHub Bot added a comment - Github user sunjincheng121 commented on the issue: https://github.com/apache/flink/pull/3944 Hi, @twalthr In this PR. I want change the method `boolLiteral` of `ExpressionParser`. @fhueske had a suggestion that we should add "true" and "false" as keywords to the parser. Because Keywords are parsed case-insensitive. And I appreciate If you can give us some suggestions. Thanks, SunJincheng
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user sunjincheng121 opened a pull request:

          https://github.com/apache/flink/pull/3944

          FLINK-6632[table]Improved the method BoolLiteral of ExpressionParse…

          This PR. only improved the method `BoolLiteral` of `ExpressionParser` for case insensitive.

          • [x] General
          • The pull request references the related JIRA issue ("FLINK-6632[table]Improved the method BoolLiteral of ExpressionParser for case insensitive.")
          • The pull request addresses only one issue
          • Each commit in the PR has a meaningful commit message (including the JIRA id)
          • [ ] Documentation
          • Documentation has been added for new functionality
          • Old documentation affected by the pull request has been updated
          • JavaDoc for public methods has been added
          • [x] Tests & Build
          • Functionality added by the pull request is covered by tests
          • `mvn clean verify` has been executed successfully locally or a Travis build has passed

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/sunjincheng121/flink FLINK-6632-PR

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/flink/pull/3944.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #3944


          commit 959af858fb0345c8d28254a3984d58a39e0405c6
          Author: sunjincheng121 <sunjincheng121@gmail.com>
          Date: 2017-05-19T10:24:09Z

          FLINK-6632[table]Improved the method BoolLiteral of ExpressionParser for case insensitive.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user sunjincheng121 opened a pull request: https://github.com/apache/flink/pull/3944 FLINK-6632 [table] Improved the method BoolLiteral of ExpressionParse… This PR. only improved the method `BoolLiteral` of `ExpressionParser` for case insensitive. [x] General The pull request references the related JIRA issue (" FLINK-6632 [table] Improved the method BoolLiteral of ExpressionParser for case insensitive.") The pull request addresses only one issue Each commit in the PR has a meaningful commit message (including the JIRA id) [ ] Documentation Documentation has been added for new functionality Old documentation affected by the pull request has been updated JavaDoc for public methods has been added [x] Tests & Build Functionality added by the pull request is covered by tests `mvn clean verify` has been executed successfully locally or a Travis build has passed You can merge this pull request into a Git repository by running: $ git pull https://github.com/sunjincheng121/flink FLINK-6632 -PR Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/3944.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #3944 commit 959af858fb0345c8d28254a3984d58a39e0405c6 Author: sunjincheng121 <sunjincheng121@gmail.com> Date: 2017-05-19T10:24:09Z FLINK-6632 [table] Improved the method BoolLiteral of ExpressionParser for case insensitive.
          Hide
          fhueske Fabian Hueske added a comment -

          I think a better approach to solve this would be to add "true" and "false" as keywords to the parser. Keywords are parsed case-insensitive.
          What do you think Timo Walther?

          Show
          fhueske Fabian Hueske added a comment - I think a better approach to solve this would be to add "true" and "false" as keywords to the parser. Keywords are parsed case-insensitive. What do you think Timo Walther ?

            People

            • Assignee:
              sunjincheng121 sunjincheng
              Reporter:
              sunjincheng121 sunjincheng
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development