[GitHub] carbondata pull request #2078: [WIP][CARBONDATA-2262] Support the syntax of ...

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2078#discussion_r176049685
 
    --- Diff: integration/spark2/src/main/spark2.1/org/apache/spark/sql/hive/CarbonSessionState.scala ---
    @@ -326,7 +326,8 @@ class CarbonSqlAstBuilder(conf: SQLConf, parser: CarbonSpark2SqlParser, sparkSes
         val fileStorage = helper.getFileStorage(ctx.createFileFormat)
     
         if (fileStorage.equalsIgnoreCase("'carbondata'") ||
    -        fileStorage.equalsIgnoreCase("'org.apache.carbondata.format'")) {
    +      fileStorage.equalsIgnoreCase("carbondata") ||
    --- End diff --
   
    line 329 is repeated


---

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2078#discussion_r176049748
 
    --- Diff: integration/spark2/src/main/spark2.2/org/apache/spark/sql/hive/CarbonSessionState.scala ---
    @@ -325,7 +325,8 @@ class CarbonSqlAstBuilder(conf: SQLConf, parser: CarbonSpark2SqlParser, sparkSes
         val fileStorage = helper.getFileStorage(ctx.createFileFormat)
     
         if (fileStorage.equalsIgnoreCase("'carbondata'") ||
    -        fileStorage.equalsIgnoreCase("'org.apache.carbondata.format'")) {
    +      fileStorage.equalsIgnoreCase("carbondata") ||
    --- End diff --
   
    repeated logic


---

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2078#discussion_r176049956
 
    --- Diff: integration/spark2/src/test/scala/org/apache/spark/SparkCommandSuite.scala ---
    @@ -35,4 +35,22 @@ class SparkCommandSuite extends Spark2QueryTest with BeforeAndAfterAll {
         sql("CREATE TABLE src_pqt(key INT, value STRING) STORED AS PARQUET")
         sql("CREATE TABLE src_orc(key INT, value STRING) STORED AS ORC")
       }
    +
    +  test("CARBONDATA-734: Don't Support the syntax of 'STORED BY 'PARQUET''") {
    +    try {
    +      sql("CREATE TABLE src_pqt(key INT, value STRING) STORED AS 'PARQUET'")
    --- End diff --
   
    This is not correct, stored as parquet or orc should be supported


---

[qiuchenjian-2]

Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2078#discussion_r176286029
 
    --- Diff: integration/spark2/src/main/spark2.2/org/apache/spark/sql/hive/CarbonSessionState.scala ---
    @@ -325,7 +325,8 @@ class CarbonSqlAstBuilder(conf: SQLConf, parser: CarbonSpark2SqlParser, sparkSes
         val fileStorage = helper.getFileStorage(ctx.createFileFormat)
     
         if (fileStorage.equalsIgnoreCase("'carbondata'") ||
    -        fileStorage.equalsIgnoreCase("'org.apache.carbondata.format'")) {
    +      fileStorage.equalsIgnoreCase("carbondata") ||
    --- End diff --
   
    It's different: 'carbondata'  and  carbondata.


---

[qiuchenjian-2]

Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2078#discussion_r176286537
 
    --- Diff: integration/spark2/src/main/spark2.1/org/apache/spark/sql/hive/CarbonSessionState.scala ---
    @@ -326,7 +326,8 @@ class CarbonSqlAstBuilder(conf: SQLConf, parser: CarbonSpark2SqlParser, sparkSes
         val fileStorage = helper.getFileStorage(ctx.createFileFormat)
     
         if (fileStorage.equalsIgnoreCase("'carbondata'") ||
    -        fileStorage.equalsIgnoreCase("'org.apache.carbondata.format'")) {
    +      fileStorage.equalsIgnoreCase("carbondata") ||
    --- End diff --
   
    It's different: 'carbondata' and carbondata.  
   
    carbon supported syntax "stored by 'carbondata'" before, but didn't  supported  syntax "stored by carbondata".
    we need support syntax "STORED AS carbondata" like "STORED AS parquet"


---

[qiuchenjian-2]

Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2078#discussion_r176286684
 
    --- Diff: integration/spark2/src/test/scala/org/apache/spark/SparkCommandSuite.scala ---
    @@ -35,4 +35,22 @@ class SparkCommandSuite extends Spark2QueryTest with BeforeAndAfterAll {
         sql("CREATE TABLE src_pqt(key INT, value STRING) STORED AS PARQUET")
         sql("CREATE TABLE src_orc(key INT, value STRING) STORED AS ORC")
       }
    +
    +  test("CARBONDATA-734: Don't Support the syntax of 'STORED BY 'PARQUET''") {
    +    try {
    +      sql("CREATE TABLE src_pqt(key INT, value STRING) STORED AS 'PARQUET'")
    --- End diff --
   
    Now carbon support syntax "stored as parquet" but don't support syntax "stored as 'parquet'".
   
    Whether do we support syntax "stored as 'parquet'"?


---

[qiuchenjian-2]

Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2078#discussion_r176288720
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParser.scala ---
    @@ -91,6 +91,9 @@ class CarbonHelperSqlAstBuilder(conf: SQLConf,
           case Some(value) =>
             if (value.children.get(1).getText.equalsIgnoreCase("by")) {
               value.storageHandler().STRING().getSymbol.getText
    +        } else if (value.children.get(1).getText.equalsIgnoreCase("as")
    --- End diff --
   
    ok, done


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2078
 
    Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3263/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2078
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/4492/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2078
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4004/



---

[qiuchenjian-2]

Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2078
 
    retest this please


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2078
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/4504/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2078
 
    Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3275/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2078
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/4510/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2078
 
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3282/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2078
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4016/



---

[qiuchenjian-2]

Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2078
 
    retest this please


---

[qiuchenjian-2]

Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2078
 
    retest sdv please


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2078
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/4513/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2078
 
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3285/



---