New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CARBONDATA-3189] Fix PreAggregate Datamap Issue #3010
Conversation
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1877/ |
Please create a JIRA issue and put it in the PR title |
Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2087/ |
Build Failed with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10132/ |
Can you show up the error message when load and query failing |
retest this please |
add to whitelist |
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1918/ |
Build Failed with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10171/ |
Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2128/ |
@Shubh18s Please fix the test case failures |
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1960/ |
@@ -1449,12 +1449,9 @@ private CarbonCommonConstants() { | |||
|
|||
public static final String SUPPORT_DIRECT_QUERY_ON_DATAMAP_DEFAULTVALUE = "false"; | |||
|
|||
@CarbonProperty | |||
public static final String VALIDATE_DIRECT_QUERY_ON_DATAMAP = | |||
"carbon.query.validate.direct.query.on.datamap"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can remove this property and use carbon.query.directQueryOnDataMap.enabled.
Need to remove from configuration-paramters.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I made changes to configuration-paramters.md accordingly and removed this property.
if (relation.carbonRelation.carbonTable.isChildDataMap) { | ||
isPreAggDataMapExists = true | ||
} | ||
} | ||
val validateQuery = CarbonProperties.getInstance | ||
.getProperty(CarbonCommonConstants.VALIDATE_DIRECT_QUERY_ON_DATAMAP, | ||
CarbonCommonConstants.VALIDATE_DIRECT_QUERY_ON_DATAMAP_DEFAULTVALUE).toBoolean | ||
.getProperty(CarbonCommonConstants.VALIDATE_DIRECT_QUERY_ON_DATAMAP) | ||
var isThrowException = false | ||
// if validate query is enabled and relation contains pre aggregate data map |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change the comment according to the changes done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Build Failed with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10212/ |
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2233/ |
9a4df75
to
80094ca
Compare
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2260/ |
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2055/ |
retest this please |
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2262/ |
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2057/ |
Build Failed with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10309/ |
@Shubh18s Please fix 2.3 build |
retest this please |
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2107/ |
Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10361/ |
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2312/ |
@@ -1445,16 +1445,10 @@ private CarbonCommonConstants() { | |||
|
|||
@CarbonProperty(dynamicConfigurable = true) | |||
public static final String SUPPORT_DIRECT_QUERY_ON_DATAMAP = | |||
"carbon.query.directQueryOnDataMap.enabled"; | |||
"carbon.query.directQueryOnDataMap.enabled"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert this change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
aggregateColumns += s"${a.getAggFunction match { | ||
case "count" => "sum" | ||
case _ => a.getAggFunction}}(${a.getColumnName})" | ||
aggregateColumns += s"${ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert this change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
} | ||
|
||
/** | ||
/** CarbonProperties.getInstance().addProperty(CarbonCommonConstants |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why this change is requried??
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2125/ |
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2331/ |
Build Failed with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10379/ |
retest this please |
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2136/ |
Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10390/ |
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2342/ |
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2358/ |
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2152/ |
Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10404/ |
retest this please |
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2168/ |
Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10421/ |
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2382/ |
Respected Reviewer please Merge this PR. |
LGTM |
Why haven't test case for this PR? @Shubh18s @kumarvishal09 |
Problem - Load and Select query was failing on table with preaggregate datamap. Cause - Previously if query on datamap was not enabled in thread, there was no check afterwards. Solution - After checking whether thread param for Direct Query On Datamap is enable. If not enable, we check in session params and then global. This closes #3010
Problem - Load and Select query was failing on table with preaggregate datamap. Cause - Previously if query on datamap was not enabled in thread, there was no check afterwards. Solution - After checking whether thread param for Direct Query On Datamap is enable. If not enable, we check in session params and then global. This closes apache#3010
Problem -
Load and Select query was failing on table with preaggregate datamap.
Cause -
Previously if query on datamap was not enabled in thread, there was no check afterwards.
Solution -
After checking whether thread param for Direct Query On Datamap is enable. If not enable, we check in session params and then global.
Be sure to do all of the following checklist to help us incorporate
your contribution quickly and easily:
Any interfaces changed?
Any backward compatibility impacted?
Document update required?
Testing done
Please provide details on
- Whether new unit test cases have been added or why no new tests are required?
- How it is tested? Please attach test report.
- Is it a performance related change? Please attach the performance test report.
- Any additional information to help reviewers in testing this change.
For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.