[GitHub] carbondata pull request #1601: [CARBONDATA-1787] Validation for table proper...

2017-12-13 Thread geetikagupta16 
Github user geetikagupta16 closed the pull request at:

https://github.com/apache/carbondata/pull/1601


---

[GitHub] carbondata pull request #1601: [CARBONDATA-1787] Validation for table proper...

2017-12-08 Thread geetikagupta16 
Github user geetikagupta16 commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/1601#discussion_r155749994
  
--- Diff: 
integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParser.scala
 ---
@@ -190,7 +191,7 @@ class CarbonHelperSqlAstBuilder(conf: SQLConf, parser: 
CarbonSpark2SqlParser)
 }
 
 val tableProperties = mutable.Map[String, String]()
-properties.foreach{property => tableProperties.put(property._1, 
property._2)}
+validatedProperties.foreach{property => 
tableProperties.put(property._1, property._2)}
--- End diff --

Can't remove the Mutable Map as it is being updated in prepareTableModel 
method


---

[GitHub] carbondata pull request #1601: [CARBONDATA-1787] Validation for table proper...

2017-12-08 Thread geetikagupta16 
Github user geetikagupta16 commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/1601#discussion_r155735141
  
--- Diff: 
integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParser.scala
 ---
@@ -190,7 +191,7 @@ class CarbonHelperSqlAstBuilder(conf: SQLConf, parser: 
CarbonSpark2SqlParser)
 }
 
 val tableProperties = mutable.Map[String, String]()
-properties.foreach{property => tableProperties.put(property._1, 
property._2)}
+validatedProperties.foreach{property => 
tableProperties.put(property._1, property._2)}
--- End diff --

Removing the mutable Map will lead to changes in the parameters of other 
methods. Should I work on that?


---

[GitHub] carbondata pull request #1601: [CARBONDATA-1787] Validation for table proper...

2017-12-07 Thread kunal642 
Github user kunal642 commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/1601#discussion_r155719835
  
--- Diff: 
integration/spark-common-cluster-test/src/test/scala/org/apache/carbondata/cluster/sdv/generated/V3offheapvectorTestCase.scala
 ---
@@ -35,7 +35,7 @@ class V3offheapvectorTestCase extends QueryTest with 
BeforeAndAfterAll {
   //Check query reponse for select * query with no filters
   test("V3_01_Query_01_033", Include) {
  dropTable("3lakh_uniqdata")
- sql(s"""CREATE TABLE 3lakh_uniqdata (CUST_ID int,CUST_NAME 
String,ACTIVE_EMUI_VERSION string, DOB timestamp, DOJ timestamp, BIGINT_COLUMN1 
bigint,BIGINT_COLUMN2 bigint,DECIMAL_COLUMN1 decimal(30,10), DECIMAL_COLUMN2 
decimal(36,10),Double_COLUMN1 double, Double_COLUMN2 double,INTEGER_COLUMN1 
int) STORED BY 'carbondata' 
TBLPROPERTIES('table_blocksize'='128','include_dictionary'='BIGINT_COLUMN1,BIGINT_COLUMN2,DECIMAL_COLUMN1,DECIMAL_COLUMN2,Double_COLUMN1,Double_COLUMN2,INTEGER_COLUMN1,CUST_ID')""").collect
+ sql(s"""CREATE TABLE 3lakh_uniqdata (CUST_ID int,CUST_NAME 
String,ACTIVE_EMUI_VERSION string, DOB timestamp, DOJ timestamp, BIGINT_COLUMN1 
bigint,BIGINT_COLUMN2 bigint,DECIMAL_COLUMN1 decimal(30,10), DECIMAL_COLUMN2 
decimal(36,10),Double_COLUMN1 double, Double_COLUMN2 double,INTEGER_COLUMN1 
int) STORED BY 'carbondata' 
TBLPROPERTIES('table_blocksize'='128','dictionary_include'='BIGINT_COLUMN1,BIGINT_COLUMN2,DECIMAL_COLUMN1,DECIMAL_COLUMN2,Double_COLUMN1,Double_COLUMN2,INTEGER_COLUMN1,CUST_ID')""").collect
--- End diff --

okay


---

[GitHub] carbondata pull request #1601: [CARBONDATA-1787] Validation for table proper...

2017-12-07 Thread geetikagupta16 
Github user geetikagupta16 commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/1601#discussion_r155718131
  
--- Diff: 
integration/spark-common-cluster-test/src/test/scala/org/apache/carbondata/cluster/sdv/generated/V3offheapvectorTestCase.scala
 ---
@@ -35,7 +35,7 @@ class V3offheapvectorTestCase extends QueryTest with 
BeforeAndAfterAll {
   //Check query reponse for select * query with no filters
   test("V3_01_Query_01_033", Include) {
  dropTable("3lakh_uniqdata")
- sql(s"""CREATE TABLE 3lakh_uniqdata (CUST_ID int,CUST_NAME 
String,ACTIVE_EMUI_VERSION string, DOB timestamp, DOJ timestamp, BIGINT_COLUMN1 
bigint,BIGINT_COLUMN2 bigint,DECIMAL_COLUMN1 decimal(30,10), DECIMAL_COLUMN2 
decimal(36,10),Double_COLUMN1 double, Double_COLUMN2 double,INTEGER_COLUMN1 
int) STORED BY 'carbondata' 
TBLPROPERTIES('table_blocksize'='128','include_dictionary'='BIGINT_COLUMN1,BIGINT_COLUMN2,DECIMAL_COLUMN1,DECIMAL_COLUMN2,Double_COLUMN1,Double_COLUMN2,INTEGER_COLUMN1,CUST_ID')""").collect
+ sql(s"""CREATE TABLE 3lakh_uniqdata (CUST_ID int,CUST_NAME 
String,ACTIVE_EMUI_VERSION string, DOB timestamp, DOJ timestamp, BIGINT_COLUMN1 
bigint,BIGINT_COLUMN2 bigint,DECIMAL_COLUMN1 decimal(30,10), DECIMAL_COLUMN2 
decimal(36,10),Double_COLUMN1 double, Double_COLUMN2 double,INTEGER_COLUMN1 
int) STORED BY 'carbondata' 
TBLPROPERTIES('table_blocksize'='128','dictionary_include'='BIGINT_COLUMN1,BIGINT_COLUMN2,DECIMAL_COLUMN1,DECIMAL_COLUMN2,Double_COLUMN1,Double_COLUMN2,INTEGER_COLUMN1,CUST_ID')""").collect
--- End diff --

In this I have changed the TBLPROPERTIES option dictionary_include


---

[GitHub] carbondata pull request #1601: [CARBONDATA-1787] Validation for table proper...

2017-12-07 Thread kunal642 
Github user kunal642 commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/1601#discussion_r155713651
  
--- Diff: 
integration/spark-common-cluster-test/src/test/scala/org/apache/carbondata/cluster/sdv/generated/V3offheapvectorTestCase.scala
 ---
@@ -35,7 +35,7 @@ class V3offheapvectorTestCase extends QueryTest with 
BeforeAndAfterAll {
   //Check query reponse for select * query with no filters
   test("V3_01_Query_01_033", Include) {
  dropTable("3lakh_uniqdata")
- sql(s"""CREATE TABLE 3lakh_uniqdata (CUST_ID int,CUST_NAME 
String,ACTIVE_EMUI_VERSION string, DOB timestamp, DOJ timestamp, BIGINT_COLUMN1 
bigint,BIGINT_COLUMN2 bigint,DECIMAL_COLUMN1 decimal(30,10), DECIMAL_COLUMN2 
decimal(36,10),Double_COLUMN1 double, Double_COLUMN2 double,INTEGER_COLUMN1 
int) STORED BY 'carbondata' 
TBLPROPERTIES('table_blocksize'='128','include_dictionary'='BIGINT_COLUMN1,BIGINT_COLUMN2,DECIMAL_COLUMN1,DECIMAL_COLUMN2,Double_COLUMN1,Double_COLUMN2,INTEGER_COLUMN1,CUST_ID')""").collect
+ sql(s"""CREATE TABLE 3lakh_uniqdata (CUST_ID int,CUST_NAME 
String,ACTIVE_EMUI_VERSION string, DOB timestamp, DOJ timestamp, BIGINT_COLUMN1 
bigint,BIGINT_COLUMN2 bigint,DECIMAL_COLUMN1 decimal(30,10), DECIMAL_COLUMN2 
decimal(36,10),Double_COLUMN1 double, Double_COLUMN2 double,INTEGER_COLUMN1 
int) STORED BY 'carbondata' 
TBLPROPERTIES('table_blocksize'='128','dictionary_include'='BIGINT_COLUMN1,BIGINT_COLUMN2,DECIMAL_COLUMN1,DECIMAL_COLUMN2,Double_COLUMN1,Double_COLUMN2,INTEGER_COLUMN1,CUST_ID')""").collect
--- End diff --

revert the unnecessary changes


---

[GitHub] carbondata pull request #1601: [CARBONDATA-1787] Validation for table proper...

2017-12-07 Thread kunal642 
Github user kunal642 commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/1601#discussion_r155713494
  
--- Diff: 
integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParser.scala
 ---
@@ -190,7 +191,7 @@ class CarbonHelperSqlAstBuilder(conf: SQLConf, parser: 
CarbonSpark2SqlParser)
 }
 
 val tableProperties = mutable.Map[String, String]()
-properties.foreach{property => tableProperties.put(property._1, 
property._2)}
+validatedProperties.foreach{property => 
tableProperties.put(property._1, property._2)}
--- End diff --

i think we can remove this code. After this we will extract the values so 
no point in creating mutable.Map. Can you please confirm this?


---

[GitHub] carbondata pull request #1601: [CARBONDATA-1787] Validation for table proper...

2017-12-07 Thread kunal642 
Github user kunal642 commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/1601#discussion_r155712770
  
--- Diff: 
integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParser.scala
 ---
@@ -232,6 +233,30 @@ class CarbonHelperSqlAstBuilder(conf: SQLConf, parser: 
CarbonSpark2SqlParser)
 CarbonCreateTableCommand(tableModel, tablePath)
   }
 
+  private def validateTableProperties(properties: Map[String, String]): 
Map[String, String] = {
+var isSupported = true
+val invalidOptions = StringBuilder.newBuilder
+val tableProperties = Seq("DICTIONARY_INCLUDE", "DICTIONARY_EXCLUDE", 
"NO_INVERTED_INDEX",
+  "SORT_COLUMNS", "TABLE_BLOCKSIZE", "STREAMING", "SORT_SCOPE", 
"COMMENT", "PARTITION_TYPE",
+  "NUM_PARTITIONS", "RANGE_INFO", "LIST_INFO", "BUCKETNUMBER", 
"BUCKETCOLUMNS", "TABLENAME")
+val tblProperties: Map[String, String] = properties.filter { property 
=>
+  if (!(tableProperties.exists(prop => 
prop.equalsIgnoreCase(property._1))
--- End diff --

we can just iterate over the map and check if any of the properties are 
invalid. No need to create new tblProperties.
Instead of filter, use collect to get invalid properties and check if any 
property is returned.


---

[GitHub] carbondata pull request #1601: [CARBONDATA-1787] Validation for table proper...

2017-12-01 Thread geetikagupta16 
GitHub user geetikagupta16 opened a pull request:

https://github.com/apache/carbondata/pull/1601

[CARBONDATA-1787] Validation for table properties in Create Table Command

Be sure to do all of the following checklist to help us incorporate 
your contribution quickly and easily:

 - [ ] Any interfaces changed? No
 
 - [ ] Any backward compatibility impacted? No
 
 - [ ] Document update required? No

 - [ ] 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. 



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/geetikagupta16/incubator-carbondata 
CARBONDATA-1787

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/carbondata/pull/1601.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 #1601


commit 27847267bae59611d4db63183a5ee39ba0ac7e89
Author: Geetika Gupta 
Date:   2017-12-01T08:19:32Z

1. Added validation for table properties
2. Added related test cases




---