[GitHub] [carbondata] akkio-97 opened a new pull request #4074: [WIP]

classic Classic list List threaded Threaded
38 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4074: [CARBONDATA-4109] Improve carbondata coverage for presto-integration code

GitBox

CarbonDataQA2 commented on pull request #4074:
URL: https://github.com/apache/carbondata/pull/4074#issuecomment-768271765


   Build Failed  with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3603/
   


----------------------------------------------------------------
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]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4074: [CARBONDATA-4109] Improve carbondata coverage for presto-integration code

GitBox
In reply to this post by GitBox

CarbonDataQA2 commented on pull request #4074:
URL: https://github.com/apache/carbondata/pull/4074#issuecomment-768425702


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


----------------------------------------------------------------
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]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4074: [CARBONDATA-4109] Improve carbondata coverage for presto-integration code

GitBox
In reply to this post by GitBox

CarbonDataQA2 commented on pull request #4074:
URL: https://github.com/apache/carbondata/pull/4074#issuecomment-768426924


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


----------------------------------------------------------------
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]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #4074: [CARBONDATA-4109] Improve carbondata coverage for presto-integration code

GitBox
In reply to this post by GitBox

ajantha-bhat commented on a change in pull request #4074:
URL: https://github.com/apache/carbondata/pull/4074#discussion_r566600381



##########
File path: integration/spark/src/test/scala/org/apache/carbondata/integration/spark/testsuite/dataload/SparkStoreCreatorForPresto.scala
##########
@@ -365,6 +370,84 @@ class SparkStoreCreatorForPresto extends QueryTest with BeforeAndAfterAll{
     sql(s"""LOAD DATA LOCAL INPATH '$resourcesPath/IUD/dest.csv' INTO TABLE streaming_table""")
   }
 
+  test("Test short vector datatype") {
+    sql("drop table if exists array_short")
+    sql(
+      "CREATE TABLE IF NOT EXISTS array_short (salary array<short>) STORED AS " +
+      "carbondata"
+    )
+    sql("insert into array_short select array(4352,35,3) ") // page datatype - short, adaptive
+    // integral codec
+  }
+
+  test("Test int vector datatype") {
+    sql("drop table if exists array_int")
+    sql(
+      "CREATE TABLE IF NOT EXISTS array_int (salary array<int>) STORED AS " +
+      "carbondata"
+    )
+    sql("insert into array_int select array(21474836,21474839,23,3) ") // page datatype - int,
+    // adaptive integral codec
+
+    sql("insert into array_int select array(21474836,21474839) ") // page datatype - byte, adaptive
+    // delta integral codec
+
+  }
+
+  test("Test long vector datatype") {
+    sql("drop table if exists array_long")
+    sql(
+      "CREATE TABLE IF NOT EXISTS array_long (salary array<long>) STORED AS " +
+      "carbondata"
+    )
+    // following are for adaptive integral codec
+    sql("insert into array_long select array(215,23,3) ") // page datatype - short
+
+    sql("insert into array_long select array(32800,23,3) ") // page datatype - short_int
+
+    sql("insert into array_long select array(32800,214748364,3) ") // page datatype - int
+
+  }
+
+  test("Test double vector datatype") {
+    sql("drop table if exists array_double")
+    sql(
+      "CREATE TABLE IF NOT EXISTS array_double (salary array<double>) STORED AS " +
+      "carbondata"
+    )
+    // following are for adaptive integral codec
+    sql("insert into array_double select array(2,3,4) ") // page datatype - byte
+
+    sql("insert into array_double select array(242,35,43) ") // page datatype - short
+
+    sql("insert into array_double select array(32799,32767) ") // page datatype - short_int
+
+    sql("insert into array_double select array(21546546,32546546,43211564) ") // page datatype - int
+
+    // following are for adaptive floating codec
+    sql("insert into array_double select array(327.99,3.2799) ") // page datatype - short_int
+
+    sql("insert into array_double select array(1,2345,108787.123) ") // page datatype - int
+
+  }
+
+  test("Test timestamp vector datatype") {
+    TimeZone.setDefault(TimeZone.getTimeZone("Asia/Kolkata"))
+    sql("drop table if exists array_timestamp")
+    sql(
+      "CREATE TABLE IF NOT EXISTS array_timestamp (time array<timestamp>) STORED AS " +
+      "carbondata"
+    )
+    sql("insert into array_timestamp select array('2020-01-11 12:00:45.0','2020-01-11 12:01:45.0')")
+    // page datatype - short-int, adaptive delta integral codec
+
+    sql("insert into array_timestamp select array('2020-01-10 12:30:45.0','2015-01-11 12:01:45.0')")
+    // page datatype - long, adaptive integral
+
+    // set timezone back to default
+    TimeZone.setDefault(TimeZone.getTimeZone("Asia/Shanghai"))

Review comment:
       Can you get a getdefault() value in a variable before setting in line 435 and set it here.




----------------------------------------------------------------
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]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #4074: [CARBONDATA-4109] Improve carbondata coverage for presto-integration code

GitBox
In reply to this post by GitBox

ajantha-bhat commented on a change in pull request #4074:
URL: https://github.com/apache/carbondata/pull/4074#discussion_r566600450



##########
File path: integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/dataload/TestLoadDataWithDiffTimestampFormat.scala
##########
@@ -354,6 +355,7 @@ class TestLoadDataWithDiffTimestampFormat extends QueryTest with BeforeAndAfterA
 
   test("test load, update data with setlenient session level property for daylight " +
        "saving time from different timezone") {
+    TimeZone.setDefault(TimeZone.getTimeZone("America/Los_Angeles"))

Review comment:
       If the above comment works, you can remove this




----------------------------------------------------------------
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]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4074: [CARBONDATA-4109] Improve carbondata coverage for presto-integration code

GitBox
In reply to this post by GitBox

CarbonDataQA2 commented on pull request #4074:
URL: https://github.com/apache/carbondata/pull/4074#issuecomment-769673025


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


----------------------------------------------------------------
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]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4074: [CARBONDATA-4109] Improve carbondata coverage for presto-integration code

GitBox
In reply to this post by GitBox

CarbonDataQA2 commented on pull request #4074:
URL: https://github.com/apache/carbondata/pull/4074#issuecomment-769676243


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


----------------------------------------------------------------
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]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] ajantha-bhat commented on pull request #4074: [CARBONDATA-4109] Improve carbondata coverage for presto-integration code

GitBox
In reply to this post by GitBox

ajantha-bhat commented on pull request #4074:
URL: https://github.com/apache/carbondata/pull/4074#issuecomment-769764455


   LGTM


----------------------------------------------------------------
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]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #4074: [CARBONDATA-4109] Improve carbondata coverage for presto-integration code

GitBox
In reply to this post by GitBox

ajantha-bhat commented on a change in pull request #4074:
URL: https://github.com/apache/carbondata/pull/4074#discussion_r566782786



##########
File path: integration/presto/src/test/scala/org/apache/carbondata/presto/integrationtest/PrestoTestUsingSparkStore.scala
##########
@@ -20,14 +20,15 @@ package org.apache.carbondata.presto.integrationtest
 import java.io.{File}
 import java.util
 
+import io.prestosql.jdbc.PrestoArray

Review comment:
       I think presto db compile will fail after this. can you handle validation in the util method like `PrestoTestUtil#validateArrayOfPrimitiveTypeData`




----------------------------------------------------------------
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]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] ajantha-bhat removed a comment on pull request #4074: [CARBONDATA-4109] Improve carbondata coverage for presto-integration code

GitBox
In reply to this post by GitBox

ajantha-bhat removed a comment on pull request #4074:
URL: https://github.com/apache/carbondata/pull/4074#issuecomment-769764455


   LGTM


----------------------------------------------------------------
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]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #4074: [CARBONDATA-4109] Improve carbondata coverage for presto-integration code

GitBox
In reply to this post by GitBox

ajantha-bhat commented on a change in pull request #4074:
URL: https://github.com/apache/carbondata/pull/4074#discussion_r566783697



##########
File path: integration/presto/src/test/scala/org/apache/carbondata/presto/integrationtest/PrestoTestUsingSparkStore.scala
##########
@@ -20,14 +20,15 @@ package org.apache.carbondata.presto.integrationtest
 import java.io.{File}
 import java.util
 
+import io.prestosql.jdbc.PrestoArray

Review comment:
       Also please verify presto db compile once to catch if any other things broke compilation.




----------------------------------------------------------------
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]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] ajantha-bhat commented on pull request #4074: [CARBONDATA-4109] Improve carbondata coverage for presto-integration code

GitBox
In reply to this post by GitBox

ajantha-bhat commented on pull request #4074:
URL: https://github.com/apache/carbondata/pull/4074#issuecomment-769848180


   @akkio-97 : Please rebase the PR


----------------------------------------------------------------
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]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] ajantha-bhat commented on pull request #4074: [CARBONDATA-4109] Improve carbondata coverage for presto-integration code

GitBox
In reply to this post by GitBox

ajantha-bhat commented on pull request #4074:
URL: https://github.com/apache/carbondata/pull/4074#issuecomment-769926506


   LGTM


----------------------------------------------------------------
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]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4074: [CARBONDATA-4109] Improve carbondata coverage for presto-integration code

GitBox
In reply to this post by GitBox

CarbonDataQA2 commented on pull request #4074:
URL: https://github.com/apache/carbondata/pull/4074#issuecomment-769941718


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


----------------------------------------------------------------
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]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4074: [CARBONDATA-4109] Improve carbondata coverage for presto-integration code

GitBox
In reply to this post by GitBox

CarbonDataQA2 commented on pull request #4074:
URL: https://github.com/apache/carbondata/pull/4074#issuecomment-769951980


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


----------------------------------------------------------------
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]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] akkio-97 commented on a change in pull request #4074: [CARBONDATA-4109] Improve carbondata coverage for presto-integration code

GitBox
In reply to this post by GitBox

akkio-97 commented on a change in pull request #4074:
URL: https://github.com/apache/carbondata/pull/4074#discussion_r567001721



##########
File path: integration/presto/src/test/scala/org/apache/carbondata/presto/integrationtest/PrestoTestUsingSparkStore.scala
##########
@@ -20,14 +20,15 @@ package org.apache.carbondata.presto.integrationtest
 import java.io.{File}
 import java.util
 
+import io.prestosql.jdbc.PrestoArray

Review comment:
       done

##########
File path: integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/dataload/TestLoadDataWithDiffTimestampFormat.scala
##########
@@ -354,6 +355,7 @@ class TestLoadDataWithDiffTimestampFormat extends QueryTest with BeforeAndAfterA
 
   test("test load, update data with setlenient session level property for daylight " +
        "saving time from different timezone") {
+    TimeZone.setDefault(TimeZone.getTimeZone("America/Los_Angeles"))

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]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] akkio-97 commented on a change in pull request #4074: [CARBONDATA-4109] Improve carbondata coverage for presto-integration code

GitBox
In reply to this post by GitBox

akkio-97 commented on a change in pull request #4074:
URL: https://github.com/apache/carbondata/pull/4074#discussion_r567001931



##########
File path: integration/spark/src/test/scala/org/apache/carbondata/integration/spark/testsuite/dataload/SparkStoreCreatorForPresto.scala
##########
@@ -365,6 +370,84 @@ class SparkStoreCreatorForPresto extends QueryTest with BeforeAndAfterAll{
     sql(s"""LOAD DATA LOCAL INPATH '$resourcesPath/IUD/dest.csv' INTO TABLE streaming_table""")
   }
 
+  test("Test short vector datatype") {
+    sql("drop table if exists array_short")
+    sql(
+      "CREATE TABLE IF NOT EXISTS array_short (salary array<short>) STORED AS " +
+      "carbondata"
+    )
+    sql("insert into array_short select array(4352,35,3) ") // page datatype - short, adaptive
+    // integral codec
+  }
+
+  test("Test int vector datatype") {
+    sql("drop table if exists array_int")
+    sql(
+      "CREATE TABLE IF NOT EXISTS array_int (salary array<int>) STORED AS " +
+      "carbondata"
+    )
+    sql("insert into array_int select array(21474836,21474839,23,3) ") // page datatype - int,
+    // adaptive integral codec
+
+    sql("insert into array_int select array(21474836,21474839) ") // page datatype - byte, adaptive
+    // delta integral codec
+
+  }
+
+  test("Test long vector datatype") {
+    sql("drop table if exists array_long")
+    sql(
+      "CREATE TABLE IF NOT EXISTS array_long (salary array<long>) STORED AS " +
+      "carbondata"
+    )
+    // following are for adaptive integral codec
+    sql("insert into array_long select array(215,23,3) ") // page datatype - short
+
+    sql("insert into array_long select array(32800,23,3) ") // page datatype - short_int
+
+    sql("insert into array_long select array(32800,214748364,3) ") // page datatype - int
+
+  }
+
+  test("Test double vector datatype") {
+    sql("drop table if exists array_double")
+    sql(
+      "CREATE TABLE IF NOT EXISTS array_double (salary array<double>) STORED AS " +
+      "carbondata"
+    )
+    // following are for adaptive integral codec
+    sql("insert into array_double select array(2,3,4) ") // page datatype - byte
+
+    sql("insert into array_double select array(242,35,43) ") // page datatype - short
+
+    sql("insert into array_double select array(32799,32767) ") // page datatype - short_int
+
+    sql("insert into array_double select array(21546546,32546546,43211564) ") // page datatype - int
+
+    // following are for adaptive floating codec
+    sql("insert into array_double select array(327.99,3.2799) ") // page datatype - short_int
+
+    sql("insert into array_double select array(1,2345,108787.123) ") // page datatype - int
+
+  }
+
+  test("Test timestamp vector datatype") {
+    TimeZone.setDefault(TimeZone.getTimeZone("Asia/Kolkata"))
+    sql("drop table if exists array_timestamp")
+    sql(
+      "CREATE TABLE IF NOT EXISTS array_timestamp (time array<timestamp>) STORED AS " +
+      "carbondata"
+    )
+    sql("insert into array_timestamp select array('2020-01-11 12:00:45.0','2020-01-11 12:01:45.0')")
+    // page datatype - short-int, adaptive delta integral codec
+
+    sql("insert into array_timestamp select array('2020-01-10 12:30:45.0','2015-01-11 12:01:45.0')")
+    // page datatype - long, adaptive integral
+
+    // set timezone back to default
+    TimeZone.setDefault(TimeZone.getTimeZone("Asia/Shanghai"))

Review comment:
       yes, 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]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] asfgit closed pull request #4074: [CARBONDATA-4109] Improve carbondata coverage for presto-integration code

GitBox
In reply to this post by GitBox

asfgit closed pull request #4074:
URL: https://github.com/apache/carbondata/pull/4074


   


----------------------------------------------------------------
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]


12