[GitHub] [carbondata] kunal642 commented on a change in pull request #3697: [HOTFIX] Refactored hive related classes to use constants
kunal642 commented on a change in pull request #3697: [HOTFIX] Refactored hive
related classes to use constants
URL: https://github.com/apache/carbondata/pull/3697#discussion_r405298312
##
File path:
integration/hive/src/main/java/org/apache/carbondata/hive/MapredCarbonOutputFormat.java
##
@@ -61,24 +68,32 @@ public void checkOutputSpecs(FileSystem fileSystem,
JobConf jobConf) throws IOEx
public FileSinkOperator.RecordWriter getHiveRecordWriter(JobConf jc, Path
finalOutPath,
Class valueClass, boolean isCompressed, Properties
tableProperties,
Progressable progress) throws IOException {
+ThreadLocalSessionInfo.setConfigurationToCurrentThread(jc);
CarbonLoadModel carbonLoadModel = null;
+// Try to get loadmodel from JobConf.
String encodedString = jc.get(LOAD_MODEL);
if (encodedString != null) {
carbonLoadModel =
(CarbonLoadModel)
ObjectSerializationUtil.convertStringToObject(encodedString);
-}
-if (carbonLoadModel == null) {
- carbonLoadModel = HiveCarbonUtil.getCarbonLoadModel(tableProperties, jc);
} else {
- for (Map.Entry entry : tableProperties.entrySet()) {
-
carbonLoadModel.getCarbonDataLoadSchema().getCarbonTable().getTableInfo().getFactTable()
-.getTableProperties().put(entry.getKey().toString().toLowerCase(),
-entry.getValue().toString().toLowerCase());
+ // Try to get loadmodel from Container environment.
+ encodedString = System.getenv("carbon");
+ if (encodedString != null) {
+carbonLoadModel =
+(CarbonLoadModel)
ObjectSerializationUtil.convertStringToObject(encodedString);
+ } else {
+carbonLoadModel = HiveCarbonUtil.getCarbonLoadModel(tableProperties,
jc);
}
}
+for (Map.Entry entry : tableProperties.entrySet()) {
Review comment:
CarbonLoadModelBuilder is used to create LoadModel before this. So the
properties would be validated
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on issue #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data
ajantha-bhat commented on issue #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data URL: https://github.com/apache/carbondata/pull/3430#issuecomment-610785518 LGTM. Thanks for the contribution. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties
ajantha-bhat commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties URL: https://github.com/apache/carbondata/pull/3696#issuecomment-610784413 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #3688: [CARBONDATA-3765] Refactor Index Metadata for CG and FG Indexes
Indhumathi27 commented on a change in pull request #3688: [CARBONDATA-3765]
Refactor Index Metadata for CG and FG Indexes
URL: https://github.com/apache/carbondata/pull/3688#discussion_r405294912
##
File path: format/src/main/thrift/schema.thrift
##
@@ -203,9 +203,9 @@ struct ParentColumnTableRelation {
3: required string columnName
}
-struct DataMapSchema {
+struct IndexSchema {
Review comment:
These changes added in `schema.thrift` are to support pre-aggregate. I will
remove those changes from `schema.thrift` file
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] akashrn5 commented on a change in pull request #3686: [CARBONDATA-3759]Refactor segmentRefreshInfo and fix cache issue in multiple application scenario
akashrn5 commented on a change in pull request #3686: [CARBONDATA-3759]Refactor
segmentRefreshInfo and fix cache issue in multiple application scenario
URL: https://github.com/apache/carbondata/pull/3686#discussion_r405286913
##
File path:
core/src/main/java/org/apache/carbondata/core/statusmanager/SegmentUpdateStatusManager.java
##
@@ -783,26 +783,26 @@ private static void closeStreams(Closeable... streams) {
/**
* Returns the invalid timestamp range of a segment.
- * @param segmentId
- * @return
*/
- public UpdateVO getInvalidTimestampRange(String segmentId) {
+ public static UpdateVO getInvalidTimestampRange(LoadMetadataDetails
loadMetadataDetails) {
UpdateVO range = new UpdateVO();
-for (LoadMetadataDetails segment : segmentDetails) {
- if (segment.getLoadName().equalsIgnoreCase(segmentId)) {
-range.setSegmentId(segmentId);
-range.setFactTimestamp(segment.getLoadStartTime());
-if (!segment.getUpdateDeltaStartTimestamp().isEmpty() && !segment
-.getUpdateDeltaEndTimestamp().isEmpty()) {
- range.setUpdateDeltaStartTimestamp(
-
CarbonUpdateUtil.getTimeStampAsLong(segment.getUpdateDeltaStartTimestamp()));
- range.setLatestUpdateTimestamp(
-
CarbonUpdateUtil.getTimeStampAsLong(segment.getUpdateDeltaEndTimestamp()));
-}
-return range;
+getInvalidTimeStampRange(range, loadMetadataDetails);
+return range;
+ }
+
+ private static void getInvalidTimeStampRange(UpdateVO range,
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:
us...@infra.apache.org
With regards,
Apache Git Services
[jira] [Resolved] (CARBONDATA-3738) Delete seg. by ID is displaying as failed with invalid ID upon deleting a added parquet segment.
[ https://issues.apache.org/jira/browse/CARBONDATA-3738?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Kunal Kapoor resolved CARBONDATA-3738. -- Resolution: Fixed > Delete seg. by ID is displaying as failed with invalid ID upon deleting a > added parquet segment. > > > Key: CARBONDATA-3738 > URL: https://issues.apache.org/jira/browse/CARBONDATA-3738 > Project: CarbonData > Issue Type: Bug > Components: spark-integration >Reporter: Vikram Ahuja >Priority: Minor > Fix For: 2.0.0 > > Time Spent: 2h > Remaining Estimate: 0h > > Delete seg. by ID is displaying as failed with invalid ID upon deleting a > added parquet segment. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #3688: [CARBONDATA-3765] Refactor Index Metadata for CG and FG Indexes
Indhumathi27 commented on a change in pull request #3688: [CARBONDATA-3765]
Refactor Index Metadata for CG and FG Indexes
URL: https://github.com/apache/carbondata/pull/3688#discussion_r405286075
##
File path:
core/src/main/java/org/apache/carbondata/core/index/IndexChooser.java
##
@@ -68,15 +67,14 @@
public IndexChooser(CarbonTable carbonTable) throws IOException {
this.carbonTable = carbonTable;
// read all indexes for this table and populate CG and FG index list
-List visibleIndexes =
-DataMapStoreManager.getInstance().getAllVisibleIndexes(carbonTable);
-Map map =
DataMapStatusManager.readDataMapStatusMap();
+List visibleIndexes = carbonTable.getAllVisibleIndexes();
cgIndexes = new ArrayList<>(visibleIndexes.size());
fgIndexes = new ArrayList<>(visibleIndexes.size());
for (TableIndex visibleIndex : visibleIndexes) {
- DataMapStatusDetail status =
map.get(visibleIndex.getDataMapSchema().getDataMapName());
- if (status != null && status.isEnabled()) {
-IndexLevel level = visibleIndex.getIndexFactory().getDataMapLevel();
+ if
(visibleIndex.getIndexSchema().getProperties().get(CarbonCommonConstants.INDEX_STATUS)
Review comment:
DatamapSchemas from old store will be not taken/considered with current code
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:
us...@infra.apache.org
With regards,
Apache Git Services
[jira] [Updated] (CARBONDATA-3738) Delete seg. by ID is displaying as failed with invalid ID upon deleting a added parquet segment.
[ https://issues.apache.org/jira/browse/CARBONDATA-3738?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Kunal Kapoor updated CARBONDATA-3738: - Fix Version/s: 2.0.0 > Delete seg. by ID is displaying as failed with invalid ID upon deleting a > added parquet segment. > > > Key: CARBONDATA-3738 > URL: https://issues.apache.org/jira/browse/CARBONDATA-3738 > Project: CarbonData > Issue Type: Bug > Components: spark-integration >Reporter: Vikram Ahuja >Priority: Minor > Fix For: 2.0.0 > > Time Spent: 2h > Remaining Estimate: 0h > > Delete seg. by ID is displaying as failed with invalid ID upon deleting a > added parquet segment. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [carbondata] Indhumathi27 commented on issue #3688: [CARBONDATA-3765] Refactor Index Metadata for CG and FG Indexes
Indhumathi27 commented on issue #3688: [CARBONDATA-3765] Refactor Index Metadata for CG and FG Indexes URL: https://github.com/apache/carbondata/pull/3688#issuecomment-610775682 @ajantha-bhat there are seperate commits for support muti-tenant and datamap refactor in this PR. If still difficult to review, i can raise one more 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] asfgit closed pull request #3659: [CARBONDATA-3738] : Delete seg. by ID is displaying as failed with invalid ID upon deleting a added parquet segment.
asfgit closed pull request #3659: [CARBONDATA-3738] : Delete seg. by ID is displaying as failed with invalid ID upon deleting a added parquet segment. URL: https://github.com/apache/carbondata/pull/3659 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data
ajantha-bhat commented on a change in pull request #3430: [CARBONDATA-3565] Fix
complex binary data broken issue when loading dataframe data
URL: https://github.com/apache/carbondata/pull/3430#discussion_r405284719
##
File path:
integration/spark/src/main/scala/org/apache/carbondata/spark/rdd/NewCarbonDataLoadRDD.scala
##
@@ -386,10 +389,15 @@ class NewRddIterator(rddIter: Iterator[Row],
val len = columns.length
var i = 0
while (i < len) {
- columns(i) = CarbonScalaUtil.getString(row, i, carbonLoadModel,
serializationNullFormat,
-complexDelimiters, timeStampFormat, dateFormat,
-isVarcharType = i < isVarcharTypeMapping.size &&
isVarcharTypeMapping(i),
-isComplexType = i < isComplexTypeMapping.size &&
isComplexTypeMapping(i))
+ columns(i) = row.get(i) match {
+case bs if bs.isInstanceOf[Array[Byte]] && isDefaultBinaryDecoder =>
+ bs.asInstanceOf[Array[Byte]]
Review comment:
Ideally, It should not destroy. I don't know why it will be problem.
But if the byte[] already there. no need to convert to string and get byte[]
again. so merging 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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat edited a comment on issue #3688: [CARBONDATA-3765] Refactor Index Metadata for CG and FG Indexes
ajantha-bhat edited a comment on issue #3688: [CARBONDATA-3765] Refactor Index Metadata for CG and FG Indexes URL: https://github.com/apache/carbondata/pull/3688#issuecomment-610773012 @Indhumathi27 : Just checked few files on high level. Couldn't focus on functionality as there is distracting 300 file rename changes. Suggest to separate rename and functionality(multi-tenant) 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data
CarbonDataQA1 commented on issue #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data URL: https://github.com/apache/carbondata/pull/3430#issuecomment-610772876 Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2672/ 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on issue #3688: [CARBONDATA-3765] Refactor Index Metadata for CG and FG Indexes
ajantha-bhat commented on issue #3688: [CARBONDATA-3765] Refactor Index Metadata for CG and FG Indexes URL: https://github.com/apache/carbondata/pull/3688#issuecomment-610773012 @Indhumathi27 : Just checked few files on high level. Couldn't focus on functionality as there is attracting 300 file rename changes. Suggest to separate rename and functionality(multi-tenant) 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] kunal642 commented on issue #3659: [CARBONDATA-3738] : Delete seg. by ID is displaying as failed with invalid ID upon deleting a added parquet segment.
kunal642 commented on issue #3659: [CARBONDATA-3738] : Delete seg. by ID is displaying as failed with invalid ID upon deleting a added parquet segment. URL: https://github.com/apache/carbondata/pull/3659#issuecomment-610772531 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #3688: [CARBONDATA-3765] Refactor Index Metadata for CG and FG Indexes
ajantha-bhat commented on a change in pull request #3688: [CARBONDATA-3765]
Refactor Index Metadata for CG and FG Indexes
URL: https://github.com/apache/carbondata/pull/3688#discussion_r405273233
##
File path:
core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java
##
@@ -1536,12 +1536,12 @@ private CarbonCommonConstants() {
//
/**
- * key prefix for set command.
'carbon.datamap.visible.dbName.tableName.dmName = false' means
+ * key prefix for set command. 'carbon.index.visible.dbName.tableName.dmName
= false' means
* that the query on 'dbName.table' will not use the datamap 'dmName'
Review comment:
should have not combined refactoring and functionality PR together. It is
hard to review now.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #3688: [CARBONDATA-3765] Refactor Index Metadata for CG and FG Indexes
ajantha-bhat commented on a change in pull request #3688: [CARBONDATA-3765]
Refactor Index Metadata for CG and FG Indexes
URL: https://github.com/apache/carbondata/pull/3688#discussion_r405279772
##
File path: format/src/main/thrift/schema.thrift
##
@@ -203,9 +203,9 @@ struct ParentColumnTableRelation {
3: required string columnName
}
-struct DataMapSchema {
+struct IndexSchema {
Review comment:
@niuge01 , @QiangCai : please check in previous merged PR also for
compatibility issues. better to test old store once.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #3688: [CARBONDATA-3765] Refactor Index Metadata for CG and FG Indexes
ajantha-bhat commented on a change in pull request #3688: [CARBONDATA-3765]
Refactor Index Metadata for CG and FG Indexes
URL: https://github.com/apache/carbondata/pull/3688#discussion_r405274752
##
File path:
core/src/main/java/org/apache/carbondata/core/index/IndexStoreManager.java
##
@@ -820,8 +715,11 @@ public synchronized void clearInvalidIndex(CarbonTable
carbonTable, List
}
private boolean hasCGIndex(CarbonTable carbonTable) throws IOException {
-for (TableIndex tableIndex : getAllVisibleIndexes(carbonTable)) {
- if
(tableIndex.getIndexFactory().getDataMapLevel().equals(IndexLevel.CG)) {
+if (null == carbonTable) {
Review comment:
Table cannot be null 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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #3688: [CARBONDATA-3765] Refactor Index Metadata for CG and FG Indexes
ajantha-bhat commented on a change in pull request #3688: [CARBONDATA-3765]
Refactor Index Metadata for CG and FG Indexes
URL: https://github.com/apache/carbondata/pull/3688#discussion_r405278856
##
File path: format/src/main/thrift/schema.thrift
##
@@ -203,9 +203,9 @@ struct ParentColumnTableRelation {
3: required string columnName
}
-struct DataMapSchema {
+struct IndexSchema {
Review comment:
Changing thrift member name will leads to compatibility issue. Old store
will have as object of DataMapSchema, type cast may fail. Have tried reading
old store that has datamap schema ?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #3688: [CARBONDATA-3765] Refactor Index Metadata for CG and FG Indexes
ajantha-bhat commented on a change in pull request #3688: [CARBONDATA-3765]
Refactor Index Metadata for CG and FG Indexes
URL: https://github.com/apache/carbondata/pull/3688#discussion_r405272670
##
File path:
core/src/main/java/org/apache/carbondata/core/index/IndexChooser.java
##
@@ -68,15 +67,14 @@
public IndexChooser(CarbonTable carbonTable) throws IOException {
this.carbonTable = carbonTable;
// read all indexes for this table and populate CG and FG index list
-List visibleIndexes =
-DataMapStoreManager.getInstance().getAllVisibleIndexes(carbonTable);
-Map map =
DataMapStatusManager.readDataMapStatusMap();
+List visibleIndexes = carbonTable.getAllVisibleIndexes();
cgIndexes = new ArrayList<>(visibleIndexes.size());
fgIndexes = new ArrayList<>(visibleIndexes.size());
for (TableIndex visibleIndex : visibleIndexes) {
- DataMapStatusDetail status =
map.get(visibleIndex.getDataMapSchema().getDataMapName());
- if (status != null && status.isEnabled()) {
-IndexLevel level = visibleIndex.getIndexFactory().getDataMapLevel();
+ if
(visibleIndex.getIndexSchema().getProperties().get(CarbonCommonConstants.INDEX_STATUS)
Review comment:
How do you handle the compatibility during upgrade ? 1.6 already wrote in
system folder. Once upgraded. Table will not have these 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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #3688: [CARBONDATA-3765] Refactor Index Metadata for CG and FG Indexes
ajantha-bhat commented on a change in pull request #3688: [CARBONDATA-3765]
Refactor Index Metadata for CG and FG Indexes
URL: https://github.com/apache/carbondata/pull/3688#discussion_r405268051
##
File path:
core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java
##
@@ -1536,12 +1536,12 @@ private CarbonCommonConstants() {
//
/**
- * key prefix for set command.
'carbon.datamap.visible.dbName.tableName.dmName = false' means
+ * key prefix for set command. 'carbon.index.visible.dbName.tableName.dmName
= false' means
* that the query on 'dbName.table' will not use the datamap 'dmName'
Review comment:
```suggestion
* that the query on 'dbName.table' will not use the index 'dmName'
```
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] kunal642 commented on a change in pull request #3686: [CARBONDATA-3759]Refactor segmentRefreshInfo and fix cache issue in multiple application scenario
kunal642 commented on a change in pull request #3686: [CARBONDATA-3759]Refactor
segmentRefreshInfo and fix cache issue in multiple application scenario
URL: https://github.com/apache/carbondata/pull/3686#discussion_r405277532
##
File path:
core/src/main/java/org/apache/carbondata/core/statusmanager/SegmentUpdateStatusManager.java
##
@@ -783,26 +783,26 @@ private static void closeStreams(Closeable... streams) {
/**
* Returns the invalid timestamp range of a segment.
- * @param segmentId
- * @return
*/
- public UpdateVO getInvalidTimestampRange(String segmentId) {
+ public static UpdateVO getInvalidTimestampRange(LoadMetadataDetails
loadMetadataDetails) {
UpdateVO range = new UpdateVO();
-for (LoadMetadataDetails segment : segmentDetails) {
- if (segment.getLoadName().equalsIgnoreCase(segmentId)) {
-range.setSegmentId(segmentId);
-range.setFactTimestamp(segment.getLoadStartTime());
-if (!segment.getUpdateDeltaStartTimestamp().isEmpty() && !segment
-.getUpdateDeltaEndTimestamp().isEmpty()) {
- range.setUpdateDeltaStartTimestamp(
-
CarbonUpdateUtil.getTimeStampAsLong(segment.getUpdateDeltaStartTimestamp()));
- range.setLatestUpdateTimestamp(
-
CarbonUpdateUtil.getTimeStampAsLong(segment.getUpdateDeltaEndTimestamp()));
-}
-return range;
+getInvalidTimeStampRange(range, loadMetadataDetails);
+return range;
+ }
+
+ private static void getInvalidTimeStampRange(UpdateVO range,
Review comment:
Can we combine this with the above method as it is not called from anywhere
else
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] Zhangshunyu opened a new pull request #3698: [WIP] Test ci
Zhangshunyu opened a new pull request #3698: [WIP] Test ci URL: https://github.com/apache/carbondata/pull/3698 …t query ### Why is this PR needed? ### What changes were proposed in this PR? ### Does this PR introduce any user interface change? - No - Yes. (please explain the change and update document) ### Is any new testcase added? - No - 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] kunal642 commented on a change in pull request #3686: [CARBONDATA-3759]Refactor segmentRefreshInfo and fix cache issue in multiple application scenario
kunal642 commented on a change in pull request #3686: [CARBONDATA-3759]Refactor
segmentRefreshInfo and fix cache issue in multiple application scenario
URL: https://github.com/apache/carbondata/pull/3686#discussion_r405277532
##
File path:
core/src/main/java/org/apache/carbondata/core/statusmanager/SegmentUpdateStatusManager.java
##
@@ -783,26 +783,26 @@ private static void closeStreams(Closeable... streams) {
/**
* Returns the invalid timestamp range of a segment.
- * @param segmentId
- * @return
*/
- public UpdateVO getInvalidTimestampRange(String segmentId) {
+ public static UpdateVO getInvalidTimestampRange(LoadMetadataDetails
loadMetadataDetails) {
UpdateVO range = new UpdateVO();
-for (LoadMetadataDetails segment : segmentDetails) {
- if (segment.getLoadName().equalsIgnoreCase(segmentId)) {
-range.setSegmentId(segmentId);
-range.setFactTimestamp(segment.getLoadStartTime());
-if (!segment.getUpdateDeltaStartTimestamp().isEmpty() && !segment
-.getUpdateDeltaEndTimestamp().isEmpty()) {
- range.setUpdateDeltaStartTimestamp(
-
CarbonUpdateUtil.getTimeStampAsLong(segment.getUpdateDeltaStartTimestamp()));
- range.setLatestUpdateTimestamp(
-
CarbonUpdateUtil.getTimeStampAsLong(segment.getUpdateDeltaEndTimestamp()));
-}
-return range;
+getInvalidTimeStampRange(range, loadMetadataDetails);
+return range;
+ }
+
+ private static void getInvalidTimeStampRange(UpdateVO range,
Review comment:
Can we conbine this with the above method as it is not called from anywhere
else
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data
CarbonDataQA1 commented on issue #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data URL: https://github.com/apache/carbondata/pull/3430#issuecomment-610765475 Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/960/ 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #3697: [HOTFIX] Refactored hive related classes to use constants
ajantha-bhat commented on a change in pull request #3697: [HOTFIX] Refactored
hive related classes to use constants
URL: https://github.com/apache/carbondata/pull/3697#discussion_r405224399
##
File path:
integration/hive/src/main/java/org/apache/carbondata/hive/MapredCarbonOutputFormat.java
##
@@ -61,24 +68,32 @@ public void checkOutputSpecs(FileSystem fileSystem,
JobConf jobConf) throws IOEx
public FileSinkOperator.RecordWriter getHiveRecordWriter(JobConf jc, Path
finalOutPath,
Class valueClass, boolean isCompressed, Properties
tableProperties,
Progressable progress) throws IOException {
+ThreadLocalSessionInfo.setConfigurationToCurrentThread(jc);
CarbonLoadModel carbonLoadModel = null;
+// Try to get loadmodel from JobConf.
String encodedString = jc.get(LOAD_MODEL);
if (encodedString != null) {
carbonLoadModel =
(CarbonLoadModel)
ObjectSerializationUtil.convertStringToObject(encodedString);
-}
-if (carbonLoadModel == null) {
- carbonLoadModel = HiveCarbonUtil.getCarbonLoadModel(tableProperties, jc);
} else {
- for (Map.Entry entry : tableProperties.entrySet()) {
-
carbonLoadModel.getCarbonDataLoadSchema().getCarbonTable().getTableInfo().getFactTable()
-.getTableProperties().put(entry.getKey().toString().toLowerCase(),
-entry.getValue().toString().toLowerCase());
+ // Try to get loadmodel from Container environment.
+ encodedString = System.getenv("carbon");
+ if (encodedString != null) {
+carbonLoadModel =
+(CarbonLoadModel)
ObjectSerializationUtil.convertStringToObject(encodedString);
+ } else {
+carbonLoadModel = HiveCarbonUtil.getCarbonLoadModel(tableProperties,
jc);
}
}
+for (Map.Entry entry : tableProperties.entrySet()) {
Review comment:
ok. But it is ideal to validate before adding to table
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] akashrn5 commented on a change in pull request #3686: [CARBONDATA-3759]Refactor segmentRefreshInfo and fix cache issue in multiple application scenario
akashrn5 commented on a change in pull request #3686: [CARBONDATA-3759]Refactor
segmentRefreshInfo and fix cache issue in multiple application scenario
URL: https://github.com/apache/carbondata/pull/3686#discussion_r405266072
##
File path:
core/src/main/java/org/apache/carbondata/core/statusmanager/SegmentUpdateStatusManager.java
##
@@ -783,26 +783,28 @@ private static void closeStreams(Closeable... streams) {
/**
* Returns the invalid timestamp range of a segment.
- * @param segmentId
- * @return
*/
- public UpdateVO getInvalidTimestampRange(String segmentId) {
+ public static UpdateVO getInvalidTimestampRange(LoadMetadataDetails
loadMetadataDetails) {
UpdateVO range = new UpdateVO();
-for (LoadMetadataDetails segment : segmentDetails) {
- if (segment.getLoadName().equalsIgnoreCase(segmentId)) {
-range.setSegmentId(segmentId);
-range.setFactTimestamp(segment.getLoadStartTime());
-if (!segment.getUpdateDeltaStartTimestamp().isEmpty() && !segment
-.getUpdateDeltaEndTimestamp().isEmpty()) {
- range.setUpdateDeltaStartTimestamp(
-
CarbonUpdateUtil.getTimeStampAsLong(segment.getUpdateDeltaStartTimestamp()));
- range.setLatestUpdateTimestamp(
-
CarbonUpdateUtil.getTimeStampAsLong(segment.getUpdateDeltaEndTimestamp()));
-}
-return range;
+if (getInvalidTimeStampRange(range, loadMetadataDetails)) return range;
+return range;
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] akashrn5 commented on issue #3676: [CARBONDATA-3754]Clean up the data file and index files after SI rebuild
akashrn5 commented on issue #3676: [CARBONDATA-3754]Clean up the data file and index files after SI rebuild URL: https://github.com/apache/carbondata/pull/3676#issuecomment-610758369 @kunal642 rebased, please check 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] kunal642 commented on issue #3682: [CARBONDATA-3753] optimize double/float stats collector
kunal642 commented on issue #3682: [CARBONDATA-3753] optimize double/float stats collector URL: https://github.com/apache/carbondata/pull/3682#issuecomment-610756333 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] kunal642 commented on issue #3676: [CARBONDATA-3754]Clean up the data file and index files after SI rebuild
kunal642 commented on issue #3676: [CARBONDATA-3754]Clean up the data file and index files after SI rebuild URL: https://github.com/apache/carbondata/pull/3676#issuecomment-610755428 @akashrn5 Please rebase 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] kunal642 commented on a change in pull request #3686: [CARBONDATA-3759]Refactor segmentRefreshInfo and fix cache issue in multiple application scenario
kunal642 commented on a change in pull request #3686: [CARBONDATA-3759]Refactor
segmentRefreshInfo and fix cache issue in multiple application scenario
URL: https://github.com/apache/carbondata/pull/3686#discussion_r405259513
##
File path:
core/src/main/java/org/apache/carbondata/core/statusmanager/SegmentUpdateStatusManager.java
##
@@ -783,26 +783,28 @@ private static void closeStreams(Closeable... streams) {
/**
* Returns the invalid timestamp range of a segment.
- * @param segmentId
- * @return
*/
- public UpdateVO getInvalidTimestampRange(String segmentId) {
+ public static UpdateVO getInvalidTimestampRange(LoadMetadataDetails
loadMetadataDetails) {
UpdateVO range = new UpdateVO();
-for (LoadMetadataDetails segment : segmentDetails) {
- if (segment.getLoadName().equalsIgnoreCase(segmentId)) {
-range.setSegmentId(segmentId);
-range.setFactTimestamp(segment.getLoadStartTime());
-if (!segment.getUpdateDeltaStartTimestamp().isEmpty() && !segment
-.getUpdateDeltaEndTimestamp().isEmpty()) {
- range.setUpdateDeltaStartTimestamp(
-
CarbonUpdateUtil.getTimeStampAsLong(segment.getUpdateDeltaStartTimestamp()));
- range.setLatestUpdateTimestamp(
-
CarbonUpdateUtil.getTimeStampAsLong(segment.getUpdateDeltaEndTimestamp()));
-}
-return range;
+if (getInvalidTimeStampRange(range, loadMetadataDetails)) return range;
+return range;
Review comment:
if check can be removed
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] IceMimosa commented on issue #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data
IceMimosa commented on issue #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data URL: https://github.com/apache/carbondata/pull/3430#issuecomment-610733022 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] IceMimosa commented on a change in pull request #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data
IceMimosa commented on a change in pull request #3430: [CARBONDATA-3565] Fix
complex binary data broken issue when loading dataframe data
URL: https://github.com/apache/carbondata/pull/3430#discussion_r405238442
##
File path:
integration/spark/src/main/scala/org/apache/carbondata/spark/rdd/NewCarbonDataLoadRDD.scala
##
@@ -386,10 +389,15 @@ class NewRddIterator(rddIter: Iterator[Row],
val len = columns.length
var i = 0
while (i < len) {
- columns(i) = CarbonScalaUtil.getString(row, i, carbonLoadModel,
serializationNullFormat,
-complexDelimiters, timeStampFormat, dateFormat,
-isVarcharType = i < isVarcharTypeMapping.size &&
isVarcharTypeMapping(i),
-isComplexType = i < isComplexTypeMapping.size &&
isComplexTypeMapping(i))
+ columns(i) = row.get(i) match {
+case bs if bs.isInstanceOf[Array[Byte]] && isDefaultBinaryDecoder =>
+ bs.asInstanceOf[Array[Byte]]
Review comment:
Yes, in default binary decoder, `CarbonScalaUtil.getString` may destroy the
byte[].
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data
ajantha-bhat commented on a change in pull request #3430: [CARBONDATA-3565] Fix
complex binary data broken issue when loading dataframe data
URL: https://github.com/apache/carbondata/pull/3430#discussion_r405231033
##
File path:
integration/spark/src/main/scala/org/apache/carbondata/spark/rdd/NewCarbonDataLoadRDD.scala
##
@@ -386,10 +389,15 @@ class NewRddIterator(rddIter: Iterator[Row],
val len = columns.length
var i = 0
while (i < len) {
- columns(i) = CarbonScalaUtil.getString(row, i, carbonLoadModel,
serializationNullFormat,
-complexDelimiters, timeStampFormat, dateFormat,
-isVarcharType = i < isVarcharTypeMapping.size &&
isVarcharTypeMapping(i),
-isComplexType = i < isComplexTypeMapping.size &&
isComplexTypeMapping(i))
+ columns(i) = row.get(i) match {
+case bs if bs.isInstanceOf[Array[Byte]] && isDefaultBinaryDecoder =>
+ bs.asInstanceOf[Array[Byte]]
Review comment:
If this is byte[] you don't want to apply the binary decoder right ?
@niuge01 check this.
changes looks ok to me.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data
ajantha-bhat commented on a change in pull request #3430: [CARBONDATA-3565] Fix
complex binary data broken issue when loading dataframe data
URL: https://github.com/apache/carbondata/pull/3430#discussion_r405230266
##
File path:
integration/spark-common-cluster-test/src/test/scala/org/apache/carbondata/cluster/sdv/generated/datasource/CreateTableUsingSparkCarbonFileFormatTestCase.scala
##
@@ -254,10 +254,8 @@ class CreateTableUsingSparkCarbonFileFormatTestCase
extends FunSuite with Before
private def clearDataMapCache(): Unit = {
if (!sqlContext.sparkContext.version.startsWith("2.1")) {
- val mapSize = DataMapStoreManager.getInstance().getAllDataMaps.size()
DataMapStoreManager.getInstance()
-.clearDataMaps(AbsoluteTableIdentifier.from(writerPath))
- assert(mapSize > DataMapStoreManager.getInstance().getAllDataMaps.size())
+.clearIndex(AbsoluteTableIdentifier.from(writerPath))
Review comment:
revert changes in this file
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data
ajantha-bhat commented on a change in pull request #3430: [CARBONDATA-3565] Fix
complex binary data broken issue when loading dataframe data
URL: https://github.com/apache/carbondata/pull/3430#discussion_r405230154
##
File path:
integration/spark-common-cluster-test/src/test/scala/org/apache/carbondata/cluster/sdv/suite/SDVSuites.scala
##
@@ -127,7 +127,6 @@ class SDVSuites3 extends Suites with BeforeAndAfterAll {
class SDVSuites4 extends Suites with BeforeAndAfterAll {
val suites = new AlterTableTestCase ::
- new BucketingTestCase ::
Review comment:
revert changes in this file
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data
ajantha-bhat commented on a change in pull request #3430: [CARBONDATA-3565] Fix
complex binary data broken issue when loading dataframe data
URL: https://github.com/apache/carbondata/pull/3430#discussion_r405230190
##
File path:
integration/spark-common-cluster-test/src/test/scala/org/apache/carbondata/cluster/sdv/generated/datasource/SparkCarbonDataSourceTestCase.scala
##
@@ -735,10 +731,8 @@ class SparkCarbonDataSourceTestCase extends FunSuite with
BeforeAndAfterAll {
val frame = sqlContext.read.format("carbon").load(warehouse1 +
"/test_folder")
assert(frame.count() == 30)
assert(frame.where("c1='a1'").count() == 3)
- val mapSize = DataMapStoreManager.getInstance().getAllDataMaps.size()
Review comment:
revert changes in this file
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data
CarbonDataQA1 commented on issue #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data URL: https://github.com/apache/carbondata/pull/3430#issuecomment-610724038 Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/957/ 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data
ajantha-bhat commented on a change in pull request #3430: [CARBONDATA-3565] Fix
complex binary data broken issue when loading dataframe data
URL: https://github.com/apache/carbondata/pull/3430#discussion_r405229590
##
File path:
integration/spark-common-cluster-test/src/test/scala/org/apache/carbondata/cluster/sdv/generated/datasource/CreateTableUsingSparkCarbonFileFormatTestCase.scala
##
@@ -254,10 +254,8 @@ class CreateTableUsingSparkCarbonFileFormatTestCase
extends FunSuite with Before
private def clearDataMapCache(): Unit = {
if (!sqlContext.sparkContext.version.startsWith("2.1")) {
- val mapSize = DataMapStoreManager.getInstance().getAllDataMaps.size()
DataMapStoreManager.getInstance()
-.clearDataMaps(AbsoluteTableIdentifier.from(writerPath))
- assert(mapSize > DataMapStoreManager.getInstance().getAllDataMaps.size())
+.clearIndex(AbsoluteTableIdentifier.from(writerPath))
Review comment:
These files are nothing to do this issue right ? please rebase and keep only
your issue changes
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat edited a comment on issue #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data
ajantha-bhat edited a comment on issue #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data URL: https://github.com/apache/carbondata/pull/3430#issuecomment-610723016 @IceMimosa : No, we should not ignore it. Lets retest again, If passed. I will review and merge. you can add a test case with `CARBON_ENABLE_BAD_RECORD_HANDLING_FOR_INSERT = true` 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on issue #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data
ajantha-bhat commented on issue #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data URL: https://github.com/apache/carbondata/pull/3430#issuecomment-610723065 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on issue #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data
ajantha-bhat commented on issue #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data URL: https://github.com/apache/carbondata/pull/3430#issuecomment-610723016 @IceMimosa : No, we should not ignore it. Lets retest again, If passed. I will review and merge 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #3697: [HOTFIX] Refactored hive related classes to use constants
ajantha-bhat commented on a change in pull request #3697: [HOTFIX] Refactored
hive related classes to use constants
URL: https://github.com/apache/carbondata/pull/3697#discussion_r405224377
##
File path:
core/src/main/java/org/apache/carbondata/core/util/CarbonProperties.java
##
@@ -855,7 +855,11 @@ private void loadProperties() {
* Return the store path
*/
public static String getStorePath() {
-return getInstance().getProperty(CarbonCommonConstants.STORE_LOCATION);
+String storePath =
getInstance().getProperty(CarbonCommonConstants.STORE_LOCATION);
+if (storePath == null) {
Review comment:
ok
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #3697: [HOTFIX] Refactored hive related classes to use constants
ajantha-bhat commented on a change in pull request #3697: [HOTFIX] Refactored
hive related classes to use constants
URL: https://github.com/apache/carbondata/pull/3697#discussion_r405224399
##
File path:
integration/hive/src/main/java/org/apache/carbondata/hive/MapredCarbonOutputFormat.java
##
@@ -61,24 +68,32 @@ public void checkOutputSpecs(FileSystem fileSystem,
JobConf jobConf) throws IOEx
public FileSinkOperator.RecordWriter getHiveRecordWriter(JobConf jc, Path
finalOutPath,
Class valueClass, boolean isCompressed, Properties
tableProperties,
Progressable progress) throws IOException {
+ThreadLocalSessionInfo.setConfigurationToCurrentThread(jc);
CarbonLoadModel carbonLoadModel = null;
+// Try to get loadmodel from JobConf.
String encodedString = jc.get(LOAD_MODEL);
if (encodedString != null) {
carbonLoadModel =
(CarbonLoadModel)
ObjectSerializationUtil.convertStringToObject(encodedString);
-}
-if (carbonLoadModel == null) {
- carbonLoadModel = HiveCarbonUtil.getCarbonLoadModel(tableProperties, jc);
} else {
- for (Map.Entry entry : tableProperties.entrySet()) {
-
carbonLoadModel.getCarbonDataLoadSchema().getCarbonTable().getTableInfo().getFactTable()
-.getTableProperties().put(entry.getKey().toString().toLowerCase(),
-entry.getValue().toString().toLowerCase());
+ // Try to get loadmodel from Container environment.
+ encodedString = System.getenv("carbon");
+ if (encodedString != null) {
+carbonLoadModel =
+(CarbonLoadModel)
ObjectSerializationUtil.convertStringToObject(encodedString);
+ } else {
+carbonLoadModel = HiveCarbonUtil.getCarbonLoadModel(tableProperties,
jc);
}
}
+for (Map.Entry entry : tableProperties.entrySet()) {
Review comment:
ok
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data
CarbonDataQA1 commented on issue #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data URL: https://github.com/apache/carbondata/pull/3430#issuecomment-610718203 Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2667/ 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] IceMimosa commented on issue #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data
IceMimosa commented on issue #3430: [CARBONDATA-3565] Fix complex binary data broken issue when loading dataframe data URL: https://github.com/apache/carbondata/pull/3430#issuecomment-610718311 The issue may be fixed by #3538, but in `CARBON_ENABLE_BAD_RECORD_HANDLING_FOR_INSERT = true` the issue still exists. I don't know if I need to ignore it. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties
CarbonDataQA1 commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties URL: https://github.com/apache/carbondata/pull/3696#issuecomment-610370243 Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/956/ 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties
CarbonDataQA1 commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties URL: https://github.com/apache/carbondata/pull/3696#issuecomment-610363572 Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2666/ 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3687: [CARBONDATA-3761] Remove redundant conversion for complex type insert
CarbonDataQA1 commented on issue #3687: [CARBONDATA-3761] Remove redundant conversion for complex type insert URL: https://github.com/apache/carbondata/pull/3687#issuecomment-610358998 Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2665/ 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3687: [CARBONDATA-3761] Remove redundant conversion for complex type insert
CarbonDataQA1 commented on issue #3687: [CARBONDATA-3761] Remove redundant conversion for complex type insert URL: https://github.com/apache/carbondata/pull/3687#issuecomment-610357826 Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/955/ 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] kunal642 commented on a change in pull request #3697: [HOTFIX] Refactored hive related classes to use constants
kunal642 commented on a change in pull request #3697: [HOTFIX] Refactored hive
related classes to use constants
URL: https://github.com/apache/carbondata/pull/3697#discussion_r404719261
##
File path:
core/src/main/java/org/apache/carbondata/core/util/CarbonProperties.java
##
@@ -855,7 +855,11 @@ private void loadProperties() {
* Return the store path
*/
public static String getStorePath() {
-return getInstance().getProperty(CarbonCommonConstants.STORE_LOCATION);
+String storePath =
getInstance().getProperty(CarbonCommonConstants.STORE_LOCATION);
+if (storePath == null) {
Review comment:
Internally spark sets the value of spark.warehouse.dir to
hive.metastore.warehouse.dir, so hive.metastore.warehouse.dir is enough 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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] kunal642 commented on a change in pull request #3697: [HOTFIX] Refactored hive related classes to use constants
kunal642 commented on a change in pull request #3697: [HOTFIX] Refactored hive
related classes to use constants
URL: https://github.com/apache/carbondata/pull/3697#discussion_r404718652
##
File path:
integration/hive/src/main/java/org/apache/carbondata/hive/MapredCarbonOutputFormat.java
##
@@ -61,24 +68,32 @@ public void checkOutputSpecs(FileSystem fileSystem,
JobConf jobConf) throws IOEx
public FileSinkOperator.RecordWriter getHiveRecordWriter(JobConf jc, Path
finalOutPath,
Class valueClass, boolean isCompressed, Properties
tableProperties,
Progressable progress) throws IOException {
+ThreadLocalSessionInfo.setConfigurationToCurrentThread(jc);
CarbonLoadModel carbonLoadModel = null;
+// Try to get loadmodel from JobConf.
String encodedString = jc.get(LOAD_MODEL);
if (encodedString != null) {
carbonLoadModel =
(CarbonLoadModel)
ObjectSerializationUtil.convertStringToObject(encodedString);
-}
-if (carbonLoadModel == null) {
- carbonLoadModel = HiveCarbonUtil.getCarbonLoadModel(tableProperties, jc);
} else {
- for (Map.Entry entry : tableProperties.entrySet()) {
-
carbonLoadModel.getCarbonDataLoadSchema().getCarbonTable().getTableInfo().getFactTable()
-.getTableProperties().put(entry.getKey().toString().toLowerCase(),
-entry.getValue().toString().toLowerCase());
+ // Try to get loadmodel from Container environment.
+ encodedString = System.getenv("carbon");
+ if (encodedString != null) {
+carbonLoadModel =
+(CarbonLoadModel)
ObjectSerializationUtil.convertStringToObject(encodedString);
+ } else {
+carbonLoadModel = HiveCarbonUtil.getCarbonLoadModel(tableProperties,
jc);
}
}
+for (Map.Entry entry : tableProperties.entrySet()) {
Review comment:
in CarbonLoadModelBuilder
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #3697: [HOTFIX] Refactored hive related classes to use constants
ajantha-bhat commented on a change in pull request #3697: [HOTFIX] Refactored
hive related classes to use constants
URL: https://github.com/apache/carbondata/pull/3697#discussion_r404713321
##
File path:
integration/hive/src/main/java/org/apache/carbondata/hive/MapredCarbonOutputFormat.java
##
@@ -61,24 +68,32 @@ public void checkOutputSpecs(FileSystem fileSystem,
JobConf jobConf) throws IOEx
public FileSinkOperator.RecordWriter getHiveRecordWriter(JobConf jc, Path
finalOutPath,
Class valueClass, boolean isCompressed, Properties
tableProperties,
Progressable progress) throws IOException {
+ThreadLocalSessionInfo.setConfigurationToCurrentThread(jc);
CarbonLoadModel carbonLoadModel = null;
+// Try to get loadmodel from JobConf.
String encodedString = jc.get(LOAD_MODEL);
if (encodedString != null) {
carbonLoadModel =
(CarbonLoadModel)
ObjectSerializationUtil.convertStringToObject(encodedString);
-}
-if (carbonLoadModel == null) {
- carbonLoadModel = HiveCarbonUtil.getCarbonLoadModel(tableProperties, jc);
} else {
- for (Map.Entry entry : tableProperties.entrySet()) {
-
carbonLoadModel.getCarbonDataLoadSchema().getCarbonTable().getTableInfo().getFactTable()
-.getTableProperties().put(entry.getKey().toString().toLowerCase(),
-entry.getValue().toString().toLowerCase());
+ // Try to get loadmodel from Container environment.
+ encodedString = System.getenv("carbon");
+ if (encodedString != null) {
+carbonLoadModel =
+(CarbonLoadModel)
ObjectSerializationUtil.convertStringToObject(encodedString);
+ } else {
+carbonLoadModel = HiveCarbonUtil.getCarbonLoadModel(tableProperties,
jc);
}
}
+for (Map.Entry entry : tableProperties.entrySet()) {
Review comment:
where are these table properties validated ?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[jira] [Resolved] (CARBONDATA-3762) Block creating materilaized views with duplicate column
[ https://issues.apache.org/jira/browse/CARBONDATA-3762?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Akash R Nilugal resolved CARBONDATA-3762. - Fix Version/s: 2.0.0 Resolution: Fixed > Block creating materilaized views with duplicate column > --- > > Key: CARBONDATA-3762 > URL: https://issues.apache.org/jira/browse/CARBONDATA-3762 > Project: CarbonData > Issue Type: Bug >Reporter: Indhumathi Muthumurugesh >Priority: Major > Fix For: 2.0.0 > > Time Spent: 2.5h > Remaining Estimate: 0h > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [carbondata] asfgit closed pull request #3690: [CARBONDATA-3762] Block creating Materialized view's with duplicate column
asfgit closed pull request #3690: [CARBONDATA-3762] Block creating Materialized view's with duplicate column URL: https://github.com/apache/carbondata/pull/3690 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] akashrn5 commented on issue #3690: [CARBONDATA-3762] Block creating Materialized view's with duplicate column
akashrn5 commented on issue #3690: [CARBONDATA-3762] Block creating Materialized view's with duplicate column URL: https://github.com/apache/carbondata/pull/3690#issuecomment-610310586 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #3697: [HOTFIX] Refactored hive related classes to use constants
ajantha-bhat commented on a change in pull request #3697: [HOTFIX] Refactored
hive related classes to use constants
URL: https://github.com/apache/carbondata/pull/3697#discussion_r404704207
##
File path:
core/src/main/java/org/apache/carbondata/core/util/CarbonProperties.java
##
@@ -855,7 +855,11 @@ private void loadProperties() {
* Return the store path
*/
public static String getStorePath() {
-return getInstance().getProperty(CarbonCommonConstants.STORE_LOCATION);
+String storePath =
getInstance().getProperty(CarbonCommonConstants.STORE_LOCATION);
+if (storePath == null) {
Review comment:
what about `spark.warehouse.dir` ??
and what if `hive.metastore.warehouse.dir` is not configured ?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties
ajantha-bhat commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties URL: https://github.com/apache/carbondata/pull/3696#issuecomment-610305928 LGTM. waiting for build to merge 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties
ajantha-bhat commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties URL: https://github.com/apache/carbondata/pull/3696#issuecomment-610305169 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3690: [CARBONDATA-3762] Block creating Materialized view's with duplicate column
CarbonDataQA1 commented on issue #3690: [CARBONDATA-3762] Block creating Materialized view's with duplicate column URL: https://github.com/apache/carbondata/pull/3690#issuecomment-610293768 Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/953/ 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3687: [CARBONDATA-3761] Remove redundant conversion for complex type insert
CarbonDataQA1 commented on issue #3687: [CARBONDATA-3761] Remove redundant conversion for complex type insert URL: https://github.com/apache/carbondata/pull/3687#issuecomment-610288582 Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2664/ 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3687: [CARBONDATA-3761] Remove redundant conversion for complex type insert
CarbonDataQA1 commented on issue #3687: [CARBONDATA-3761] Remove redundant conversion for complex type insert URL: https://github.com/apache/carbondata/pull/3687#issuecomment-610288144 Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/954/ 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3690: [CARBONDATA-3762] Block creating Materialized view's with duplicate column
CarbonDataQA1 commented on issue #3690: [CARBONDATA-3762] Block creating Materialized view's with duplicate column URL: https://github.com/apache/carbondata/pull/3690#issuecomment-610286943 Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2663/ 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on issue #3687: [CARBONDATA-3761] Remove redundant conversion for complex type insert
ajantha-bhat commented on issue #3687: [CARBONDATA-3761] Remove redundant conversion for complex type insert URL: https://github.com/apache/carbondata/pull/3687#issuecomment-610286383 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3697: [HOTFIX] Refactored hive related classes to use constants
CarbonDataQA1 commented on issue #3697: [HOTFIX] Refactored hive related classes to use constants URL: https://github.com/apache/carbondata/pull/3697#issuecomment-610277405 Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2662/ 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3697: [HOTFIX] Refactored hive related classes to use constants
CarbonDataQA1 commented on issue #3697: [HOTFIX] Refactored hive related classes to use constants URL: https://github.com/apache/carbondata/pull/3697#issuecomment-610275632 Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/952/ 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties
CarbonDataQA1 commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties URL: https://github.com/apache/carbondata/pull/3696#issuecomment-610275540 Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2661/ 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties
CarbonDataQA1 commented on issue #3696: [HOTFIX] Fix Repeated access to getSegmentProperties URL: https://github.com/apache/carbondata/pull/3696#issuecomment-610272808 Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/951/ 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: us...@infra.apache.org With regards, Apache Git Services
[jira] [Resolved] (CARBONDATA-3766) Fix wrong dataSize being displayed for show segments for added segments
[ https://issues.apache.org/jira/browse/CARBONDATA-3766?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Akash R Nilugal resolved CARBONDATA-3766. - Fix Version/s: 2.0.0 Resolution: Fixed > Fix wrong dataSize being displayed for show segments for added segments > --- > > Key: CARBONDATA-3766 > URL: https://issues.apache.org/jira/browse/CARBONDATA-3766 > Project: CarbonData > Issue Type: Bug >Reporter: Kunal Kapoor >Assignee: Kunal Kapoor >Priority: Major > Fix For: 2.0.0 > > Time Spent: 1h 50m > Remaining Estimate: 0h > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [carbondata] asfgit closed pull request #3680: [CARBONDATA-3766] Fixed desc formatted and show segment data size issues
asfgit closed pull request #3680: [CARBONDATA-3766] Fixed desc formatted and show segment data size issues URL: https://github.com/apache/carbondata/pull/3680 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] akashrn5 commented on issue #3680: [CARBONDATA-3766] Fixed desc formatted and show segment data size issues
akashrn5 commented on issue #3680: [CARBONDATA-3766] Fixed desc formatted and show segment data size issues URL: https://github.com/apache/carbondata/pull/3680#issuecomment-610244816 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] kunal642 commented on a change in pull request #3680: [CARBONDATA-3766] Fixed desc formatted and show segment data size issues
kunal642 commented on a change in pull request #3680: [CARBONDATA-3766] Fixed
desc formatted and show segment data size issues
URL: https://github.com/apache/carbondata/pull/3680#discussion_r404617995
##
File path:
core/src/main/java/org/apache/carbondata/core/metadata/SegmentFileStore.java
##
@@ -527,10 +527,11 @@ public static boolean updateTableStatusFile(CarbonTable
carbonTable, String segm
for (LoadMetadataDetails detail : listOfLoadFolderDetailsArray) {
// if the segments is in the list of marked for delete then update
the status.
if (segmentId.equals(detail.getLoadName())) {
+detail.setLoadEndTime(System.currentTimeMillis());
Review comment:
I think we should not add external segments check for common things, it
should be okay if we set here because we are not using this end time in any
other logic/decision making
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] akashrn5 commented on issue #3690: [CARBONDATA-3762] Block creating Materialized view's with duplicate column
akashrn5 commented on issue #3690: [CARBONDATA-3762] Block creating Materialized view's with duplicate column URL: https://github.com/apache/carbondata/pull/3690#issuecomment-610219374 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #3690: [CARBONDATA-3762] Block creating Materialized view's with duplicate column
Indhumathi27 commented on a change in pull request #3690: [CARBONDATA-3762]
Block creating Materialized view's with duplicate column
URL: https://github.com/apache/carbondata/pull/3690#discussion_r404585118
##
File path:
mv/core/src/test/scala/org/apache/carbondata/mv/rewrite/TestAllOperationsOnMV.scala
##
@@ -626,6 +626,49 @@ class TestAllOperationsOnMV extends QueryTest with
BeforeAndAfterEach {
sql("drop table IF EXISTS maintable")
}
+ test("test duplicate column name in mv") {
+sql("drop table IF EXISTS maintable")
+sql("create table maintable(name string, c_code int, price int) STORED AS
carbondata")
+sql("insert into table maintable values('abc',21,2000),('mno',24,3000)")
+sql("drop materialized view if exists mv1")
+val res1 = sql("select name,sum(c_code) from maintable group by name")
+val res2 = sql("select name, name,sum(c_code),sum(c_code) from maintable
group by name")
+val res3 = sql("select c_code,price from maintable")
+sql("create materialized view mv1 as select name,sum(c_code) from
maintable group by name")
+val df1 = sql("select name,sum(c_code) from maintable group by name")
+TestUtil.verifyMVDataMap(df1.queryExecution.optimizedPlan, "mv1")
+checkAnswer(res1, df1)
+val df2 = sql("select name, name,sum(c_code),sum(c_code) from maintable
group by name")
+TestUtil.verifyMVDataMap(df2.queryExecution.optimizedPlan, "mv1")
+checkAnswer(df2, res2)
+sql("drop materialized view if exists mv2")
+sql("create materialized view mv2 as select c_code,price from maintable")
+val df3 = sql("select c_code,price from maintable")
+TestUtil.verifyMVDataMap(df3.queryExecution.optimizedPlan, "mv2")
+checkAnswer(res3, df3)
+val df4 = sql("select c_code,price,price,c_code from maintable")
+TestUtil.verifyMVDataMap(df4.queryExecution.optimizedPlan, "mv2")
+checkAnswer(df4, Seq(Row(21,2000,2000,21), Row(24,3000,3000,24)))
+sql("drop table IF EXISTS maintable")
+ }
+
+ test("test duplicate column with different alias name") {
+sql("drop table IF EXISTS maintable")
+sql("create table maintable(name string, c_code int, price int) STORED AS
carbondata")
+sql("insert into table maintable values('abc',21,2000),('mno',24,3000)")
+sql("drop materialized view if exists mv1")
+intercept[MalformedMVCommandException] {
+ sql("create materialized view mv1 as select name,sum(c_code),sum(c_code)
as a from maintable group by name")
+}.getMessage.contains("Cannot create mv having duplicate column with
different alias name: sum(CAST(maintable.`c_code` AS BIGINT)) AS `a`")
Review comment:
I am directly doing (column).sql to display columns, because in case of join
on more tables, it is required to display with qualifier. In case of above
scenario, it will be difficult to get only column name. I think it is better to
keep like 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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] akashrn5 commented on a change in pull request #3680: [CARBONDATA-3766] Fixed desc formatted and show segment data size issues
akashrn5 commented on a change in pull request #3680: [CARBONDATA-3766] Fixed
desc formatted and show segment data size issues
URL: https://github.com/apache/carbondata/pull/3680#discussion_r404582168
##
File path:
core/src/main/java/org/apache/carbondata/core/metadata/SegmentFileStore.java
##
@@ -527,10 +527,11 @@ public static boolean updateTableStatusFile(CarbonTable
carbonTable, String segm
for (LoadMetadataDetails detail : listOfLoadFolderDetailsArray) {
// if the segments is in the list of marked for delete then update
the status.
if (segmentId.equals(detail.getLoadName())) {
+detail.setLoadEndTime(System.currentTimeMillis());
Review comment:
is this common code to both carbon and non carbon segments? if it is then
better to set it only in non carbon segments 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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] akashrn5 commented on a change in pull request #3690: [CARBONDATA-3762] Block creating Materialized view's with duplicate column
akashrn5 commented on a change in pull request #3690: [CARBONDATA-3762] Block
creating Materialized view's with duplicate column
URL: https://github.com/apache/carbondata/pull/3690#discussion_r404580379
##
File path:
mv/core/src/test/scala/org/apache/carbondata/mv/rewrite/TestAllOperationsOnMV.scala
##
@@ -626,6 +626,49 @@ class TestAllOperationsOnMV extends QueryTest with
BeforeAndAfterEach {
sql("drop table IF EXISTS maintable")
}
+ test("test duplicate column name in mv") {
+sql("drop table IF EXISTS maintable")
+sql("create table maintable(name string, c_code int, price int) STORED AS
carbondata")
+sql("insert into table maintable values('abc',21,2000),('mno',24,3000)")
+sql("drop materialized view if exists mv1")
+val res1 = sql("select name,sum(c_code) from maintable group by name")
+val res2 = sql("select name, name,sum(c_code),sum(c_code) from maintable
group by name")
+val res3 = sql("select c_code,price from maintable")
+sql("create materialized view mv1 as select name,sum(c_code) from
maintable group by name")
+val df1 = sql("select name,sum(c_code) from maintable group by name")
+TestUtil.verifyMVDataMap(df1.queryExecution.optimizedPlan, "mv1")
+checkAnswer(res1, df1)
+val df2 = sql("select name, name,sum(c_code),sum(c_code) from maintable
group by name")
+TestUtil.verifyMVDataMap(df2.queryExecution.optimizedPlan, "mv1")
+checkAnswer(df2, res2)
+sql("drop materialized view if exists mv2")
+sql("create materialized view mv2 as select c_code,price from maintable")
+val df3 = sql("select c_code,price from maintable")
+TestUtil.verifyMVDataMap(df3.queryExecution.optimizedPlan, "mv2")
+checkAnswer(res3, df3)
+val df4 = sql("select c_code,price,price,c_code from maintable")
+TestUtil.verifyMVDataMap(df4.queryExecution.optimizedPlan, "mv2")
+checkAnswer(df4, Seq(Row(21,2000,2000,21), Row(24,3000,3000,24)))
+sql("drop table IF EXISTS maintable")
+ }
+
+ test("test duplicate column with different alias name") {
+sql("drop table IF EXISTS maintable")
+sql("create table maintable(name string, c_code int, price int) STORED AS
carbondata")
+sql("insert into table maintable values('abc',21,2000),('mno',24,3000)")
+sql("drop materialized view if exists mv1")
+intercept[MalformedMVCommandException] {
+ sql("create materialized view mv1 as select name,sum(c_code),sum(c_code)
as a from maintable group by name")
+}.getMessage.contains("Cannot create mv having duplicate column with
different alias name: sum(CAST(maintable.`c_code` AS BIGINT)) AS `a`")
Review comment:
can't we give simpler column name than `sum(CAST(maintable.`c_code` AS
BIGINT)) AS `a`` is it possible? may be in case of big queries it will be
difficult to find out i think.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] akashrn5 commented on a change in pull request #3690: [CARBONDATA-3762] Block creating Materialized view's with duplicate column
akashrn5 commented on a change in pull request #3690: [CARBONDATA-3762] Block
creating Materialized view's with duplicate column
URL: https://github.com/apache/carbondata/pull/3690#discussion_r404580379
##
File path:
mv/core/src/test/scala/org/apache/carbondata/mv/rewrite/TestAllOperationsOnMV.scala
##
@@ -626,6 +626,49 @@ class TestAllOperationsOnMV extends QueryTest with
BeforeAndAfterEach {
sql("drop table IF EXISTS maintable")
}
+ test("test duplicate column name in mv") {
+sql("drop table IF EXISTS maintable")
+sql("create table maintable(name string, c_code int, price int) STORED AS
carbondata")
+sql("insert into table maintable values('abc',21,2000),('mno',24,3000)")
+sql("drop materialized view if exists mv1")
+val res1 = sql("select name,sum(c_code) from maintable group by name")
+val res2 = sql("select name, name,sum(c_code),sum(c_code) from maintable
group by name")
+val res3 = sql("select c_code,price from maintable")
+sql("create materialized view mv1 as select name,sum(c_code) from
maintable group by name")
+val df1 = sql("select name,sum(c_code) from maintable group by name")
+TestUtil.verifyMVDataMap(df1.queryExecution.optimizedPlan, "mv1")
+checkAnswer(res1, df1)
+val df2 = sql("select name, name,sum(c_code),sum(c_code) from maintable
group by name")
+TestUtil.verifyMVDataMap(df2.queryExecution.optimizedPlan, "mv1")
+checkAnswer(df2, res2)
+sql("drop materialized view if exists mv2")
+sql("create materialized view mv2 as select c_code,price from maintable")
+val df3 = sql("select c_code,price from maintable")
+TestUtil.verifyMVDataMap(df3.queryExecution.optimizedPlan, "mv2")
+checkAnswer(res3, df3)
+val df4 = sql("select c_code,price,price,c_code from maintable")
+TestUtil.verifyMVDataMap(df4.queryExecution.optimizedPlan, "mv2")
+checkAnswer(df4, Seq(Row(21,2000,2000,21), Row(24,3000,3000,24)))
+sql("drop table IF EXISTS maintable")
+ }
+
+ test("test duplicate column with different alias name") {
+sql("drop table IF EXISTS maintable")
+sql("create table maintable(name string, c_code int, price int) STORED AS
carbondata")
+sql("insert into table maintable values('abc',21,2000),('mno',24,3000)")
+sql("drop materialized view if exists mv1")
+intercept[MalformedMVCommandException] {
+ sql("create materialized view mv1 as select name,sum(c_code),sum(c_code)
as a from maintable group by name")
+}.getMessage.contains("Cannot create mv having duplicate column with
different alias name: sum(CAST(maintable.`c_code` AS BIGINT)) AS `a`")
Review comment:
can't we give simpler column name than `sum(CAST(maintable.`c_code` AS
BIGINT)) AS `a`` is it possible?
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:
us...@infra.apache.org
With regards,
Apache Git Services
