Skip to content
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-2550][CARBONDATA-2576][MV] Fix limit and average function issue in MV query #2480

Closed
wants to merge 2 commits into from

Conversation

ravipesala
Copy link
Contributor

This PR depends on #2453
Problem: Limit is not working on mv queries and the average is also not working.
Solution: Correct the limit queries and average queries through making as full refresh query.

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.

@CarbonDataQA
Copy link

Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/5763/

@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/6999/

@ravipesala
Copy link
Contributor Author

SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/5751/

@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/7090/

@ravipesala ravipesala force-pushed the mv-2550 branch 2 times, most recently from 3b7885e to 5d2634e Compare July 15, 2018 12:07
@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/7190/

@CarbonDataQA
Copy link

Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/5965/

@ravipesala
Copy link
Contributor Author

SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/5856/

case agg: AggregateExpression => agg
case agg: AggregateExpression =>
isFullReload = agg.aggregateFunction match {
case avg: Average => true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add comment to explain why it is true

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@@ -385,7 +390,7 @@ object MVHelper {
val relation =
g.dataMapTableRelation.get.asInstanceOf[MVPlanWrapper].plan.asInstanceOf[Select]
val aliasMap = getAttributeMap(relation.outputList, g.outputList)
val updatedFlagSpec: Seq[Seq[ArrayBuffer[SortOrder]]] = updateSortOrder(
val updatedFlagSpec: Seq[Seq[Any]] = updateSortOrder(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this for limit to work? can you add comment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/7288/

@CarbonDataQA
Copy link

Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6055/

@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/7298/

@CarbonDataQA
Copy link

Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6065/

@ravipesala
Copy link
Contributor Author

SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/5918/

@@ -429,7 +436,7 @@ object MVHelper {
/**
* Updates the flagspec of given select plan with attributes of relation select plan
*/
private def updateSortOrder(keepAlias: Boolean,
private def updateFlagSpec(keepAlias: Boolean,
select: Select,
relation: Select,
aliasMap: Map[AttributeKey, NamedExpression]) = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add return type to this func

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/7417/

@CarbonDataQA
Copy link

Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6182/

@jackylk
Copy link
Contributor

jackylk commented Jul 24, 2018

The conflict is due to test case adding in #2476
It is not a code problem and I have resolved the conflict. Merging this to master

@asfgit asfgit closed this in 83562ae Jul 24, 2018
asfgit pushed a commit that referenced this pull request Jul 30, 2018
… issue in MV query

Problem: Limit is not working on mv queries and the average is also not working.
Solution: Correct the limit queries and average queries through making as full refresh query.

This closes #2480
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants