GitHub user jackylk opened a pull request:
https://github.com/apache/carbondata/pull/3066
[CARBONDATA-3244] Add benchmark for Change Data Capture scenario
CDC (change data capture) is a common scenario for analyzing slowly changed
table in data warehouse.
It is good to add
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2963#discussion_r245013088
--- Diff: pom.xml ---
@@ -527,6 +526,7 @@
examples/spark2
datamap/lucene
datamap/bloom
+datamap
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/3019
LGTM
---
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3019#discussion_r244269584
--- Diff:
integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/table/CarbonCreateTableCommand.scala
---
@@ -157,7 +157,7 @@ case
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3019#discussion_r244269478
--- Diff:
core/src/main/java/org/apache/carbondata/core/datastore/impl/FileFactory.java
---
@@ -369,6 +369,24 @@ public static boolean
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3019#discussion_r244269435
--- Diff:
integration/presto/src/main/java/org/apache/carbondata/presto/CarbondataModule.java
---
@@ -17,62 +17,150 @@
package
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3019#discussion_r244254924
--- Diff:
integration/presto/src/main/java/org/apache/carbondata/presto/PrestoFilterUtil.java
---
@@ -78,32 +72,33 @@
private static final
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3019#discussion_r244254915
--- Diff:
integration/presto/src/main/java/org/apache/carbondata/presto/PrestoFilterUtil.java
---
@@ -78,32 +72,33 @@
private static final
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3019#discussion_r244254822
--- Diff:
integration/presto/src/test/scala/org/apache/carbondata/presto/server/PrestoServer.scala
---
@@ -129,6 +130,21 @@ class PrestoServer
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3019#discussion_r244254647
--- Diff:
integration/presto/src/test/scala/org/apache/carbondata/presto/util/CarbonDataStoreCreator.scala
---
@@ -80,7 +80,7 @@ object
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2161
LGTM. Merging into master branch
---
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/3018
LGTM
---
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2984
LGTM
---
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3019#discussion_r244172111
--- Diff:
integration/presto/src/main/java/org/apache/carbondata/presto/CarbondataConnectorFactory.java
---
@@ -17,69 +17,177 @@
package
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/3004
LGTM
---
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3017#discussion_r244065800
--- Diff:
core/src/main/java/org/apache/carbondata/core/locks/LocalFileLock.java ---
@@ -112,7 +112,7 @@ public LocalFileLock(String lockFileLocation
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3026#discussion_r244065240
--- Diff:
integration/spark-datasource/src/main/spark2.1andspark2.2/org/apache/spark/sql/CarbonDictionaryUtil.java
---
@@ -0,0 +1,116
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2963#discussion_r244007049
--- Diff:
datamap/example/src/main/java/org/apache/carbondata/datamap/minmax/MinMaxDataMapFactory.java
---
@@ -0,0 +1,353 @@
+/*
+ * Licensed
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/3001
@QiangCai Can you explain the dependency of readers after this PR? Ideally,
presto module should depends on streaming reader in core or hadoop modules.
---
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2970#discussion_r244005751
--- Diff:
processing/src/main/java/org/apache/carbondata/processing/store/CarbonFactDataHandlerColumnar.java
---
@@ -144,14 +144,14 @@ private void
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2970#discussion_r244005776
--- Diff:
processing/src/main/java/org/apache/carbondata/processing/store/CarbonFactDataHandlerColumnar.java
---
@@ -144,14 +144,14 @@ private void
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2970#discussion_r244005707
--- Diff:
core/src/main/java/org/apache/carbondata/core/util/CarbonThreadFactory.java ---
@@ -34,14 +34,26 @@
*/
private String name
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2161
@chandrasaripaka thanks for contributing this, I have only last 1 commend.
It can be merged after that is resolved, if it is really an issue
---
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3017#discussion_r244003686
--- Diff:
core/src/main/java/org/apache/carbondata/core/locks/LocalFileLock.java ---
@@ -112,7 +112,7 @@ public LocalFileLock(String lockFileLocation
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/3004
@SteNicholas Let's first merge this PR and then you can start refactory on
carbondata-hive module
---
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3004#discussion_r244002248
--- Diff:
integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestCreateHiveTableWithCarbonDS.scala
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3004#discussion_r244002150
--- Diff:
integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestCreateHiveTableWithCarbonDS.scala
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3004#discussion_r244001857
--- Diff:
integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestCreateHiveTableWithCarbonDS.scala
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3004#discussion_r244001900
--- Diff:
integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestCreateHiveTableWithCarbonDS.scala
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3004#discussion_r244001878
--- Diff:
integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestCreateHiveTableWithCarbonDS.scala
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3004#discussion_r244001830
--- Diff:
integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestCreateHiveTableWithCarbonDS.scala
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3004#discussion_r244001671
--- Diff:
integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestCreateHiveTableWithCarbonDS.scala
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3004#discussion_r244001631
--- Diff:
integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestCreateHiveTableWithCarbonDS.scala
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/3012
Thanks for contributing this fix
---
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/3012
LGTM
---
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2990
LGTM. Thanks for working on this
---
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2161
Since user normally uses Alluxio as a read cache, I think we can firstly
verify carbon on alluxio for the query scenario. As I am still not very sure
what is the correct way to implement rename
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2969
LGTM, can you squash your commits and rebase to master. I could not squash
it
---
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/3010
Please create a JIRA issue and put it in the PR title
---
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2990#discussion_r243250514
--- Diff:
integration/spark2/src/test/scala/org/apache/spark/carbondata/restructure/vectorreader/AlterTableColumnRenameTestCase.scala
---
@@ -0,0
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2990#discussion_r243250452
--- Diff:
integration/spark2/src/test/scala/org/apache/carbondata/datamap/bloom/BloomCoarseGrainDataMapSuite.scala
---
@@ -548,11 +548,16 @@ class
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2990#discussion_r243248566
--- Diff:
integration/spark2/src/main/scala/org/apache/spark/util/AlterTableUtil.scala ---
@@ -269,12 +269,50 @@ object AlterTableUtil
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2990#discussion_r243248408
--- Diff:
integration/spark2/src/main/scala/org/apache/spark/sql/execution/strategy/StreamingTableStrategy.scala
---
@@ -54,10 +54,10 @@ private[sql
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2990#discussion_r243247751
--- Diff:
integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableColRenameDataTypeChangeCommand.scala
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2990#discussion_r243245093
--- Diff:
integration/spark-common/src/main/scala/org/apache/spark/sql/catalyst/CarbonDDLSqlParser.scala
---
@@ -1511,7 +1514,16 @@ abstract class
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2990#discussion_r243242285
--- Diff:
integration/spark-common/src/main/scala/org/apache/spark/sql/catalyst/CarbonDDLSqlParser.scala
---
@@ -1487,16 +1487,19 @@ abstract class
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2969#discussion_r243150967
--- Diff:
integration/hive/src/main/scala/org/apache/carbondata/hiveexample/HiveExample.scala
---
@@ -22,52 +22,49 @@ import java.sql.{DriverManager
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2969#discussion_r243149546
--- Diff:
integration/hive/src/main/scala/org/apache/carbondata/hiveexample/HiveExample.scala
---
@@ -85,25 +82,35 @@ object HiveExample
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2969#discussion_r243149331
--- Diff:
integration/hive/src/main/scala/org/apache/carbondata/hiveexample/HiveExample.scala
---
@@ -49,17 +49,19 @@ object HiveExample
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2925
LGTM
---
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2984#discussion_r241938979
--- Diff:
datamap/bloom/src/main/java/org/apache/carbondata/datamap/bloom/BloomCoarseGrainDataMap.java
---
@@ -227,6 +231,12 @@ private String
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2925#discussion_r241774500
--- Diff: integration/spark2/pom.xml ---
@@ -134,6 +134,11 @@
+
--- End diff --
I see
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2973
Please close this if you are not working on it. Thanks
---
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2925#discussion_r241736440
--- Diff: integration/spark2/pom.xml ---
@@ -134,6 +134,11 @@
+
--- End diff --
I have never
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2925#discussion_r241736214
--- Diff:
integration/spark2/src/main/scala/org/apache/carbondata/spark/thriftserver/CarbonThriftServer.scala
---
@@ -48,8 +48,13 @@ object
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2890#discussion_r241735498
--- Diff:
processing/src/main/java/org/apache/carbondata/processing/util/CarbonLoaderUtil.java
---
@@ -569,25 +569,25 @@ public static Dictionary
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2914#discussion_r241285417
--- Diff:
core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java
---
@@ -40,1372 +41,1024 @@ private CarbonCommonConstants
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2914#discussion_r241285278
--- Diff:
core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java
---
@@ -40,1372 +41,1024 @@ private CarbonCommonConstants
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2914#discussion_r241285056
--- Diff:
core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java
---
@@ -40,1372 +41,1024 @@ private CarbonCommonConstants
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2978
@chenliang613 column vector code (CarbonColumnVector interface) and a base
implementation (CarbonColumnVectorImpl class) is in carbon-core module, but
still for every engine integration layer
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2978
LGTM
---
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2978
LGTM
---
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2978#discussion_r240530952
--- Diff:
integration/presto/src/main/java/org/apache/carbondata/presto/ColumnarVectorWrapperDirect.java
---
@@ -0,0 +1,320
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2978#discussion_r240530928
--- Diff:
integration/presto/src/main/java/org/apache/carbondata/presto/ColumnarVectorWrapperDirect.java
---
@@ -0,0 +1,320
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2978#discussion_r240531041
--- Diff:
integration/presto/src/main/java/org/apache/carbondata/presto/ColumnarVectorWrapperDirect.java
---
@@ -0,0 +1,320
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2978#discussion_r240530272
--- Diff:
integration/presto/src/main/java/org/apache/carbondata/presto/ColumnarVectorWrapperDirect.java
---
@@ -0,0 +1,320
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2978#discussion_r240528243
--- Diff:
integration/presto/src/main/java/org/apache/carbondata/presto/ColumnarVectorWrapperDirect.java
---
@@ -0,0 +1,320
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2978#discussion_r240528010
--- Diff:
integration/presto/src/main/java/org/apache/carbondata/presto/ColumnarVectorWrapperDirect.java
---
@@ -0,0 +1,320
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2978#discussion_r240528169
--- Diff:
integration/presto/src/main/java/org/apache/carbondata/presto/ColumnarVectorWrapperDirect.java
---
@@ -0,0 +1,320
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2978#discussion_r240526032
--- Diff:
core/src/main/java/org/apache/carbondata/core/scan/result/vector/impl/CarbonColumnVectorImpl.java
---
@@ -367,7 +378,14 @@ public void
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2978#discussion_r240524512
--- Diff:
core/src/main/java/org/apache/carbondata/core/datastore/chunk/store/impl/safe/AbstractNonDictionaryVectorFiller.java
---
@@ -83,20 +84,37
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2978#discussion_r240524219
--- Diff:
core/src/main/java/org/apache/carbondata/core/datastore/chunk/store/impl/safe/AbstractNonDictionaryVectorFiller.java
---
@@ -83,20 +84,37
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2978#discussion_r240519199
--- Diff:
core/src/main/java/org/apache/carbondata/core/scan/result/vector/impl/directread/SequentialFill.java
---
@@ -0,0 +1,33
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2940
LGTM
---
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2941#discussion_r236035510
--- Diff:
integration/spark2/src/main/scala/org/apache/spark/sql/__init__.py ---
@@ -0,0 +1,6 @@
+from __future__ import absolute_import
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2941#discussion_r236035500
--- Diff:
integration/spark2/src/main/scala/org/apache/spark/sql/CarbonSession.py ---
@@ -0,0 +1,122 @@
+from __future__ import print_function
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2941#discussion_r236035495
--- Diff:
integration/spark2/src/main/scala/org/apache/spark/sql/CarbonSession.py ---
@@ -0,0 +1,122 @@
+from __future__ import print_function
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2940#discussion_r235494866
--- Diff: integration/spark2/pom.xml ---
@@ -105,6 +105,11 @@
+
+ org.apache.httpcomponents
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2936
please describe this PR
---
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2926
LGTM
---
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2932
LGTM
can be merged after CI passed
---
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2872
LGTM
please change the title to incluce jira ticket number
---
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2863
LGTM
---
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2917
This is a good feature, will it show the number of rows got deleted or
updated?
hope it can get into next version.
---
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2926#discussion_r234399060
--- Diff:
core/src/main/java/org/apache/carbondata/core/constants/CarbonV3DataFormatConstants.java
---
@@ -35,9 +35,9 @@
String
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2926
`table_blocklet_size` is a table property that can be used in create table
---
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2872#discussion_r234209828
--- Diff:
core/src/main/java/org/apache/carbondata/core/datastore/ReusableDataBuffer.java
---
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2872#discussion_r234209700
--- Diff:
core/src/main/java/org/apache/carbondata/core/datastore/ReusableDataBuffer.java
---
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2922
Is this for CARBONDATA-3103?
---
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2920#discussion_r233470872
--- Diff:
store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonWriterBuilder.java
---
@@ -438,13 +438,13 @@ public CarbonWriter build() throws
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2920#discussion_r233437690
--- Diff:
store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonWriterBuilder.java
---
@@ -438,13 +438,13 @@ public CarbonWriter build() throws
GitHub user jackylk opened a pull request:
https://github.com/apache/carbondata/pull/2920
[HOTFIX] Improve log message in CarbonWriterBuilder
In master the log message is not proper:
`AppName is not set, please use writtenBy() API to set the App Namewhich is
using SDK
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2909
Since it is using CSV split in this PR, there are more tasks and each task
is smaller, thus we can get better parallelism and require less resource to run
in each task
---
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2863#discussion_r233341050
--- Diff:
core/src/main/java/org/apache/carbondata/core/scan/result/vector/impl/CarbonColumnVectorImpl.java
---
@@ -53,6 +53,12
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2863#discussion_r233340004
--- Diff:
core/src/main/java/org/apache/carbondata/core/datastore/page/encoding/compress/DirectCompressCodec.java
---
@@ -224,130 +238,134 @@ public
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2863#discussion_r29949
--- Diff:
core/src/main/java/org/apache/carbondata/core/datastore/page/encoding/compress/DirectCompressCodec.java
---
@@ -224,130 +238,134 @@ public
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2863#discussion_r28236
--- Diff:
core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java
---
@@ -2006,6 +2006,12 @@ private CarbonCommonConstants
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2863#discussion_r27530
--- Diff:
core/src/main/java/org/apache/carbondata/core/datastore/page/ColumnPageValueConverter.java
---
@@ -37,5 +40,6 @@
double decodeDouble
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2909
retest this please
---
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2914#discussion_r232629531
--- Diff:
core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java
---
@@ -1237,10 +1238,12 @@ private CarbonCommonConstants
1 - 100 of 3034 matches
Mail list logo