[GitHub] [carbondata] ShreelekhyaG opened a new pull request #3849: [WIP] table level timestampformat

[GitBox]

ShreelekhyaG opened a new pull request #3849:
URL: https://github.com/apache/carbondata/pull/3849


    ### Why is this PR needed?
    To support timestamp format table level.
   
    ### What changes were proposed in this PR?
   Made the priority of timestamp format as:
    1) Load command options
    2) Table level properties
    3) configurable properties (carbon.timestamp.format)
       
    ### Does this PR introduce any user interface change?
    - No
   
    ### Is any new testcase added?
    - Yes
   
       
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3849:
URL: https://github.com/apache/carbondata/pull/3849#issuecomment-659392712


   Build Failed  with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3407/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3849:
URL: https://github.com/apache/carbondata/pull/3849#issuecomment-659447560


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/1665/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

ShreelekhyaG commented on pull request #3849:
URL: https://github.com/apache/carbondata/pull/3849#issuecomment-659467768


   retest this please


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3849:
URL: https://github.com/apache/carbondata/pull/3849#issuecomment-659550711


   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3408/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3849:
URL: https://github.com/apache/carbondata/pull/3849#issuecomment-659554650


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/1666/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

Zhangshunyu commented on pull request #3849:
URL: https://github.com/apache/carbondata/pull/3849#issuecomment-659774957


   Greate! This is a useful feature.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

QiangCai commented on pull request #3849:
URL: https://github.com/apache/carbondata/pull/3849#issuecomment-661062638


   two suggestions:
   1. support DATEFORMAT AND TIMESTAMPFORMAT
   2. use the same property name with load options.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

QiangCai commented on a change in pull request #3849:
URL: https://github.com/apache/carbondata/pull/3849#discussion_r457429601



##########
File path: integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/dataload/TestLoadDataWithDiffTimestampFormat.scala
##########
@@ -122,6 +122,35 @@ class TestLoadDataWithDiffTimestampFormat extends QueryTest with BeforeAndAfterA
 
   }
 
+  test("test load data with timestamp format set at different levels") {
+    sql("DROP TABLE IF EXISTS t3")
+    sql(
+      """
+           CREATE TABLE IF NOT EXISTS t3
+           (ID Int, date date, starttime Timestamp, country String,
+           name String, phonetype String, serialname String, salary Int)
+           STORED AS carbondata TBLPROPERTIES('table_timestampformat'='yyyy-MM-dd')

Review comment:
       please add test case for "describe formatted table" and verify the table property list that should contain new properties.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

ShreelekhyaG commented on a change in pull request #3849:
URL: https://github.com/apache/carbondata/pull/3849#discussion_r458050377



##########
File path: integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/dataload/TestLoadDataWithDiffTimestampFormat.scala
##########
@@ -122,6 +122,35 @@ class TestLoadDataWithDiffTimestampFormat extends QueryTest with BeforeAndAfterA
 
   }
 
+  test("test load data with timestamp format set at different levels") {
+    sql("DROP TABLE IF EXISTS t3")
+    sql(
+      """
+           CREATE TABLE IF NOT EXISTS t3
+           (ID Int, date date, starttime Timestamp, country String,
+           name String, phonetype String, serialname String, salary Int)
+           STORED AS carbondata TBLPROPERTIES('table_timestampformat'='yyyy-MM-dd')

Review comment:
       Added new test case.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3849:
URL: https://github.com/apache/carbondata/pull/3849#issuecomment-661896331


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/1714/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3849:
URL: https://github.com/apache/carbondata/pull/3849#issuecomment-661896594


   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3456/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

QiangCai commented on pull request #3849:
URL: https://github.com/apache/carbondata/pull/3849#issuecomment-662199441


   please check all usage of CarbonCommonConstants.CARBON_TIMESTAMP_FORMAT/CarbonCommonConstants.CARBON_DATE_FORMAT and consider whether it need get table properties or not.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

ShreelekhyaG commented on pull request #3849:
URL: https://github.com/apache/carbondata/pull/3849#issuecomment-662279167


   checked and updated.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3849:
URL: https://github.com/apache/carbondata/pull/3849#issuecomment-662336254


   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3461/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3849:
URL: https://github.com/apache/carbondata/pull/3849#issuecomment-662337358


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/1719/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

QiangCai commented on a change in pull request #3849:
URL: https://github.com/apache/carbondata/pull/3849#discussion_r459259001



##########
File path: integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/dataload/TestLoadDataWithDiffTimestampFormat.scala
##########
@@ -122,6 +122,65 @@ class TestLoadDataWithDiffTimestampFormat extends QueryTest with BeforeAndAfterA
 
   }
 
+  test("test load data with date and timestamp format set at different levels") {

Review comment:
       please add test case for insertinto command to use table properties

##########
File path: integration/spark/src/main/scala/org/apache/carbondata/spark/CarbonOption.scala
##########
@@ -77,5 +80,15 @@ class CarbonOption(options: Map[String, String]) {
   lazy val overwriteEnabled: Boolean =
     options.getOrElse("overwrite", "false").toBoolean
 
+  lazy val timestampformat: String = options.getOrElse("timestampformat",
+    CarbonProperties.getInstance
+      .getProperty(CarbonCommonConstants.CARBON_TIMESTAMP_FORMAT,
+        CarbonCommonConstants.CARBON_TIMESTAMP_DEFAULT_FORMAT))
+
+  lazy val dateformat: String = options.getOrElse("dateformat",
+    CarbonProperties.getInstance
+      .getProperty(CarbonCommonConstants.CARBON_DATE_FORMAT,
+        CarbonCommonConstants.CARBON_DATE_DEFAULT_FORMAT))
+

Review comment:
       directly get it from options




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

QiangCai commented on a change in pull request #3849:
URL: https://github.com/apache/carbondata/pull/3849#discussion_r459337221



##########
File path: integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/dataload/TestLoadDataWithDiffTimestampFormat.scala
##########
@@ -122,6 +122,65 @@ class TestLoadDataWithDiffTimestampFormat extends QueryTest with BeforeAndAfterA
 
   }
 
+  test("test load data with date and timestamp format set at different levels") {
+    sql("DROP TABLE IF EXISTS t3")
+    sql(
+      """
+           CREATE TABLE IF NOT EXISTS t3
+           (ID Int, date date, starttime Timestamp, country String,
+           name String, phonetype String, serialname String, salary Int)
+           STORED AS carbondata TBLPROPERTIES('dateformat'='yyyy/MM/dd',
+           'timestampformat'='yyyy-MM-dd HH:mm')
+        """)
+    sql(
+      s"""
+           LOAD DATA LOCAL INPATH '$resourcesPath/timeStampFormatData1.csv' into table t3
+           """)
+    sql(
+      s"""
+           LOAD DATA LOCAL INPATH '$resourcesPath/timeStampFormatData2.csv' into table t3
+           OPTIONS('dateformat' = 'yyyy-MM-dd','timestampformat'='yyyy/MM/dd HH:mm:ss')
+           """)
+    val sdf = new SimpleDateFormat("yyyy-MM-dd")
+    checkAnswer(
+      sql("SELECT starttime FROM t3 WHERE ID = 1"),
+      Seq(Row(Timestamp.valueOf("2016-07-23 01:01:00")))
+    )
+    checkAnswer(
+      sql("SELECT starttime FROM t3 WHERE ID = 18"),
+      Seq(Row(Timestamp.valueOf("2016-07-25 02:32:02")))
+    )
+    checkAnswer(
+      sql("SELECT date FROM t3 WHERE ID = 1"),
+      Seq(Row(new Date(sdf.parse("2015-07-23").getTime)))
+    )
+    checkAnswer(
+      sql("SELECT date FROM t3 WHERE ID = 18"),
+      Seq(Row(new Date(sdf.parse("2015-07-25").getTime)))
+    )
+  }
+
+  test("test create table with date and timestamp format and check describe formatted") {
+    sql("DROP TABLE IF EXISTS t3")
+    sql(
+      """
+           CREATE TABLE IF NOT EXISTS t3
+           (ID Int, date date, starttime Timestamp, country String,
+           name String, phonetype String, serialname String, salary Int)
+           STORED AS carbondata TBLPROPERTIES('dateformat'='yyyy/MM/dd',
+           'timestampformat'='yyyy-MM-dd HH:mm')
+        """)
+    val descTable = sql(s"describe formatted t3").collect
+    descTable.find(_.get(0).toString.contains("Date Format")) match {
+      case Some(row) => assert(row.get(1).toString.contains("yyyy/MM/dd"))
+      case None => assert(false)
+    }
+    descTable.find(_.get(0).toString.contains("Timestamp Format")) match {
+      case Some(row) => assert(row.get(1).toString.contains("yyyy-MM-dd HH:mm"))
+      case None => assert(false)
+    }
+  }
+

Review comment:
       please add more test case for alter table SET and UNSET tblproperties




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

ShreelekhyaG commented on a change in pull request #3849:
URL: https://github.com/apache/carbondata/pull/3849#discussion_r459371870



##########
File path: integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/dataload/TestLoadDataWithDiffTimestampFormat.scala
##########
@@ -122,6 +122,65 @@ class TestLoadDataWithDiffTimestampFormat extends QueryTest with BeforeAndAfterA
 
   }
 
+  test("test load data with date and timestamp format set at different levels") {

Review comment:
       ok done.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

ShreelekhyaG commented on a change in pull request #3849:
URL: https://github.com/apache/carbondata/pull/3849#discussion_r459371981



##########
File path: integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/dataload/TestLoadDataWithDiffTimestampFormat.scala
##########
@@ -122,6 +122,65 @@ class TestLoadDataWithDiffTimestampFormat extends QueryTest with BeforeAndAfterA
 
   }
 
+  test("test load data with date and timestamp format set at different levels") {
+    sql("DROP TABLE IF EXISTS t3")
+    sql(
+      """
+           CREATE TABLE IF NOT EXISTS t3
+           (ID Int, date date, starttime Timestamp, country String,
+           name String, phonetype String, serialname String, salary Int)
+           STORED AS carbondata TBLPROPERTIES('dateformat'='yyyy/MM/dd',
+           'timestampformat'='yyyy-MM-dd HH:mm')
+        """)
+    sql(
+      s"""
+           LOAD DATA LOCAL INPATH '$resourcesPath/timeStampFormatData1.csv' into table t3
+           """)
+    sql(
+      s"""
+           LOAD DATA LOCAL INPATH '$resourcesPath/timeStampFormatData2.csv' into table t3
+           OPTIONS('dateformat' = 'yyyy-MM-dd','timestampformat'='yyyy/MM/dd HH:mm:ss')
+           """)
+    val sdf = new SimpleDateFormat("yyyy-MM-dd")
+    checkAnswer(
+      sql("SELECT starttime FROM t3 WHERE ID = 1"),
+      Seq(Row(Timestamp.valueOf("2016-07-23 01:01:00")))
+    )
+    checkAnswer(
+      sql("SELECT starttime FROM t3 WHERE ID = 18"),
+      Seq(Row(Timestamp.valueOf("2016-07-25 02:32:02")))
+    )
+    checkAnswer(
+      sql("SELECT date FROM t3 WHERE ID = 1"),
+      Seq(Row(new Date(sdf.parse("2015-07-23").getTime)))
+    )
+    checkAnswer(
+      sql("SELECT date FROM t3 WHERE ID = 18"),
+      Seq(Row(new Date(sdf.parse("2015-07-25").getTime)))
+    )
+  }
+
+  test("test create table with date and timestamp format and check describe formatted") {
+    sql("DROP TABLE IF EXISTS t3")
+    sql(
+      """
+           CREATE TABLE IF NOT EXISTS t3
+           (ID Int, date date, starttime Timestamp, country String,
+           name String, phonetype String, serialname String, salary Int)
+           STORED AS carbondata TBLPROPERTIES('dateformat'='yyyy/MM/dd',
+           'timestampformat'='yyyy-MM-dd HH:mm')
+        """)
+    val descTable = sql(s"describe formatted t3").collect
+    descTable.find(_.get(0).toString.contains("Date Format")) match {
+      case Some(row) => assert(row.get(1).toString.contains("yyyy/MM/dd"))
+      case None => assert(false)
+    }
+    descTable.find(_.get(0).toString.contains("Timestamp Format")) match {
+      case Some(row) => assert(row.get(1).toString.contains("yyyy-MM-dd HH:mm"))
+      case None => assert(false)
+    }
+  }
+

Review comment:
       done.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]