Github user srowen commented on the issue:
https://github.com/apache/spark/pull/12601
Merged to master
---
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
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/12601
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/65891/
Test PASSed.
---
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/12601
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
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/12601
**[Test build #65891 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65891/consoleFull)**
for PR 12601 at commit
Github user JustinPihony commented on the issue:
https://github.com/apache/spark/pull/12601
@srowen The doc changes have been reviewed, so this should be good to go
---
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
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/12601
**[Test build #65891 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65891/consoleFull)**
for PR 12601 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/12601
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/65860/
Test PASSed.
---
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/12601
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
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/12601
**[Test build #65860 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65860/consoleFull)**
for PR 12601 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/12601
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
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/12601
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/65858/
Test PASSed.
---
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/12601
Mostly LGTM, except three minor comments.
Thank you for your hard work, @JustinPihony !
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/12601
**[Test build #65860 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65860/consoleFull)**
for PR 12601 at commit
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/12601
Thanks for mentioning me. It looks good to me in my personal view.
---
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
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/12601
**[Test build #65858 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65858/consoleFull)**
for PR 12601 at commit
Github user JustinPihony commented on the issue:
https://github.com/apache/spark/pull/12601
@gatorsmile I added the R and SQL documentation. I took the SQL portion
from https://github.com/apache/spark/pull/6121/files
---
If your project is set up for it, you can reply to this email
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/12601
Not sure you already knew it. Just want to share the commands how to build
the doc.
```Scala
SKIP_API=1 jekyll build
SKIP_API=1 jekyll serve
```
After the second
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/12601
@JustinPihony The document changes in Scala, JAVA and Python look good to
me, but could you please also add the examples for both SQL and R?
- The R API example for `write.jdbc` can be
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/12601
Sure, will build the document in my local computer and review it soon.
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
Github user JustinPihony commented on the issue:
https://github.com/apache/spark/pull/12601
@HyukjinKwon @gatorsmile Could you please review the documentation that I
added so that we can put this to bed :)
---
If your project is set up for it, you can reply to this email and have
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/12601
OK, this isn't really my area. It looks reasonable to me. @gatorsmile ?
I think https://github.com/apache/spark/pull/12601#issuecomment-246165894
was suggesting that some short notes should be
Github user JustinPihony commented on the issue:
https://github.com/apache/spark/pull/12601
@srowen Ping. I don't think there is anything on my plate. This should be
mergeable
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
Github user JustinPihony commented on the issue:
https://github.com/apache/spark/pull/12601
@srowen I had to fix something on my local machine to get proper test
results, but this should be good to go now.
---
If your project is set up for it, you can reply to this email and have
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/12601
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/65460/
Test PASSed.
---
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/12601
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
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/12601
**[Test build #65460 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65460/consoleFull)**
for PR 12601 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/12601
**[Test build #65460 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65460/consoleFull)**
for PR 12601 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/12601
**[Test build #65455 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65455/consoleFull)**
for PR 12601 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/12601
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
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/12601
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/65455/
Test FAILed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/12601
**[Test build #65455 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65455/consoleFull)**
for PR 12601 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/12601
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
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/12601
**[Test build #65454 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65454/consoleFull)**
for PR 12601 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/12601
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/65454/
Test FAILed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/12601
**[Test build #65454 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65454/consoleFull)**
for PR 12601 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/12601
**[Test build #65452 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65452/consoleFull)**
for PR 12601 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/12601
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/65452/
Test FAILed.
---
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/12601
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
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/12601
**[Test build #65452 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65452/consoleFull)**
for PR 12601 at commit
Github user JustinPihony commented on the issue:
https://github.com/apache/spark/pull/12601
@srowen [This documentation is pulled directly from the code
examples](https://github.com/apache/spark/blame/master/docs/sql-programming-guide.md#L1080).
That said, in double checking this I
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/12601
I don't see that any docs were added? I mean in the user facing
documentation under docs/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
Github user JustinPihony commented on the issue:
https://github.com/apache/spark/pull/12601
@HyukjinKwon @gatorsmile @srowen Can one of you review my documentation
change so we can get this pushed out :)?
---
If your project is set up for it, you can reply to this email and have
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/12601
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/65275/
Test PASSed.
---
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/12601
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
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/12601
**[Test build #65275 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65275/consoleFull)**
for PR 12601 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/12601
**[Test build #65275 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65275/consoleFull)**
for PR 12601 at commit
Github user JustinPihony commented on the issue:
https://github.com/apache/spark/pull/12601
@srowen Documentation added.
---
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
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/12601
Yeah! In the
[document](http://spark.apache.org/docs/latest/sql-programming-guide.html#jdbc-to-other-databases),
we did not document the write path for JDBC. Now, it is a good chance to do it
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/12601
@JustinPihony et al, this looks OK to me too given the reviews, but do we
need a doc update too? now `write("jdbc")` works where it didn't before?
---
If your project is set up for it, you can
Github user JustinPihony commented on the issue:
https://github.com/apache/spark/pull/12601
Can this be merged 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
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/12601
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
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/12601
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/65087/
Test PASSed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/12601
**[Test build #65087 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65087/consoleFull)**
for PR 12601 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/12601
**[Test build #65087 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65087/consoleFull)**
for PR 12601 at commit
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/12601
ok to test
---
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
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/12601
@srowen @cloud-fan @rxin Could you trigger the test? The current version
looks good to me. We need to check whether all the test cases can pass.
Thanks for your work, @JustinPihony !
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/12601
+1 for triggering the test.
---
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
Github user JustinPihony commented on the issue:
https://github.com/apache/spark/pull/12601
This seems good now. Could I please get this to at least run through the
automatic tests so we can push this through finally?
---
If your project is set up for it, you can reply to this
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/12601
Thank you for adding more test cases! It looks many duplicate codes. Could
you just write a common function and each test case just call this function,
instead of copying all the codes?
---
Github user JustinPihony commented on the issue:
https://github.com/apache/spark/pull/12601
@gatorsmile wrt tests, I had actually had these but stashed them to make a
merge easier and forgot to re-add them. I have since pushed them :)
---
If your project is set up for it, you can
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/12601
@JustinPihony Based on my understanding, we need to minimize the code
changes at our best. Any extra code changes might introduce unnecessary bugs.
At the same time, we need to improve
Github user JustinPihony commented on the issue:
https://github.com/apache/spark/pull/12601
@HyukjinKwon You are correct that it is a different issue, so I went ahead
and removed the code, which actually showed that it was closer to ~5 lines of
code... So can we please move past that
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/12601
IMHO, it'd be another issue to support user-given schema. As the original
issue is to support `write.format(..).save()`, it'd be different one.
Also, I think it is arguable to support
Github user JustinPihony commented on the issue:
https://github.com/apache/spark/pull/12601
@gatorsmile Again, because the work is already done and it allows a broader
flexibility for the user. It is only ~20 lines of simple code. What is the
reason to not have it?
---
If your
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/12601
It sounds like you still keep `SchemaRelationProvider`? What is the benefit?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If
Github user JustinPihony commented on the issue:
https://github.com/apache/spark/pull/12601
@srowen I have addressed your concerns and also merged the branch to handle
the past few changes to jdbc. I think this should be good to go now.
---
If your project is set up for it, you can
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/12601
It sounds like you understand my points. Then, please update your PR based
on my latest changes and we can review your PR. Thanks!
---
If your project is set up for it, you can reply to this
Github user JustinPihony commented on the issue:
https://github.com/apache/spark/pull/12601
Yes I realize that `SchemaRelationProvider` is not necessary, but the
legwork has already been done, so why not take advantage of it. Even still,
this original PR is actually not that far from
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/12601
To speed it up, let me change my PR
https://github.com/apache/spark/pull/14077 based on your feedback. You can
judge which one is better. Thanks!
---
If your project is set up for it, you can
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/12601
You know, https://github.com/apache/spark/pull/14075 has been merged. It is
unnecessary to extend `SchemaRelationProvider` in your PR, right?
---
If your project is set up for it, you can
Github user JustinPihony commented on the issue:
https://github.com/apache/spark/pull/12601
@gatorsmile I addressed the rollback in [this
comment](https://github.com/apache/spark/pull/12601#issuecomment-233076856) and
haven't heard back one way or the other. @srowen ?
---
If your
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/12601
Can you simplify the PR based on our previous discussion?
---
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
Github user JustinPihony commented on the issue:
https://github.com/apache/spark/pull/12601
@srowen I have addressed all of your comments. The only changes made were
to remove `sys.error` and use `require` where appropriate.
---
If your project is set up for it, you can reply to
Github user JustinPihony commented on the issue:
https://github.com/apache/spark/pull/12601
@rxin I am sure you are busy, so if there is anybody else I can ping to
have this reviewed please let me know.
---
If your project is set up for it, you can reply to this email and have your
Github user JustinPihony commented on the issue:
https://github.com/apache/spark/pull/12601
@rxin Another ping for this to be reviewed please :)
---
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
Github user rxin commented on the issue:
https://github.com/apache/spark/pull/12601
Thanks for pinging. Will take a look soon!
---
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
Github user JustinPihony commented on the issue:
https://github.com/apache/spark/pull/12601
Bumping my JIRA comment to here for @rxin to respond please?
@rxin Given the bug found in SPARK-16401, the CreatableRelationProvider is
not necessary. However it might be nice to have
Github user JustinPihony commented on the issue:
https://github.com/apache/spark/pull/12601
@gatorsmile I did just review it and still prefer mine...a simpler PR does
not necessarily mean it is more correct.
---
If your project is set up for it, you can reply to this email and have
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/12601
@JustinPihony Sorry, I did not realize you submitted a PR for the same
issue. Could you please review my PR?
https://github.com/apache/spark/pull/14077 I think my solution might be cleaner
and
Github user JustinPihony commented on the issue:
https://github.com/apache/spark/pull/12601
Bump @HyukjinKwon I have some comments to your comments. Could you please
review them and I can push my changes.
---
If your project is set up for it, you can reply to this email and have
Github user JustinPihony commented on the issue:
https://github.com/apache/spark/pull/12601
Bump :) Anybody able to review this one for me please?
---
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
81 matches
Mail list logo
