[GitHub] spark pull request: [SPARK-14441] [SQL] Consolidate DDL tests

2016-04-25 Thread bomeng
Github user bomeng closed the pull request at:

https://github.com/apache/spark/pull/12347


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-14441] [SQL] Consolidate DDL tests

2016-04-25 Thread bomeng
Github user bomeng commented on the pull request:

https://github.com/apache/spark/pull/12347#issuecomment-214518666
  
closing this PR. thanks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-14441] [SQL] Consolidate DDL tests

2016-04-23 Thread rxin
Github user rxin commented on the pull request:

https://github.com/apache/spark/pull/12347#issuecomment-213683729
  
Yea the current tests are pretty scattered and they are way too big to 
merge into a single one. Can you just close this pull request? Thanks.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-14441] [SQL] Consolidate DDL tests

2016-04-13 Thread bomeng
Github user bomeng commented on the pull request:

https://github.com/apache/spark/pull/12347#issuecomment-209622770
  
Ok, not a problem. Thanks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-14441] [SQL] Consolidate DDL tests

2016-04-13 Thread andrewor14
Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/12347#issuecomment-209600458
  
@gatorsmile that's a good point. I think a better goal of the issue is to 
make the tests consistent and make sure the functionality of the test suites 
are clearly isolated. I don't think just literally merging all the test files 
together is a great idea.

@bomeng also we should wait for all the other DDLs tasks have finished 
before we start working on this one. Otherwise there will be a lot of conflicts 
and the later PRs will still put them in the wrong places.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-14441] [SQL] Consolidate DDL tests

2016-04-12 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/12347#issuecomment-209217404
  
Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-14441] [SQL] Consolidate DDL tests

2016-04-12 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/12347#issuecomment-209217405
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55680/
Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-14441] [SQL] Consolidate DDL tests

2016-04-12 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/12347#issuecomment-209217278
  
**[Test build #55680 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55680/consoleFull)**
 for PR 12347 at commit 
[`06c8948`](https://github.com/apache/spark/commit/06c8948b20cacccead019e42e3a07b89f727893a).
 * This patch passes all tests.
 * This patch merges cleanly.
 * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-14441] [SQL] Consolidate DDL tests

2016-04-12 Thread gatorsmile
Github user gatorsmile commented on the pull request:

https://github.com/apache/spark/pull/12347#issuecomment-209200845
  
To be honest, I do not know why we need to merge these test case files. 
Their purposes are different. One is to verify the functionalities of parsers; 
another is to verify the execution of DDL/commands. In the future, we might add 
more test cases to both files. The files will grow bigger and bigger.  

cc @yhuai @andrewor14 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-14441] [SQL] Consolidate DDL tests

2016-04-12 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/12347#issuecomment-209200120
  
**[Test build #55680 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55680/consoleFull)**
 for PR 12347 at commit 
[`06c8948`](https://github.com/apache/spark/commit/06c8948b20cacccead019e42e3a07b89f727893a).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-14441] [SQL] Consolidate DDL tests

2016-04-12 Thread gatorsmile
Github user gatorsmile commented on the pull request:

https://github.com/apache/spark/pull/12347#issuecomment-209200210
  
```Use .contains(...) instead == Some(...) for Options, this method is 
introduced in Scala 2.11 and it is recommended method to use for this 
purpose;```
This should not be used in Spark code. It will break the 2.10 build. 

See my PR: https://github.com/apache/spark/pull/12201


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-14441] [SQL] Consolidate DDL tests

2016-04-12 Thread gatorsmile
Github user gatorsmile commented on a diff in the pull request:

https://github.com/apache/spark/pull/12347#discussion_r59485621
  
--- Diff: 
sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveDDLCommandSuite.scala ---
@@ -113,10 +244,10 @@ class HiveDDLCommandSuite extends PlanTest {
 
 val (desc, exists) = extractTableDesc(s2)
 assert(exists)
-assert(desc.identifier.database == Some("mydb"))
+assert(desc.identifier.database.contains("mydb"))
--- End diff --

All these similar changes will break the Scala 2.10 build


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-14441] [SQL] Consolidate DDL tests

2016-04-12 Thread bomeng
GitHub user bomeng opened a pull request:

https://github.com/apache/spark/pull/12347

[SPARK-14441] [SQL] Consolidate DDL tests

## What changes were proposed in this pull request?

Today we have `DDLSuite`, `DDLCommandSuite`, `HiveDDLCommandSuite` and 
`HiveDDLSuite`. In this PR, I am trying to consolidate the files as much as 
possible. Two files are left for now, since it is good to put Hive related test 
suite in the Hive package. 

Along with the combination, I also did some modification of current codes 
mainly in order to improve the codings, here is the summary:
1. Use `.contains()` instead `==` for Options, this method is introduced in 
Scala 2.11 and it is recommended method to use for this purpose;
2. Make map consistent to use `->`, instead of 2 elements tuple;
3. Use `isEmpty()` instead of `== None`
4. Add `private` to the parser and narrow the scope of implicit (put inside 
one of the tests); 
5. Modify the names of some tests to be unique, since they are in one file 
now;

## How was this patch tested?

I did not change the logic of any tests, just move around and improve the 
codes, so existing test cases should remain same.

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

$ git pull https://github.com/bomeng/spark SPARK-14441

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

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


commit af298b63693586e7c0598ccd98ba3f9e6a7634f7
Author: bomeng 
Date:   2016-04-13T02:19:27Z

Consolidate DDL tests

commit 06c8948b20cacccead019e42e3a07b89f727893a
Author: bomeng 
Date:   2016-04-13T02:21:52Z

remove extra line




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org