You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@carbondata.apache.org by akashrn5 <gi...@git.apache.org> on 2017/12/05 13:31:37 UTC
[GitHub] carbondata pull request #1615: [CARBONDATA-1592]added new parameters for cre...
GitHub user akashrn5 opened a pull request:
https://github.com/apache/carbondata/pull/1615
[CARBONDATA-1592]added new parameters for create and load events
added new parameters for create and load events, which will help for some specific validations on table during create and load
Be sure to do all of the following checklist to help us incorporate
your contribution quickly and easily:
- [*] Any interfaces changed?
NA
- [*] Any backward compatibility impacted?
NA
- [*] Document update required?
NA
- [*] 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.
only parameters are added for events, testing is not needed, unless listeners are added
- [*] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/akashrn5/incubator-carbondata param
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/carbondata/pull/1615.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1615
----
commit 7818e1b57054463060f863ebc29da36d374560b4
Author: akashrn5 <ak...@gmail.com>
Date: 2017-12-05T13:22:35Z
added new parameters for create and load events
----
---
[GitHub] carbondata issue #1615: [CARBONDATA-1592]added new parameters for create and...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1615
Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/517/
---
[GitHub] carbondata issue #1615: [CARBONDATA-1592]added new parameters for create and...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/1615
retest this please
---
[GitHub] carbondata issue #1615: [CARBONDATA-1592]added new parameters for create and...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1615
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/1789/
---
[GitHub] carbondata issue #1615: [CARBONDATA-1592]added new parameters for create and...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1615
Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/514/
---
[GitHub] carbondata issue #1615: [CARBONDATA-1592]added new parameters for create and...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1615
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2181/
---
[GitHub] carbondata pull request #1615: [CARBONDATA-1592]added new parameters for cre...
Posted by akashrn5 <gi...@git.apache.org>.
Github user akashrn5 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1615#discussion_r155267087
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/management/CarbonLoadDataCommand.scala ---
@@ -148,7 +148,9 @@ case class CarbonLoadDataCommand(
carbonLoadModel,
factPath,
dataFrame.isDefined,
- optionsFinal)
+ optionsFinal,
+ options,
--- End diff --
from finalOptions we will get all the default values of that particular option, but if we need only the options given from user,we need options also, so this one is added
---
[GitHub] carbondata pull request #1615: [CARBONDATA-1592]added new parameters for cre...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1615#discussion_r155252245
--- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/events/CreateTableEvents.scala ---
@@ -20,14 +20,16 @@ package org.apache.carbondata.events
import org.apache.spark.sql._
import org.apache.carbondata.core.metadata.AbsoluteTableIdentifier
+import org.apache.carbondata.core.metadata.schema.table.TableInfo
/**
* Class for handling operations before start of a load process.
* Example usage: For validation purpose
*/
case class CreateTablePreExecutionEvent(
sparkSession: SparkSession,
- identifier: AbsoluteTableIdentifier) extends Event with TableEventInfo
+ identifier: AbsoluteTableIdentifier,
+ tableModel: Option[TableInfo]) extends Event with TableEventInfo
--- End diff --
rename the parameter to tableInfo
---
[GitHub] carbondata pull request #1615: [CARBONDATA-1592]added new parameters for cre...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/carbondata/pull/1615
---
[GitHub] carbondata pull request #1615: [CARBONDATA-1592]added new parameters for cre...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1615#discussion_r154960501
--- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/events/CreateTableEvents.scala ---
@@ -27,7 +28,8 @@ import org.apache.carbondata.core.metadata.AbsoluteTableIdentifier
*/
case class CreateTablePreExecutionEvent(
sparkSession: SparkSession,
- identifier: AbsoluteTableIdentifier) extends Event with TableEventInfo
+ identifier: AbsoluteTableIdentifier,
+ tableModel: Option[TableModel]) extends Event with TableEventInfo
--- End diff --
TableModel is a temporary data structure that used in spark integration only, while TableInfo represent the carbon table entity, so we should use TableInfo instead of TableModel in any framework and interface code
---
[GitHub] carbondata issue #1615: [CARBONDATA-1592]added new parameters for create and...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1615
Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/468/
---
[GitHub] carbondata issue #1615: [CARBONDATA-1592]added new parameters for create and...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1615
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/1782/
---
[GitHub] carbondata issue #1615: [CARBONDATA-1592]added new parameters for create and...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1615
Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/551/
---
[GitHub] carbondata issue #1615: [CARBONDATA-1592]added new parameters for create and...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1615
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/1808/
---
[GitHub] carbondata issue #1615: [CARBONDATA-1592]added new parameters for create and...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/1615
LGTM
---
[GitHub] carbondata issue #1615: [CARBONDATA-1592]added new parameters for create and...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1615
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2152/
---
[GitHub] carbondata issue #1615: [CARBONDATA-1592]added new parameters for create and...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1615
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/1736/
---
[GitHub] carbondata issue #1615: [CARBONDATA-1592]added new parameters for create and...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1615
Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/558/
---
[GitHub] carbondata issue #1615: [CARBONDATA-1592]added new parameters for create and...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1615
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2111/
---
[GitHub] carbondata issue #1615: [CARBONDATA-1592]added new parameters for create and...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1615
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2116/
---
[GitHub] carbondata pull request #1615: [CARBONDATA-1592]added new parameters for cre...
Posted by KanakaKumar <gi...@git.apache.org>.
Github user KanakaKumar commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1615#discussion_r155310298
--- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/events/LoadEvents.scala ---
@@ -31,8 +31,10 @@ case class LoadTablePreExecutionEvent(sparkSession: SparkSession,
carbonLoadModel: CarbonLoadModel,
factPath: String,
isDataFrameDefined: Boolean,
- optionsFinal: scala.collection
- .mutable.Map[String, String]) extends Event with LoadEventInfo
+ optionsFinal: scala.collection.mutable.Map[String, String],
+ // options are needed if we need only the load options given by user
+ options: Map[String, String],
--- End diff --
Please rename the options as userProvidedOptions
---
[GitHub] carbondata pull request #1615: [CARBONDATA-1592]added new parameters for cre...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1615#discussion_r154976392
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/management/CarbonLoadDataCommand.scala ---
@@ -148,7 +148,9 @@ case class CarbonLoadDataCommand(
carbonLoadModel,
factPath,
dataFrame.isDefined,
- optionsFinal)
+ optionsFinal,
+ options,
--- End diff --
Why passing to options, can you merge them into one?
---
[GitHub] carbondata issue #1615: [CARBONDATA-1592]added new parameters for create and...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1615
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/1727/
---
[GitHub] carbondata pull request #1615: [CARBONDATA-1592]added new parameters for cre...
Posted by akashrn5 <gi...@git.apache.org>.
Github user akashrn5 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1615#discussion_r155253089
--- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/events/CreateTableEvents.scala ---
@@ -20,14 +20,16 @@ package org.apache.carbondata.events
import org.apache.spark.sql._
import org.apache.carbondata.core.metadata.AbsoluteTableIdentifier
+import org.apache.carbondata.core.metadata.schema.table.TableInfo
/**
* Class for handling operations before start of a load process.
* Example usage: For validation purpose
*/
case class CreateTablePreExecutionEvent(
sparkSession: SparkSession,
- identifier: AbsoluteTableIdentifier) extends Event with TableEventInfo
+ identifier: AbsoluteTableIdentifier,
+ tableModel: Option[TableInfo]) extends Event with TableEventInfo
--- End diff --
done, please check
---
[GitHub] carbondata issue #1615: [CARBONDATA-1592]added new parameters for create and...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/1615
LGTM
---
[GitHub] carbondata issue #1615: [CARBONDATA-1592]added new parameters for create and...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1615
Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/519/
---
[GitHub] carbondata pull request #1615: [CARBONDATA-1592]added new parameters for cre...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1615#discussion_r154976204
--- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/events/LoadEvents.scala ---
@@ -32,7 +32,9 @@ case class LoadTablePreExecutionEvent(sparkSession: SparkSession,
factPath: String,
isDataFrameDefined: Boolean,
optionsFinal: scala.collection
- .mutable.Map[String, String]) extends Event with LoadEventInfo
+ .mutable.Map[String, String],
--- End diff --
this belong to previous line
---
[GitHub] carbondata issue #1615: [CARBONDATA-1592]added new parameters for create and...
Posted by akashrn5 <gi...@git.apache.org>.
Github user akashrn5 commented on the issue:
https://github.com/apache/carbondata/pull/1615
handled comment, plesae review @jackylk
---
[GitHub] carbondata issue #1615: [CARBONDATA-1592]added new parameters for create and...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1615
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/1771/
---
[GitHub] carbondata issue #1615: [CARBONDATA-1592]added new parameters for create and...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1615
Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/462/
---