You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by prabhjyotsingh <gi...@git.apache.org> on 2017/01/05 05:53:55 UTC
[GitHub] zeppelin pull request #1845: [ZEPPELIN-1906] Use multiple InterpreterResult ...
GitHub user prabhjyotsingh opened a pull request:
https://github.com/apache/zeppelin/pull/1845
[ZEPPELIN-1906] Use multiple InterpreterResult for displaying multiple JDBC queries
### What is this PR for?
Use multiple InterpreterResult for displaying multiple JDBC queries.
IMO since other sql editors allows to execute multiple sql separated with ";" and ours display mechanism being more powerful, hence, it should also allow the same.
### What type of PR is it?
[Improvement]
### What is the Jira issue?
* [ZEPPELIN-1906](https://issues.apache.org/jira/browse/ZEPPELIN-1906)
### How should this be tested?
Try running following in a paragraph and check for output.
```
%jdbc
create table test_temp_table (id int);
select column_name, data_type, character_maximum_length from INFORMATION_SCHEMA.COLUMNS where table_name = 'test_temp_table';
SELECT table_name FROM information_schema.tables WHERE table_schema = 'public';
drop table test_temp_table;
SELECT table_name FROM information_schema.tables WHERE table_schema = 'public';
```
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update? N/A
* Is there breaking changes for older versions? N/A
* Does this needs documentation? N/A
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/prabhjyotsingh/zeppelin ZEPPELIN-1906
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/1845.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 #1845
----
commit f5ab79687126849778809c9f1851171bacc52e15
Author: Prabhjyot Singh <pr...@gmail.com>
Date: 2017-01-05T05:48:29Z
Use multiple InterpreterResult for displaying multiple JDBC queries
----
---
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] zeppelin issue #1845: [ZEPPELIN-1906] Use multiple InterpreterResult for dis...
Posted by prabhjyotsingh <gi...@git.apache.org>.
Github user prabhjyotsingh commented on the issue:
https://github.com/apache/zeppelin/pull/1845
That logic is something that I have not changed, and I think we can take care/discuss it in a different PR.
This is what I've tried on the existing functionality.
---
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] zeppelin issue #1845: [ZEPPELIN-1906] Use multiple InterpreterResult for dis...
Posted by prabhjyotsingh <gi...@git.apache.org>.
Github user prabhjyotsingh commented on the issue:
https://github.com/apache/zeppelin/pull/1845
@astroshim do you have some cycle to review 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1845: [ZEPPELIN-1906] Use multiple InterpreterResult for dis...
Posted by astroshim <gi...@git.apache.org>.
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1845
Sorry for late response @prabhjyotsingh, @jongyoul .
Let me review 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
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] zeppelin pull request #1845: [ZEPPELIN-1906] Use multiple InterpreterResult ...
Posted by prabhjyotsingh <gi...@git.apache.org>.
GitHub user prabhjyotsingh reopened a pull request:
https://github.com/apache/zeppelin/pull/1845
[ZEPPELIN-1906] Use multiple InterpreterResult for displaying multiple JDBC queries
### What is this PR for?
Use multiple InterpreterResult for displaying multiple JDBC queries.
IMO since other sql editors allows to execute multiple sql separated with ";" and ours display mechanism being more powerful, hence, it should also allow the same.
### What type of PR is it?
[Improvement]
### What is the Jira issue?
* [ZEPPELIN-1906](https://issues.apache.org/jira/browse/ZEPPELIN-1906)
### How should this be tested?
Try running following in a paragraph (with Postgres setting) and check for output.
```
%jdbc
create table test_temp_table (id int);
select column_name, data_type, character_maximum_length from INFORMATION_SCHEMA.COLUMNS where table_name = 'test_temp_table';
SELECT table_name FROM information_schema.tables WHERE table_schema = 'public';
drop table test_temp_table;
SELECT table_name FROM information_schema.tables WHERE table_schema = 'public';
```
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update? N/A
* Is there breaking changes for older versions? N/A
* Does this needs documentation? N/A
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/prabhjyotsingh/zeppelin ZEPPELIN-1906
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/1845.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 #1845
----
commit f5ab79687126849778809c9f1851171bacc52e15
Author: Prabhjyot Singh <pr...@gmail.com>
Date: 2017-01-05T05:48:29Z
Use multiple InterpreterResult for displaying multiple JDBC queries
commit e675190ac0674086dd7d34b4f810f3b0dab0a519
Author: Prabhjyot Singh <pr...@gmail.com>
Date: 2017-01-05T06:52:23Z
user same connection instead of creating new everytime
commit c096e76b1805fc5c3fea5bce1a31c0987ffae8cc
Author: Prabhjyot Singh <pr...@gmail.com>
Date: 2017-01-05T06:53:53Z
remove extra empty lines
commit e6727b555c146b1209b0533414b658b9c3079a52
Author: Prabhjyot Singh <pr...@gmail.com>
Date: 2017-01-05T15:06:38Z
add testcase for spliting sql.
commit f3da37f14601739219a8d771fbf3f5d0b7fce200
Author: Prabhjyot Singh <pr...@gmail.com>
Date: 2017-01-05T16:05:32Z
replace regex with slightly better logic.
commit ac4663d706841c8909f9ce99fbe952d2f9c4b1ac
Author: Prabhjyot Singh <pr...@gmail.com>
Date: 2017-01-05T16:19:24Z
add block comment
commit b3e742e8f60f8e5835fdd5e8aa006d0219310709
Author: Prabhjyot Singh <pr...@gmail.com>
Date: 2017-01-05T16:34:36Z
fixing checkstyle-fail-build
commit f9fd5c694f132a8953bf970d1686cdef714e1bae
Author: Prabhjyot Singh <pr...@gmail.com>
Date: 2017-01-05T16:57:19Z
allow last query to be without ";"
----
---
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] zeppelin issue #1845: [ZEPPELIN-1906] Use multiple InterpreterResult for dis...
Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/1845
LGTM, but I notice there's one large blank area before this exception. Is it expected ?
---
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] zeppelin issue #1845: [ZEPPELIN-1906] Use multiple InterpreterResult for dis...
Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/1845
@prabhjyotsingh What if one query fails ? Would it continue to run the remaining queries or abort ? And since this is a new feature, we might better to add document for it.
---
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] zeppelin issue #1845: [ZEPPELIN-1906] Use multiple InterpreterResult for dis...
Posted by r-kamath <gi...@git.apache.org>.
Github user r-kamath commented on the issue:
https://github.com/apache/zeppelin/pull/1845
Tested. LGTM.
---
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] zeppelin pull request #1845: [ZEPPELIN-1906] Use multiple InterpreterResult ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/zeppelin/pull/1845
---
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] zeppelin issue #1845: [ZEPPELIN-1906] Use multiple InterpreterResult for dis...
Posted by astroshim <gi...@git.apache.org>.
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1845
@prabhjyotsingh Yea I know you didn't change it. That's because it doesn't have state before.
I agree that we can take care/discuss it in a different 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1845: [ZEPPELIN-1906] Use multiple InterpreterResult for dis...
Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the issue:
https://github.com/apache/zeppelin/pull/1845
@astroshim Can you also review this PR?
---
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] zeppelin pull request #1845: [ZEPPELIN-1906] Use multiple InterpreterResult ...
Posted by prabhjyotsingh <gi...@git.apache.org>.
Github user prabhjyotsingh closed the pull request at:
https://github.com/apache/zeppelin/pull/1845
---
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] zeppelin issue #1845: [ZEPPELIN-1906] Use multiple InterpreterResult for dis...
Posted by astroshim <gi...@git.apache.org>.
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1845
@prabhjyotsingh Thank you for great improvement!
I tested this with mysql and all features are working well except one.
In following case, database connection would be closed and user should `use db` command again.
so I think it needs something for managing connection state.
1. use db
2. execute right query.
3. execute fail query.
4. execute right query : then db connection would be closed.
What do you think?
---
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] zeppelin issue #1845: [ZEPPELIN-1906] Use multiple InterpreterResult for dis...
Posted by prabhjyotsingh <gi...@git.apache.org>.
Github user prabhjyotsingh commented on the issue:
https://github.com/apache/zeppelin/pull/1845
@zjffdu How about this ? On error show output of previous successfully run query.
---
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] zeppelin issue #1845: [ZEPPELIN-1906] Use multiple InterpreterResult for dis...
Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/1845
I don't think it make sense to have a fixed minimum table size. It should also take into account the output row number. Anyway, we can improve it in a followup ticket.
---
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] zeppelin issue #1845: [ZEPPELIN-1906] Use multiple InterpreterResult for dis...
Posted by prabhjyotsingh <gi...@git.apache.org>.
Github user prabhjyotsingh commented on the issue:
https://github.com/apache/zeppelin/pull/1845
Yes, that is our default minimum size of display table.
CI fails [#9847.5](https://travis-ci.org/apache/zeppelin/jobs/189775789); which is not relevant to this. Will merge this if no more discussion.
```
12:50:20,122 ERROR org.apache.zeppelin.search.LuceneSearch:120 - Failed to open index dir RAMDirectory@4cc68af8 lockFactory=org.apache.lucene.store.SingleInstanceLockFactory@7ecdc728, make sure indexing finished OK
org.apache.lucene.index.IndexNotFoundException: no segments* file found in RAMDirectory@4cc68af8 lockFactory=org.apache.lucene.store.SingleInstanceLockFactory@7ecdc728: files: []
at org.apache.lucene.index.SegmentInfos$FindSegmentsFile.run(SegmentInfos.java:726)
at org.apache.lucene.index.StandardDirectoryReader.open(StandardDirectoryReader.java:50)
at org.apache.lucene.index.DirectoryReader.open(DirectoryReader.java:63)
at org.apache.zeppelin.search.LuceneSearch.query(LuceneSearch.java:104)
at org.apache.zeppelin.search.LuceneSearchTest.canNotSearchBeforeIndexing(LuceneSearchTest.java:147)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:606)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:264)
at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:153)
at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:124)
at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:200)
at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:153)
at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:103)
```
---
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] zeppelin issue #1845: [ZEPPELIN-1906] Use multiple InterpreterResult for dis...
Posted by prabhjyotsingh <gi...@git.apache.org>.
Github user prabhjyotsingh commented on the issue:
https://github.com/apache/zeppelin/pull/1845
@zjffdu replaced regex with slightly better logic, let me know what you think about 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1845: [ZEPPELIN-1906] Use multiple InterpreterResult for dis...
Posted by prabhjyotsingh <gi...@git.apache.org>.
Github user prabhjyotsingh commented on the issue:
https://github.com/apache/zeppelin/pull/1845
`What if one query fails ?`
There is no change in that. When it fails and doesn't continue.
Before:
After:
But I do see scope for improvement. Let me update that.
---
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.
---