[GitHub] spark issue #14077: [SPARK-16402] [SQL] JDBC Source: Implement save API of D...

[gatorsmile]

Github user gatorsmile commented on the issue:

https://github.com/apache/spark/pull/14077
  
Sure, let me close it now


---
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 issue #14077: [SPARK-16402] [SQL] JDBC Source: Implement save API of D...

[HyukjinKwon]

Github user HyukjinKwon commented on the issue:

https://github.com/apache/spark/pull/14077
  
@gatorsmile Just a reminder that we might be able to close this.


---
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 issue #14077: [SPARK-16402] [SQL] JDBC Source: Implement save API of D...

[AmplabJenkins]

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/14077
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/64806/
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 issue #14077: [SPARK-16402] [SQL] JDBC Source: Implement save API of D...

[AmplabJenkins]

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/14077
  
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 issue #14077: [SPARK-16402] [SQL] JDBC Source: Implement save API of D...

[SparkQA]

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/14077
  
**[Test build #64806 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/64806/consoleFull)**
 for PR 14077 at commit 
[`07e3168`](https://github.com/apache/spark/commit/07e316823ed17e89c3df0aaccf3fbb958afcfe3a).
 * 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 issue #14077: [SPARK-16402] [SQL] JDBC Source: Implement save API of D...

[AmplabJenkins]

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/14077
  
Merged build finished. Test FAILed.


---
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 issue #14077: [SPARK-16402] [SQL] JDBC Source: Implement save API of D...

[AmplabJenkins]

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/14077
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/64805/
Test FAILed.


---
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 issue #14077: [SPARK-16402] [SQL] JDBC Source: Implement save API of D...

[SparkQA]

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/14077
  
**[Test build #64805 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/64805/consoleFull)**
 for PR 14077 at commit 
[`2e799ce`](https://github.com/apache/spark/commit/2e799ce86652bc5c03d21fdbf0a11fab20b37c39).
 * This patch **fails Spark unit 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 issue #14077: [SPARK-16402] [SQL] JDBC Source: Implement save API of D...

[SparkQA]

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/14077
  
**[Test build #64806 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/64806/consoleFull)**
 for PR 14077 at commit 
[`07e3168`](https://github.com/apache/spark/commit/07e316823ed17e89c3df0aaccf3fbb958afcfe3a).


---
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 issue #14077: [SPARK-16402] [SQL] JDBC Source: Implement save API of D...

[SparkQA]

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/14077
  
**[Test build #64805 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/64805/consoleFull)**
 for PR 14077 at commit 
[`2e799ce`](https://github.com/apache/spark/commit/2e799ce86652bc5c03d21fdbf0a11fab20b37c39).


---
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 issue #14077: [SPARK-16402] [SQL] JDBC Source: Implement save API of D...

[JustinPihony]

Github user JustinPihony commented on the issue:

https://github.com/apache/spark/pull/14077
  
@gatorsmile As I said above, I actually think it might be better to keep 
the work that was already done and am waiting for Reynold's feedback.


---
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 issue #14077: [SPARK-16402] [SQL] JDBC Source: Implement save API of D...

[gatorsmile]

Github user gatorsmile commented on the issue:

https://github.com/apache/spark/pull/14077
  
@JustinPihony How about you first moving the `copy` function in your PR 
now? Then, we can review your PR before the SPARK-16401 is resolved. 


---
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 issue #14077: [SPARK-16402] [SQL] JDBC Source: Implement save API of D...

[JustinPihony]

Github user JustinPihony commented on the issue:

https://github.com/apache/spark/pull/14077
  
Thanks. I will have to wait until SPARK-16401 is resolved or else the code 
will not pass tests, though.


---
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 issue #14077: [SPARK-16402] [SQL] JDBC Source: Implement save API of D...

[gatorsmile]

Github user gatorsmile commented on the issue:

https://github.com/apache/spark/pull/14077
  
@JustinPihony You know, I do not care which PR is merged eventually. You 
can try to clean your PR at your best. I will review your PR when it is ready. 
Thanks for your work! Please continue to submit more PRs for improving Spark. 

To reduce the code changes in your PR, I think we should not extend 
`SchemaRelationProvider`. Now, I think you can assume the copy location has 
been fixed.  

Since this is related to Data Source APIs, CC @rxin @yhuai 


---
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 issue #14077: [SPARK-16402] [SQL] JDBC Source: Implement save API of D...

[JustinPihony]

Github user JustinPihony commented on the issue:

https://github.com/apache/spark/pull/14077
  
Then the best course of action would be to use my current impl as it works 
no matter the position of copy. I can add the additional tests if that would 
make it more amenable?  Otherwise I'll push a reduced code set in the morning, 
but it would rely on the copy location move PR

> On Jul 7, 2016, at 1:27 AM, Hyukjin Kwon  wrote:
> 
> (Personally, I hope this does not get delayed because this usage was 
introduced in Spark Summit PPT and I guess users would try to use this API.)
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.
> 



---
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 issue #14077: [SPARK-16402] [SQL] JDBC Source: Implement save API of D...

[HyukjinKwon]

Github user HyukjinKwon commented on the issue:

https://github.com/apache/spark/pull/14077
  
(Personally, I hope this does not get delayed because this usage was 
introduced in Spark Summit PPT and I guess users would try to use this API.)


---
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 issue #14077: [SPARK-16402] [SQL] JDBC Source: Implement save API of D...

[gatorsmile]

Github user gatorsmile commented on the issue:

https://github.com/apache/spark/pull/14077
  
Thank you for confirming that it is a bug in another PR. 

Regarding the solution of this PR, it is not a true circular reference. The 
solution in this PR is to minimize the duplicate codes. I also think it make 
senses to move the common code logics from `jdbc` API to `createRelation` 
implementation of `CreatableRelationProvider`. The JDBC-specific logics should 
not be exposed to the `DataFrameWriter` APIs. 

If you wants to do it in your PR, I am also fine. Please minimize the code 
changes and add the test cases introduced in 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 issue #14077: [SPARK-16402] [SQL] JDBC Source: Implement save API of D...

[JustinPihony]

Github user JustinPihony commented on the issue:

https://github.com/apache/spark/pull/14077
  
@gatorsmile If `copy` is a bug, then that is fine with me (I just commented 
my findings on this and will be curious to hear back). That said, it would make 
my implementation simpler. I'd be fine with simplifying it down to a basic 
save, however I am still not OK with the circular reference. It adds confusion 
to debugging and future refactorings. And to fix that, you end up back at my 
PR. So, ultimately this still seems like a duplicate to me.


---
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 issue #14077: [SPARK-16402] [SQL] JDBC Source: Implement save API of D...

[gatorsmile]

Github user gatorsmile commented on the issue:

https://github.com/apache/spark/pull/14077
  
@JustinPihony Thanks for your review! Let me try to answer your concerns. 
- The `copy` function location is actually a bug. See another PR: 
https://github.com/apache/spark/pull/14075. 
- The trait `CreatableRelationProvider` was introduced for the `save` API. 


---
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 issue #14077: [SPARK-16402] [SQL] JDBC Source: Implement save API of D...

[JustinPihony]

Github user JustinPihony commented on the issue:

https://github.com/apache/spark/pull/14077
  
This may seem simpler, but that's because it seems to be taking some 
shortcuts to avoid having to refactor. This currently creates a cycle along the 
lines of `df.save.df.jdbc`. Wouldn't it be better to fix the code than to work 
around it? Additionally, is moving the `copy` appropriate? Maybe it was put on 
the outside errantly and it is correct, but I'm not sure it can be moved 
without researching it properly.


---
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