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.

Sign up for GitHub

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

Jump to bottom

[CARBONDATA-3530] Support Create timeseries MV Datamap with the supported granularity levels. #3437

Closed
wants to merge 1 commit into from
Closed

[CARBONDATA-3530] Support Create timeseries MV Datamap with the supported granularity levels. #3437

wants to merge 1 commit into from

Conversation

Indhumathi27
Copy link
Contributor

@Indhumathi27 Indhumathi27 commented Nov 6, 2019 •
edited

Use existing mv datamapProvider to create datamap with timeseries queries with the supported granularity levels

This PR supports,

  1. Creating MV datamap with timeseries queries
    Syntax for creating mv-timeseries datamap:
    CREATE datamap <datamap_name> on <table_name> using 'mv' as Select timeseries(time_column,'granularity'), ... from <table_name> group by timeseries(time_column,'granularity')
    Queries having timeseries udf of timestamp/date column can be created as a mv datamap.
  2. Added Validations for providing time stamp column and granularity.

Please refer design document attached in https://issues.apache.org/jira/browse/CARBONDATA-3525 for more info.

  • Any interfaces changed?

  • Any backward compatibility impacted?

  • Document update required?

  • Testing done

  • For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.

@CarbonDataQA
Copy link

Build Failed with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/760/

@Indhumathi27 Indhumathi27 changed the title [CARBONDATA-3530] Support Create timeseries MV Datamap with the supported granularity levels. [WIP] Support Create timeseries MV Datamap with the supported granularity levels. Nov 6, 2019
@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/761/

@CarbonDataQA
Copy link

Build Failed with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/767/

@CarbonDataQA
Copy link

Build Success with Spark 2.3.2, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/769/

@Indhumathi27
Copy link
Contributor Author

retest this please

@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/762/

@CarbonDataQA
Copy link

Build Success with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/768/

@CarbonDataQA
Copy link

Build Success with Spark 2.3.2, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/770/

@Indhumathi27 Indhumathi27 changed the title [WIP] Support Create timeseries MV Datamap with the supported granularity levels. [CARBONDATA-3530] Support Create timeseries MV Datamap with the supported granularity levels. Nov 7, 2019
@akashrn5 akashrn5 mentioned this pull request Nov 7, 2019
Closed
5 tasks
@jackylk
Copy link
Contributor

jackylk commented Nov 8, 2019

Thanks for working on this.
Can you describe more detail about this PR? Like how to create this MV, what options are supported, and which kind of query can be accelerated, and what internal change is made in this PR, etc.

@Indhumathi27
Copy link
Contributor Author

@jackylk Updated the comment. Please check

@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/812/

@CarbonDataQA
Copy link

Build Success with Spark 2.3.2, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/819/

@CarbonDataQA
Copy link

Build Success with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/816/

@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/824/

@CarbonDataQA
Copy link

Build Success with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/827/

@CarbonDataQA
Copy link

Build Success with Spark 2.3.2, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/830/

@CarbonDataQA
Copy link

Build Success with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/897/

@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/893/

@CarbonDataQA
Copy link

Build Success with Spark 2.3.2, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/898/

@CarbonDataQA
Copy link

Build Failed with Spark 2.3.2, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/901/

@CarbonDataQA
Copy link

Build Failed with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/900/

@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/894/

@CarbonDataQA
Copy link

Build Success with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/901/

@CarbonDataQA
Copy link

Build Failed with Spark 2.3.2, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/902/

@Indhumathi27
Copy link
Contributor Author

retest this please

@Indhumathi27
Copy link
Contributor Author

@ajantha-bhat Handled all the comments. Please review

@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/895/

@Indhumathi27 Indhumathi27 force-pushed the mv_timeseries_create branch 2 times, most recently from fd2f34a to 7b502fb Compare November 16, 2019 13:17
@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/897/

@CarbonDataQA
Copy link

Build Success with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/902/

@CarbonDataQA
Copy link

Build Success with Spark 2.3.2, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/903/

@CarbonDataQA
Copy link

Build Success with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/904/

@CarbonDataQA
Copy link

Build Success with Spark 2.3.2, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/905/

kumarvishal09
kumarvishal09 reviewed Nov 17, 2019
View reviewed changes
core/src/main/java/org/apache/carbondata/core/datamap/DataMapStoreManager.java
DataMapCatalog dataMapCatalog = dataMapCatalogs.get(name);
if (dataMapCatalog == null) {
dataMapCatalog = dataMapProvider.createDataMapCatalog();
if (dataMapCatalog != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

here if dataMapCatlog is still null Line 246 then it will not register the data map schema is it intentional, can please add some comments

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay. Added. Please review

kumarvishal09
kumarvishal09 reviewed Nov 17, 2019
View reviewed changes
datamap/mv/core/src/main/scala/org/apache/carbondata/mv/datamap/MVHelper.scala
@@ -138,6 +145,17 @@ object MVHelper {
}
parentTablesList.add(mainCarbonTable.get)
}

// Check if load is in progress in any of the parent table mapped to the datamap
Copy link
Contributor

Choose a reason for hiding this comment

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

why this validation is added for timeseries?? Or some bug which u have fixed as a part of this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a bug fixed as part of this PR

kumarvishal09
kumarvishal09 reviewed Nov 17, 2019
View reviewed changes
datamap/mv/core/src/main/scala/org/apache/carbondata/mv/datamap/MVHelper.scala Outdated
alias
}
// timeseries column and granularity is not null, then validate
if ((null != timeSeriesColumn).&&(null != granularity)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

please remove the extra braces

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay

@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/906/

@CarbonDataQA
Copy link

Build Success with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/913/

@CarbonDataQA
Copy link

Build Success with Spark 2.3.2, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/914/

@ajantha-bhat
Copy link
Member

LGTM

1 similar comment
@kumarvishal09
Copy link
Contributor

LGTM

@asfgit asfgit closed this in 93c52ed Nov 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ajantha-bhat ajantha-bhat ajantha-bhat left review comments

@kumarvishal09 kumarvishal09 kumarvishal09 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Indhumathi27 @CarbonDataQA @jackylk @ajantha-bhat @kumarvishal09