You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by jongyoul <gi...@git.apache.org> on 2015/12/06 13:53:27 UTC
[GitHub] incubator-zeppelin pull request: ZEPPELIN-487 Change supporting mu...
GitHub user jongyoul opened a pull request:
https://github.com/apache/incubator-zeppelin/pull/517
ZEPPELIN-487 Change supporting multiple statements to multiple connections
Changed multiple statements to multiple connection. Some JDBC don't support parallel executions with one connection.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jongyoul/incubator-zeppelin ZEPPELIN-487
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-zeppelin/pull/517.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 #517
----
commit b23dcb4a1677d3f16799e3eeda6346bc0f544efb
Author: Jongyoul Lee <jo...@gmail.com>
Date: 2015-12-06T10:57:50Z
ZEPPELIN-487 HiveInterpreter Multiple Connections
- keyConnectionMap -> propertyKeyConnectionMap
commit 113a5835ab73a2bcdb9de7f6fff2a1dda207f2c5
Author: Jongyoul Lee <jo...@gmail.com>
Date: 2015-12-06T12:48:36Z
ZEPPELIN-487 HiveInterpreter Multiple Connections
- Multiple Statements -> multiple connections
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: ZEPPELIN-487 Change supporting mu...
Posted by prasadwagle <gi...@git.apache.org>.
Github user prasadwagle commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/517#issuecomment-162349965
I tested the pull request and it allowed Vertica queries to execute in parallel with one modification. Thanks!
I had to comment:
// if (statement.isClosed()) {
// connection = getConnection(propertyKey);
// statement = connection.createStatement();
// }
since I ran into:
java.lang.AbstractMethodError: com.vertica.jdbc.VerticaStatementImpl.isClosed()Z
at org.apache.zeppelin.hive.HiveInterpreter.getStatement(HiveInterpreter.java:208)
Can you help me understand the need for the statement.isClosed() check in line 208 since the statement is created in line 207?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: ZEPPELIN-487 Change supporting mu...
Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/517#issuecomment-162368296
It's soft of defensive code for avoiding error. Do you think it's redundant? I'll fix this code more general :-). Thanks for the review quickly.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: ZEPPELIN-487 Change supporting mu...
Posted by jongyoul <gi...@git.apache.org>.
GitHub user jongyoul reopened a pull request:
https://github.com/apache/incubator-zeppelin/pull/517
ZEPPELIN-487 Change supporting multiple statements to multiple connections
Changed multiple statements to multiple connection. Some JDBC don't support parallel executions with one connection.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jongyoul/incubator-zeppelin ZEPPELIN-487
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-zeppelin/pull/517.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 #517
----
commit b23dcb4a1677d3f16799e3eeda6346bc0f544efb
Author: Jongyoul Lee <jo...@gmail.com>
Date: 2015-12-06T10:57:50Z
ZEPPELIN-487 HiveInterpreter Multiple Connections
- keyConnectionMap -> propertyKeyConnectionMap
commit 113a5835ab73a2bcdb9de7f6fff2a1dda207f2c5
Author: Jongyoul Lee <jo...@gmail.com>
Date: 2015-12-06T12:48:36Z
ZEPPELIN-487 HiveInterpreter Multiple Connections
- Multiple Statements -> multiple connections
commit fa547494c1ffed24bd816d3d55e611377f24a7df
Author: Jongyoul Lee <jo...@gmail.com>
Date: 2015-12-07T03:04:18Z
ZEPPELIN-487 HiveInterpreter Multiple Connections
- Fix the error when statement doesn't support isClosed method
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: ZEPPELIN-487 Change supporting mu...
Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul closed the pull request at:
https://github.com/apache/incubator-zeppelin/pull/517
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: ZEPPELIN-487 Change supporting mu...
Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/517#issuecomment-162734758
@praagarw Could you please review this PR again?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: ZEPPELIN-487 Change supporting mu...
Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/517#issuecomment-163107054
@praagarw Sure, but I couldn't guarantee all of jdbc connections create statement when it's alive. Thus double check might be helpful in that case. And thanks for kind review. I'm merging in three day.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: ZEPPELIN-487 Change supporting mu...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-zeppelin/pull/517
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: ZEPPELIN-487 Change supporting mu...
Posted by prasadwagle <gi...@git.apache.org>.
Github user prasadwagle commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/517#issuecomment-162991822
Hi Jongyoul, since createStatement is implemented in the JDBC drivers, I think it is unlikely it will return a closed statement. However, I am all for defensive coding and your fix in ZEPPELIN-487 looks good 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: ZEPPELIN-487 Change supporting mu...
Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/517#issuecomment-163107111
Merging without additional 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 have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---