You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by sarutak <gi...@git.apache.org> on 2014/10/15 23:32:19 UTC

[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

GitHub user sarutak opened a pull request:

    https://github.com/apache/spark/pull/2816

    [SPARK-3959][SPARK-3960][SQL] SqlParser fails to parse literal -9223372036854775808 (Long.MinValue). / We can apply unary minus only to literal.

    SqlParser fails to parse -9223372036854775808 (Long.MinValue) so we cannot write queries such like as follows.
    
        SELECT value FROM someTable WHERE value > -9223372036854775808
    
    Additionally, because of the wrong syntax definition, we cannot apply unary minus only to literal. So, we cannot write such expressions.
    
        -(value1 + value2) // Parenthesized expressions
        -column // Columns
        -MAX(column) // Functions

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

    $ git pull https://github.com/sarutak/spark spark-sql-dsl-improvement2

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

    https://github.com/apache/spark/pull/2816.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 #2816
    
----
commit 6e0b6d7361b630994219d4efea66c8c38e90531a
Author: Kousuke Saruta <sa...@oss.nttdata.co.jp>
Date:   2014-10-15T11:06:21Z

    WIP

commit 54b12a1de6d89caeec5efe54d3a1c7e738c82e9a
Author: Kousuke Saruta <sa...@oss.nttdata.co.jp>
Date:   2014-10-15T21:25:51Z

    Added some more tests to SQLQuerySuite

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2816#discussion_r18953740
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/SqlParser.scala ---
    @@ -301,17 +301,67 @@ class SqlParser extends AbstractSparkSQLParser {
         CAST ~ "(" ~> expression ~ (AS ~> dataType) <~ ")" ^^ { case exp ~ t => Cast(exp, t) }
     
       protected lazy val literal: Parser[Literal] =
    -    ( numericLit ^^ {
    -        case i if i.toLong > Int.MaxValue => Literal(i.toLong)
    -        case i => Literal(i.toInt)
    -      }
    +    ( numericLiteral
         | NULL ^^^ Literal(null, NullType)
    -    | floatLit ^^ {case f => Literal(f.toDouble) }
    +    | booleanLiteral
         | stringLit ^^ {case s => Literal(s, StringType) }
         )
     
    +  protected lazy val booleanLiteral: Parser[Literal] =
    +    ( TRUE ^^^ Literal(true, BooleanType)
    +    | FALSE ^^^ Literal(false, BooleanType)
    +    )
    +
    +  protected lazy val numericLiteral: Parser[Literal] =
    +    signedNumericLiteral | unsignedNumericLiteral | floatLit ^^ { f => Literal(f.toDouble) }
    +
    +  protected lazy val sign: Parser[String] =
    +    "+" | "-"
    +
    +  protected lazy val signedNumericLiteral: Parser[Literal] =
    +    sign ~ numericLit  ^^ { case s ~ l => Literal(getProperTypeInt(s + l)) }
    +
    +
    +  protected lazy val unsignedNumericLiteral: Parser[Literal] =
    +    numericLit ^^ { n => Literal(getProperTypeInt(n)) }
    +
    +
    +  private val longMax = BigDecimal(s"${Long.MaxValue}")
    +  private val longMin = BigDecimal(s"${Long.MinValue}")
    +  private val intMax = BigDecimal(s"${Int.MaxValue}")
    +  private val intMin = BigDecimal(s"${Int.MinValue}")
    +
    +  private def getProperTypeInt(value: String) = {
    +    val bigIntValue = BigDecimal(value)
    +
    +    if (value.startsWith("-")) {
    +      if (bigIntValue < intMin) {
    +        if (bigIntValue < longMin) {
    +          bigIntValue
    +        } else {
    +          value.toLong
    +        }
    +      } else {
    +        value.toInt
    +      }
    +    } else {
    +      if (bigIntValue > intMax) {
    +        if (bigIntValue > longMax) {
    +          bigIntValue
    +        } else {
    +          value.toLong
    +        }
    +      } else {
    +        value.toInt
    +      }
    +    }
    --- End diff --
    
    How about this:
    
    ```scala
    bigIntValue match {
      case v if v < longMin || v > longMax => v
      case v if v < intMin  || v > intMax  => v.toLong
      case v                               => v.toInt
    }
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59652379
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21892/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60475566
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22204/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2816#discussion_r18952854
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/SqlParser.scala ---
    @@ -301,17 +301,67 @@ class SqlParser extends AbstractSparkSQLParser {
         CAST ~ "(" ~> expression ~ (AS ~> dataType) <~ ")" ^^ { case exp ~ t => Cast(exp, t) }
     
       protected lazy val literal: Parser[Literal] =
    -    ( numericLit ^^ {
    -        case i if i.toLong > Int.MaxValue => Literal(i.toLong)
    -        case i => Literal(i.toInt)
    -      }
    +    ( numericLiteral
         | NULL ^^^ Literal(null, NullType)
    -    | floatLit ^^ {case f => Literal(f.toDouble) }
    +    | booleanLiteral
         | stringLit ^^ {case s => Literal(s, StringType) }
         )
     
    +  protected lazy val booleanLiteral: Parser[Literal] =
    +    ( TRUE ^^^ Literal(true, BooleanType)
    +    | FALSE ^^^ Literal(false, BooleanType)
    +    )
    +
    +  protected lazy val numericLiteral: Parser[Literal] =
    +    signedNumericLiteral | unsignedNumericLiteral | floatLit ^^ { f => Literal(f.toDouble) }
    +
    +  protected lazy val sign: Parser[String] =
    +    "+" | "-"
    +
    +  protected lazy val signedNumericLiteral: Parser[Literal] =
    +    sign ~ numericLit  ^^ { case s ~ l => Literal(getProperTypeInt(s + l)) }
    +
    +
    +  protected lazy val unsignedNumericLiteral: Parser[Literal] =
    +    numericLit ^^ { n => Literal(getProperTypeInt(n)) }
    +
    +
    +  private val longMax = BigDecimal(s"${Long.MaxValue}")
    +  private val longMin = BigDecimal(s"${Long.MinValue}")
    +  private val intMax = BigDecimal(s"${Int.MaxValue}")
    +  private val intMin = BigDecimal(s"${Int.MinValue}")
    +
    +  private def getProperTypeInt(value: String) = {
    +    val bigIntValue = BigDecimal(value)
    +
    +    if (value.startsWith("-")) {
    +      if (bigIntValue < intMin) {
    +        if (bigIntValue < longMin) {
    +          bigIntValue
    +        } else {
    +          value.toLong
    +        }
    +      } else {
    +        value.toInt
    +      }
    +    } else {
    +      if (bigIntValue > intMax) {
    +        if (bigIntValue > longMax) {
    +          bigIntValue
    +        } else {
    +          value.toLong
    +        }
    +      } else {
    +        value.toInt
    +      }
    +    }
    +  }
    +
       protected lazy val floatLit: Parser[String] =
    -    elem("decimal", _.isInstanceOf[lexical.FloatLit]) ^^ (_.chars)
    +    ( opt(sign) ~ "." ~ unsignedNumericLiteral ^^ {
    --- End diff --
    
    I'd prefer the `sign.?` syntax here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2816#discussion_r18934453
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/SqlParser.scala ---
    @@ -323,13 +327,38 @@ class SqlParser extends AbstractSparkSQLParser {
         | cast
         | "(" ~> expression <~ ")"
         | function
    -    | "-" ~> literal ^^ UnaryMinus
    +    | signedNumericLiteral
    +    | sign ~> expression ^^ UnaryMinus
    --- End diff --
    
    This shoud be `"-" ~> expression ^^ UnaryMinus`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60533657
  
      [Test build #22259 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22259/consoleFull) for   PR 2816 at commit [`32a5005`](https://github.com/apache/spark/commit/32a5005cc9100ad16580a09c218b96b3cdd04636).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2816#discussion_r18952637
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/SqlParser.scala ---
    @@ -301,17 +301,67 @@ class SqlParser extends AbstractSparkSQLParser {
         CAST ~ "(" ~> expression ~ (AS ~> dataType) <~ ")" ^^ { case exp ~ t => Cast(exp, t) }
     
       protected lazy val literal: Parser[Literal] =
    -    ( numericLit ^^ {
    -        case i if i.toLong > Int.MaxValue => Literal(i.toLong)
    -        case i => Literal(i.toInt)
    -      }
    +    ( numericLiteral
    --- End diff --
    
    Please keep the naming convention consistent, either use `numericLit` or rename all `xxxLit` to `xxxLiteral`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59652376
  
      [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21892/consoleFull) for   PR 2816 at commit [`5c847ac`](https://github.com/apache/spark/commit/5c847aca4e7d618dee7b8c647bdca6f845d328e3).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59399446
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21809/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2816#discussion_r18953268
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/SqlParser.scala ---
    @@ -323,11 +373,11 @@ class SqlParser extends AbstractSparkSQLParser {
         | cast
         | "(" ~> expression <~ ")"
         | function
    -    | "-" ~> literal ^^ UnaryMinus
         | dotExpressionHeader
         | ident ^^ UnresolvedAttribute
         | "*" ^^^ Star(None)
         | literal
    +    | sign ~ expression ^^ { case s ~ e => if (s == "-") UnaryMinus(e) else e }
    --- End diff --
    
    Sign should be optional. Also I think it would be better to move this rule to the last alternative of `comparisonExpression`, namely from:
    
    ```scala
    | termExpression
    ```
    
    to
    
    ```scala
    | sign.? ~ termExpression ^^ {
        case Some("-") ~ e => UnaryMinus(e)
        case _ => e
      }
    ```
    
    The reason is that the type of `termExpression` is guaranteed to be numeric.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by sarutak <gi...@git.apache.org>.
Github user sarutak commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59650806
  
    retest this please.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59857250
  
      [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21938/consoleFull) for   PR 2816 at commit [`025374c`](https://github.com/apache/spark/commit/025374cc90790278e7d10cb06cc7976fd177a941).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59650495
  
      [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21890/consoleFull) for   PR 2816 at commit [`5c847ac`](https://github.com/apache/spark/commit/5c847aca4e7d618dee7b8c647bdca6f845d328e3).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60518234
  
      [Test build #22246 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22246/consoleFull) for   PR 2816 at commit [`8c07bc7`](https://github.com/apache/spark/commit/8c07bc763827f873bc3192d50bda88c786754c9f).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59399435
  
      [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21809/consoleFull) for   PR 2816 at commit [`938ba13`](https://github.com/apache/spark/commit/938ba13757b0979fd93185f901f1ebc26efb9caf).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60517037
  
      [Test build #22244 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22244/consoleFull) for   PR 2816 at commit [`13b78fe`](https://github.com/apache/spark/commit/13b78fe18e5646c9680f1d655c1ecc10247961b8).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59724233
  
      [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21913/consoleFull) for   PR 2816 at commit [`a580dd4`](https://github.com/apache/spark/commit/a580dd436d007265ed6cdba9666f9d27e3025f57).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59857551
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21938/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60055358
  
      [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22029/consoleFull) for   PR 2816 at commit [`58be459`](https://github.com/apache/spark/commit/58be459c9d9cd69aa3d9007b102dddf83237f034).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2816#discussion_r19001459
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/SqlParser.scala ---
    @@ -301,33 +301,75 @@ class SqlParser extends AbstractSparkSQLParser {
         CAST ~ "(" ~> expression ~ (AS ~> dataType) <~ ")" ^^ { case exp ~ t => Cast(exp, t) }
     
       protected lazy val literal: Parser[Literal] =
    -    ( numericLit ^^ {
    -        case i if i.toLong > Int.MaxValue => Literal(i.toLong)
    -        case i => Literal(i.toInt)
    -      }
    -    | NULL ^^^ Literal(null, NullType)
    -    | floatLit ^^ {case f => Literal(f.toDouble) }
    +    ( signedNumericLiteral
    +    | unsignedNumericLiteral
    --- End diff --
    
    We could just use `numericLiteral` here to replace above 2 lines.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60519191
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22246/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60533154
  
      [Test build #454 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/454/consoleFull) for   PR 2816 at commit [`c2bab5e`](https://github.com/apache/spark/commit/c2bab5ebbcdc3c7ed1d66e9b719109f66b326e58).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60534496
  
      [Test build #454 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/454/consoleFull) for   PR 2816 at commit [`c2bab5e`](https://github.com/apache/spark/commit/c2bab5ebbcdc3c7ed1d66e9b719109f66b326e58).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by sarutak <gi...@git.apache.org>.
Github user sarutak commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2816#discussion_r18933954
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/SqlParser.scala ---
    @@ -300,11 +300,15 @@ class SqlParser extends AbstractSparkSQLParser {
       protected lazy val cast: Parser[Expression] =
         CAST ~ "(" ~> expression ~ (AS ~> dataType) <~ ")" ^^ { case exp ~ t => Cast(exp, t) }
     
    -  protected lazy val literal: Parser[Literal] =
    -    ( numericLit ^^ {
    -        case i if i.toLong > Int.MaxValue => Literal(i.toLong)
    -        case i => Literal(i.toInt)
    -      }
    +  protected lazy val generalLiteral: Parser[Literal] =
    +    ( unsignedNumericLiteral ^^ {
    +      case n =>
    --- End diff --
    
    In the original code, variable i is always unsigned numeric expressed as String. If i is 9223372036854775808, i.toLong should fail because Long is between -9223372036854775808 and 9223372036854775807.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59470604
  
      [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21846/consoleFull) for   PR 2816 at commit [`96cfcbb`](https://github.com/apache/spark/commit/96cfcbb8c7b3c5de4eae599f92c5c0d659b22ab7).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59351094
  
      [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21803/consoleFull) for   PR 2816 at commit [`735c1fe`](https://github.com/apache/spark/commit/735c1fed46c14158698d4c2434d7c5f2e3e24213).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59281039
  
      [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21778/consoleFull) for   PR 2816 at commit [`54b12a1`](https://github.com/apache/spark/commit/54b12a1de6d89caeec5efe54d3a1c7e738c82e9a).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59287331
  
      [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21778/consoleFull) for   PR 2816 at commit [`54b12a1`](https://github.com/apache/spark/commit/54b12a1de6d89caeec5efe54d3a1c7e738c82e9a).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by sarutak <gi...@git.apache.org>.
Github user sarutak commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59467617
  
    retest this please.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59462076
  
      [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21840/consoleFull) for   PR 2816 at commit [`56817b1`](https://github.com/apache/spark/commit/56817b196136447f26c26c2b4088c408acd66cdc).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60049567
  
      [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22029/consoleFull) for   PR 2816 at commit [`58be459`](https://github.com/apache/spark/commit/58be459c9d9cd69aa3d9007b102dddf83237f034).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by sarutak <gi...@git.apache.org>.
Github user sarutak commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60475704
  
    retest this please.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59865472
  
      [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21942/consoleFull) for   PR 2816 at commit [`025374c`](https://github.com/apache/spark/commit/025374cc90790278e7d10cb06cc7976fd177a941).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59287338
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21778/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60517842
  
      [Test build #22244 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22244/consoleFull) for   PR 2816 at commit [`13b78fe`](https://github.com/apache/spark/commit/13b78fe18e5646c9680f1d655c1ecc10247961b8).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2816#discussion_r18933714
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/SqlParser.scala ---
    @@ -300,11 +300,15 @@ class SqlParser extends AbstractSparkSQLParser {
       protected lazy val cast: Parser[Expression] =
         CAST ~ "(" ~> expression ~ (AS ~> dataType) <~ ")" ^^ { case exp ~ t => Cast(exp, t) }
     
    -  protected lazy val literal: Parser[Literal] =
    -    ( numericLit ^^ {
    -        case i if i.toLong > Int.MaxValue => Literal(i.toLong)
    -        case i => Literal(i.toInt)
    -      }
    +  protected lazy val generalLiteral: Parser[Literal] =
    +    ( unsignedNumericLiteral ^^ {
    +      case n =>
    --- End diff --
    
    why not follow the origin format `case i if i.toLong > Int.MaxValue => Literal(i.toLong)` which looks compact.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59469454
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21845/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60517844
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22244/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59304268
  
    Hi, @sarutak , nice catch for this problem! And I have some small suggestion for the code:
    
        protected lazy val positiveNumericLiteral: Parser[Literal] =
          "+".? ~> numericLit ^^ {
            case i if i.toLong > Int.MaxValue => Literal(i.toLong)
            case i => Literal(i.toInt)
          }
    
        protected lazy val negativeNumericLiteral: Parser[Literal] =
          "-" ~> numericLit ^^ {
            case i if s"-$i".toLong < Int.MinValue => Literal(s"-$i".toLong)
            case i => Literal(s"-$i".toInt)
          }
    Then we can rewrite `literal` as:
    
        protected lazy val literal: Parser[Literal] =
          ( positiveNumericLiteral
          | negativeNumericLiteral
          | NULL ^^^ Literal(null, NullType)
          | floatLit ^^ {case f => Literal(f.toDouble) }
          | stringLit ^^ {case s => Literal(s, StringType) }
        )
    Finally, add `"-" ~> expression ^^ UnaryMinus` and `"+" ~> expression` to `baseExpression`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2816#discussion_r18952827
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/SqlParser.scala ---
    @@ -301,17 +301,67 @@ class SqlParser extends AbstractSparkSQLParser {
         CAST ~ "(" ~> expression ~ (AS ~> dataType) <~ ")" ^^ { case exp ~ t => Cast(exp, t) }
     
       protected lazy val literal: Parser[Literal] =
    -    ( numericLit ^^ {
    -        case i if i.toLong > Int.MaxValue => Literal(i.toLong)
    -        case i => Literal(i.toInt)
    -      }
    +    ( numericLiteral
         | NULL ^^^ Literal(null, NullType)
    -    | floatLit ^^ {case f => Literal(f.toDouble) }
    +    | booleanLiteral
         | stringLit ^^ {case s => Literal(s, StringType) }
         )
     
    +  protected lazy val booleanLiteral: Parser[Literal] =
    +    ( TRUE ^^^ Literal(true, BooleanType)
    +    | FALSE ^^^ Literal(false, BooleanType)
    +    )
    +
    +  protected lazy val numericLiteral: Parser[Literal] =
    +    signedNumericLiteral | unsignedNumericLiteral | floatLit ^^ { f => Literal(f.toDouble) }
    +
    +  protected lazy val sign: Parser[String] =
    +    "+" | "-"
    +
    +  protected lazy val signedNumericLiteral: Parser[Literal] =
    +    sign ~ numericLit  ^^ { case s ~ l => Literal(getProperTypeInt(s + l)) }
    --- End diff --
    
    Sign should be optional:
    
    ```scala
    sign.? ~ numericLit ^^
    { case s ~ l => Literal(getProperTypeInt(s.getOrElse("") + l)) }
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60532222
  
      [Test build #22256 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22256/consoleFull) for   PR 2816 at commit [`c2bab5e`](https://github.com/apache/spark/commit/c2bab5ebbcdc3c7ed1d66e9b719109f66b326e58).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59470610
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21846/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59732530
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21913/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60536785
  
      [Test build #22259 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22259/consoleFull) for   PR 2816 at commit [`32a5005`](https://github.com/apache/spark/commit/32a5005cc9100ad16580a09c218b96b3cdd04636).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2816#discussion_r18953527
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/SqlParser.scala ---
    @@ -301,17 +301,67 @@ class SqlParser extends AbstractSparkSQLParser {
         CAST ~ "(" ~> expression ~ (AS ~> dataType) <~ ")" ^^ { case exp ~ t => Cast(exp, t) }
     
       protected lazy val literal: Parser[Literal] =
    -    ( numericLit ^^ {
    -        case i if i.toLong > Int.MaxValue => Literal(i.toLong)
    -        case i => Literal(i.toInt)
    -      }
    +    ( numericLiteral
         | NULL ^^^ Literal(null, NullType)
    -    | floatLit ^^ {case f => Literal(f.toDouble) }
    +    | booleanLiteral
         | stringLit ^^ {case s => Literal(s, StringType) }
         )
     
    +  protected lazy val booleanLiteral: Parser[Literal] =
    +    ( TRUE ^^^ Literal(true, BooleanType)
    +    | FALSE ^^^ Literal(false, BooleanType)
    +    )
    +
    +  protected lazy val numericLiteral: Parser[Literal] =
    +    signedNumericLiteral | unsignedNumericLiteral | floatLit ^^ { f => Literal(f.toDouble) }
    +
    +  protected lazy val sign: Parser[String] =
    +    "+" | "-"
    +
    +  protected lazy val signedNumericLiteral: Parser[Literal] =
    +    sign ~ numericLit  ^^ { case s ~ l => Literal(getProperTypeInt(s + l)) }
    +
    +
    +  protected lazy val unsignedNumericLiteral: Parser[Literal] =
    +    numericLit ^^ { n => Literal(getProperTypeInt(n)) }
    +
    +
    +  private val longMax = BigDecimal(s"${Long.MaxValue}")
    +  private val longMin = BigDecimal(s"${Long.MinValue}")
    +  private val intMax = BigDecimal(s"${Int.MaxValue}")
    +  private val intMin = BigDecimal(s"${Int.MinValue}")
    +
    +  private def getProperTypeInt(value: String) = {
    --- End diff --
    
    How about `toNarrowestNumericType`?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59461663
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21836/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/2816


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60535201
  
      [Test build #463 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/463/consoleFull) for   PR 2816 at commit [`32a5005`](https://github.com/apache/spark/commit/32a5005cc9100ad16580a09c218b96b3cdd04636).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60537666
  
    Awesome, thanks for doing this and for all the test cases!  Merged to master.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60519189
  
      [Test build #22246 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22246/consoleFull) for   PR 2816 at commit [`8c07bc7`](https://github.com/apache/spark/commit/8c07bc763827f873bc3192d50bda88c786754c9f).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60475848
  
      [Test build #22213 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22213/consoleFull) for   PR 2816 at commit [`13b78fe`](https://github.com/apache/spark/commit/13b78fe18e5646c9680f1d655c1ecc10247961b8).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59650498
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21890/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59732528
  
      [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21913/consoleFull) for   PR 2816 at commit [`a580dd4`](https://github.com/apache/spark/commit/a580dd436d007265ed6cdba9666f9d27e3025f57).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59857548
  
      [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21938/consoleFull) for   PR 2816 at commit [`025374c`](https://github.com/apache/spark/commit/025374cc90790278e7d10cb06cc7976fd177a941).
     * This patch **fails to build**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59650907
  
      [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21892/consoleFull) for   PR 2816 at commit [`5c847ac`](https://github.com/apache/spark/commit/5c847aca4e7d618dee7b8c647bdca6f845d328e3).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59464602
  
      [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21840/consoleFull) for   PR 2816 at commit [`56817b1`](https://github.com/apache/spark/commit/56817b196136447f26c26c2b4088c408acd66cdc).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2816#discussion_r18993402
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/SqlParser.scala ---
    @@ -301,33 +301,78 @@ class SqlParser extends AbstractSparkSQLParser {
         CAST ~ "(" ~> expression ~ (AS ~> dataType) <~ ")" ^^ { case exp ~ t => Cast(exp, t) }
     
       protected lazy val literal: Parser[Literal] =
    -    ( numericLit ^^ {
    -        case i if i.toLong > Int.MaxValue => Literal(i.toLong)
    -        case i => Literal(i.toInt)
    -      }
    -    | NULL ^^^ Literal(null, NullType)
    -    | floatLit ^^ {case f => Literal(f.toDouble) }
    +    (  signedLiteral
    +    |unsignedNumericLiteral
    +    | booleanLiteral
         | stringLit ^^ {case s => Literal(s, StringType) }
    +    | NULL ^^^ Literal(null, NullType)
    +    )
    +
    +  protected lazy val signedLiteral: Parser[Literal] =
    --- End diff --
    
    Seems that this rule is redundant?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59467818
  
      [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21846/consoleFull) for   PR 2816 at commit [`96cfcbb`](https://github.com/apache/spark/commit/96cfcbb8c7b3c5de4eae599f92c5c0d659b22ab7).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60476545
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22213/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59346655
  
      [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21803/consoleFull) for   PR 2816 at commit [`735c1fe`](https://github.com/apache/spark/commit/735c1fed46c14158698d4c2434d7c5f2e3e24213).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59466816
  
      [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21845/consoleFull) for   PR 2816 at commit [`96cfcbb`](https://github.com/apache/spark/commit/96cfcbb8c7b3c5de4eae599f92c5c0d659b22ab7).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60475563
  
      [Test build #22204 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22204/consoleFull) for   PR 2816 at commit [`13b78fe`](https://github.com/apache/spark/commit/13b78fe18e5646c9680f1d655c1ecc10247961b8).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `class SparkContext(config: SparkConf) extends SparkStatusAPI with Logging `
      * `  class JobUIData(`
      * `public final class JavaStatusAPIDemo `
      * `  public static final class IdentityWithDelay<T> implements Function<T, T> `



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2816#discussion_r18993345
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/SqlParser.scala ---
    @@ -301,33 +301,78 @@ class SqlParser extends AbstractSparkSQLParser {
         CAST ~ "(" ~> expression ~ (AS ~> dataType) <~ ")" ^^ { case exp ~ t => Cast(exp, t) }
     
       protected lazy val literal: Parser[Literal] =
    -    ( numericLit ^^ {
    -        case i if i.toLong > Int.MaxValue => Literal(i.toLong)
    -        case i => Literal(i.toInt)
    -      }
    -    | NULL ^^^ Literal(null, NullType)
    -    | floatLit ^^ {case f => Literal(f.toDouble) }
    +    (  signedLiteral
    +    |unsignedNumericLiteral
    --- End diff --
    
    Indentation is off in these two lines.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by sarutak <gi...@git.apache.org>.
Github user sarutak commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59460731
  
    @liancheng Thanks for your advice!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59861153
  
      [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21942/consoleFull) for   PR 2816 at commit [`025374c`](https://github.com/apache/spark/commit/025374cc90790278e7d10cb06cc7976fd177a941).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2816#discussion_r18953909
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/SqlParser.scala ---
    @@ -301,17 +301,67 @@ class SqlParser extends AbstractSparkSQLParser {
         CAST ~ "(" ~> expression ~ (AS ~> dataType) <~ ")" ^^ { case exp ~ t => Cast(exp, t) }
     
       protected lazy val literal: Parser[Literal] =
    -    ( numericLit ^^ {
    -        case i if i.toLong > Int.MaxValue => Literal(i.toLong)
    -        case i => Literal(i.toInt)
    -      }
    +    ( numericLiteral
         | NULL ^^^ Literal(null, NullType)
    -    | floatLit ^^ {case f => Literal(f.toDouble) }
    +    | booleanLiteral
         | stringLit ^^ {case s => Literal(s, StringType) }
         )
     
    +  protected lazy val booleanLiteral: Parser[Literal] =
    +    ( TRUE ^^^ Literal(true, BooleanType)
    +    | FALSE ^^^ Literal(false, BooleanType)
    +    )
    +
    +  protected lazy val numericLiteral: Parser[Literal] =
    +    signedNumericLiteral | unsignedNumericLiteral | floatLit ^^ { f => Literal(f.toDouble) }
    +
    +  protected lazy val sign: Parser[String] =
    +    "+" | "-"
    +
    +  protected lazy val signedNumericLiteral: Parser[Literal] =
    +    sign ~ numericLit  ^^ { case s ~ l => Literal(getProperTypeInt(s + l)) }
    --- End diff --
    
    Ah, I see, you made signed and unsigned numerics into two branches.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by sarutak <gi...@git.apache.org>.
Github user sarutak commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60049213
  
    @yhuai I could fix what you pointed. Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59865477
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21942/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59860616
  
    ok to test


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by sarutak <gi...@git.apache.org>.
Github user sarutak commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60516825
  
    retest this please.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59392306
  
      [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21809/consoleFull) for   PR 2816 at commit [`938ba13`](https://github.com/apache/spark/commit/938ba13757b0979fd93185f901f1ebc26efb9caf).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59464604
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21840/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59345860
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21802/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by sarutak <gi...@git.apache.org>.
Github user sarutak commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59461721
  
    retest this please.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2816#discussion_r18952642
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/SqlParser.scala ---
    @@ -301,17 +301,67 @@ class SqlParser extends AbstractSparkSQLParser {
         CAST ~ "(" ~> expression ~ (AS ~> dataType) <~ ")" ^^ { case exp ~ t => Cast(exp, t) }
     
       protected lazy val literal: Parser[Literal] =
    -    ( numericLit ^^ {
    -        case i if i.toLong > Int.MaxValue => Literal(i.toLong)
    -        case i => Literal(i.toInt)
    -      }
    +    ( numericLiteral
         | NULL ^^^ Literal(null, NullType)
    -    | floatLit ^^ {case f => Literal(f.toDouble) }
    +    | booleanLiteral
    --- End diff --
    
    Same as above.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59469449
  
      [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21845/consoleFull) for   PR 2816 at commit [`96cfcbb`](https://github.com/apache/spark/commit/96cfcbb8c7b3c5de4eae599f92c5c0d659b22ab7).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59649582
  
      [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21890/consoleFull) for   PR 2816 at commit [`5c847ac`](https://github.com/apache/spark/commit/5c847aca4e7d618dee7b8c647bdca6f845d328e3).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by sarutak <gi...@git.apache.org>.
Github user sarutak commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60035199
  
    @yhuai Thanks for pointing. Actually we cannot still move it out from ignore because 92233720368547758061.2 is BigDecimal type but num_str + 1.2 is Double type so comparing fails.
    In Spark SQL, floating-point-number literal is represented as Double type and arithmetic operation one of which operand is String results Double type.
    I think we need another discussion for the type of floating-point-number literal and implicit cast.
    I'll open another PR.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59467174
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21844/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60476543
  
      [Test build #22213 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22213/consoleFull) for   PR 2816 at commit [`13b78fe`](https://github.com/apache/spark/commit/13b78fe18e5646c9680f1d655c1ecc10247961b8).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2816#discussion_r19001491
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/SqlParser.scala ---
    @@ -301,33 +301,75 @@ class SqlParser extends AbstractSparkSQLParser {
         CAST ~ "(" ~> expression ~ (AS ~> dataType) <~ ")" ^^ { case exp ~ t => Cast(exp, t) }
     
       protected lazy val literal: Parser[Literal] =
    -    ( numericLit ^^ {
    -        case i if i.toLong > Int.MaxValue => Literal(i.toLong)
    -        case i => Literal(i.toInt)
    -      }
    -    | NULL ^^^ Literal(null, NullType)
    -    | floatLit ^^ {case f => Literal(f.toDouble) }
    +    ( signedNumericLiteral
    +    | unsignedNumericLiteral
    +    | booleanLiteral
         | stringLit ^^ {case s => Literal(s, StringType) }
    +    | NULL ^^^ Literal(null, NullType)
    +    )
    +
    +  protected lazy val booleanLiteral: Parser[Literal] =
    +    ( TRUE ^^^ Literal(true, BooleanType)
    +    | FALSE ^^^ Literal(false, BooleanType)
    +    )
    +
    +  protected lazy val numericLiteral: Parser[Literal] =
    +    signedNumericLiteral | unsignedNumericLiteral
    +
    +  protected lazy val sign: Parser[String] =
    +    "+" | "-"
    +
    +  protected lazy val signedNumericLiteral: Parser[Literal] =
    +    ( sign ~ numericLit  ^^ { case s ~ l => Literal(toNarrowestIntegerType(s + l)) }
    +    | sign ~ floatLit ^^ { case s ~ f => Literal((s + f).toDouble) }
    +    )
    +
    +  protected lazy val unsignedNumericLiteral: Parser[Literal] =
    +    ( numericLit ^^ { n => Literal(toNarrowestIntegerType(n)) }
    +    | floatLit ^^ { f => Literal(f.toDouble) }
         )
     
    +  private val longMax = BigDecimal(s"${Long.MaxValue}")
    +  private val longMin = BigDecimal(s"${Long.MinValue}")
    +  private val intMax = BigDecimal(s"${Int.MaxValue}")
    +  private val intMin = BigDecimal(s"${Int.MinValue}")
    +
    +  private def toNarrowestIntegerType(value: String) = {
    +    val bigIntValue = BigDecimal(value)
    +
    +    bigIntValue match {
    +      case v if v < longMin || v > longMax => v
    +      case v if v < intMin || v > intMax => v.toLong
    +      case v => v.toInt
    +    }
    +  }
    +
       protected lazy val floatLit: Parser[String] =
    -    elem("decimal", _.isInstanceOf[lexical.FloatLit]) ^^ (_.chars)
    +    ( "." ~> unsignedNumericLiteral ^^ { u => "0." + u }
    +    | elem("decimal", _.isInstanceOf[lexical.FloatLit]) ^^ (_.chars)
    +    )
     
       protected lazy val baseExpression: PackratParser[Expression] =
    -    ( expression ~ ("[" ~> expression <~ "]") ^^
    +    ( "*" ^^^ Star(None)
    +    | primary
    +    | signedPrimary
    --- End diff --
    
    I think we don't need this line here, as `primary` include `signedPrimary`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60474895
  
      [Test build #22204 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22204/consoleFull) for   PR 2816 at commit [`13b78fe`](https://github.com/apache/spark/commit/13b78fe18e5646c9680f1d655c1ecc10247961b8).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60536997
  
      [Test build #463 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/463/consoleFull) for   PR 2816 at commit [`32a5005`](https://github.com/apache/spark/commit/32a5005cc9100ad16580a09c218b96b3cdd04636).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by sarutak <gi...@git.apache.org>.
Github user sarutak commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59860458
  
    retest this please.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by sarutak <gi...@git.apache.org>.
Github user sarutak commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59304994
  
    FYI, following URL shows BNF of SQL99.
    savage.net.au/SQL/sql-99.bnf.html
    
    By the way, I think, BNF of Spark SQL and semantic analysis is little bit different from Standard SQL.
    The point I mentioned in this PR is one of those difference.
    I'd like to refine BNF of Spark SQL in another PR.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59345856
  
      [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21802/consoleFull) for   PR 2816 at commit [`037a948`](https://github.com/apache/spark/commit/037a9488ef36bd8971f90556663b31456e628314).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59351102
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21803/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60533346
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22256/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60027697
  
    @sarutak Can you also move [this related test](https://github.com/apache/spark/blob/master/sql/core/src/test/scala/org/apache/spark/sql/json/JsonSuite.scala#L421) out of `ignore`? Thank you:)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2816#discussion_r18952768
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/SqlParser.scala ---
    @@ -301,17 +301,67 @@ class SqlParser extends AbstractSparkSQLParser {
         CAST ~ "(" ~> expression ~ (AS ~> dataType) <~ ")" ^^ { case exp ~ t => Cast(exp, t) }
     
       protected lazy val literal: Parser[Literal] =
    -    ( numericLit ^^ {
    -        case i if i.toLong > Int.MaxValue => Literal(i.toLong)
    -        case i => Literal(i.toInt)
    -      }
    +    ( numericLiteral
    --- End diff --
    
    Sorry, just found out that `numericLit` is a member of `StdTokenParsers`. Then this naming is alright.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by sarutak <gi...@git.apache.org>.
Github user sarutak commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60381288
  
    /CC @marmbrus 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60055364
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22029/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60536788
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22259/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-60533343
  
      [Test build #22256 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22256/consoleFull) for   PR 2816 at commit [`c2bab5e`](https://github.com/apache/spark/commit/c2bab5ebbcdc3c7ed1d66e9b719109f66b326e58).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59342873
  
      [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21802/consoleFull) for   PR 2816 at commit [`037a948`](https://github.com/apache/spark/commit/037a9488ef36bd8971f90556663b31456e628314).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2816#discussion_r19001551
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/SqlParser.scala ---
    @@ -301,33 +301,75 @@ class SqlParser extends AbstractSparkSQLParser {
         CAST ~ "(" ~> expression ~ (AS ~> dataType) <~ ")" ^^ { case exp ~ t => Cast(exp, t) }
     
       protected lazy val literal: Parser[Literal] =
    -    ( numericLit ^^ {
    -        case i if i.toLong > Int.MaxValue => Literal(i.toLong)
    -        case i => Literal(i.toInt)
    -      }
    -    | NULL ^^^ Literal(null, NullType)
    -    | floatLit ^^ {case f => Literal(f.toDouble) }
    +    ( signedNumericLiteral
    +    | unsignedNumericLiteral
    +    | booleanLiteral
         | stringLit ^^ {case s => Literal(s, StringType) }
    +    | NULL ^^^ Literal(null, NullType)
    +    )
    +
    +  protected lazy val booleanLiteral: Parser[Literal] =
    +    ( TRUE ^^^ Literal(true, BooleanType)
    +    | FALSE ^^^ Literal(false, BooleanType)
    +    )
    +
    +  protected lazy val numericLiteral: Parser[Literal] =
    +    signedNumericLiteral | unsignedNumericLiteral
    +
    +  protected lazy val sign: Parser[String] =
    +    "+" | "-"
    +
    +  protected lazy val signedNumericLiteral: Parser[Literal] =
    +    ( sign ~ numericLit  ^^ { case s ~ l => Literal(toNarrowestIntegerType(s + l)) }
    +    | sign ~ floatLit ^^ { case s ~ f => Literal((s + f).toDouble) }
    +    )
    +
    +  protected lazy val unsignedNumericLiteral: Parser[Literal] =
    +    ( numericLit ^^ { n => Literal(toNarrowestIntegerType(n)) }
    +    | floatLit ^^ { f => Literal(f.toDouble) }
         )
     
    +  private val longMax = BigDecimal(s"${Long.MaxValue}")
    +  private val longMin = BigDecimal(s"${Long.MinValue}")
    +  private val intMax = BigDecimal(s"${Int.MaxValue}")
    +  private val intMin = BigDecimal(s"${Int.MinValue}")
    +
    +  private def toNarrowestIntegerType(value: String) = {
    +    val bigIntValue = BigDecimal(value)
    +
    +    bigIntValue match {
    +      case v if v < longMin || v > longMax => v
    +      case v if v < intMin || v > intMax => v.toLong
    +      case v => v.toInt
    +    }
    +  }
    +
       protected lazy val floatLit: Parser[String] =
    -    elem("decimal", _.isInstanceOf[lexical.FloatLit]) ^^ (_.chars)
    +    ( "." ~> unsignedNumericLiteral ^^ { u => "0." + u }
    +    | elem("decimal", _.isInstanceOf[lexical.FloatLit]) ^^ (_.chars)
    +    )
     
       protected lazy val baseExpression: PackratParser[Expression] =
    --- End diff --
    
    Now the `baseExpression` don't have recursion explicitly, we can define it as normal `Parser`, and define `primary` as `PackratParser`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-3959][SPARK-3960][SQL] SqlParser fails ...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:

    https://github.com/apache/spark/pull/2816#issuecomment-59458384
  
    LGTM except for minor styling issues. Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org