[ZEPPELIN-1876] improved comptetion with schema/table/column separati… by sotnich · Pull Request #1886 · apache/zeppelin

sotnich · 2017-01-11T15:53:18Z

…on + sqlcompleter tests

What is this PR for?

This PR changes autocompletion behaviour in jdbc interpeter.
There are some changes:

[main change] autocompletion now depends on what are you typing. Now there are four types of competion: schema, table, column and keywords. If you typing new word then autocompetion suggests only keywords and schema names. If you are typing after schema name with point then you get list of tables in that schema. Also if you typing a name after point after a table name you will get a list of column names of this table.
autocomption now supports aliases in sql. If you write alias for a table in sql you will get a list of columns for an aliased table if you write down alias and point.
autocompletion now load keywords only in low case (otherwise there are so many keywords in a list of autocompletion that it is becomes uncomfortable)

What type of PR is it?

Improvement

Todos

- sort names in the output of autocompletion
- list only schema names if we are typing a schema name (for example after keywork FROM)
- add description in autocompletion list for schema names - schema, for table names - table and so
- autocompletion must initialize on opening of interpeter (not only after execution of sql)
- update autocompletion schemas only after execute update sql, not after every sql ???
- new option for postgresql interpreter: postgresql.completer.schema.filter. Filter schema names loaded into autocompletion (no more garbage schema names).

What is the Jira issue?

Open an issue on Jira https://issues.apache.org/jira/browse/ZEPPELIN/
Put link here, and add [ZEPPELIN-Jira number] in PR title, eg. [ZEPPELIN-533]

How should this be tested?

Outline the steps to test the PR here.

Screenshots (if appropriate)

https://issues.apache.org/jira/secure/attachment/12845228/auto1.JPG
https://issues.apache.org/jira/secure/attachment/12845229/auto2.JPG
https://issues.apache.org/jira/secure/attachment/12845230/auto3.JPG

Questions:

Does the licenses files need update? No
Is there breaking changes for older versions? No
Does this needs documentation? Yes

jongyoul · 2017-01-11T16:12:10Z

@astroshim Could you please review this PR?

felixcheung · 2017-01-13T17:36:19Z

jdbc/src/main/java/org/apache/zeppelin/jdbc/SqlCompleter.java

+      } finally {
+        schemas.close();
+      }
+    } catch (Throwable t) {


Is Exception ok? Should it be Throwable?

Yeah, thanks, SQLException is better. Have changed it.

sotnich · 2017-01-18T07:28:26Z

Don't think CI errors are related to this PR.

jongyoul · 2017-01-18T16:40:08Z

jdbc/src/test/java/org/apache/zeppelin/jdbc/SqlCompleterTest.java

@@ -0,0 +1,313 @@
+package org.apache.zeppelin.jdbc;


You need to add license header into this file.

Unapproved licenses: /home/travis/build/apache/zeppelin/jdbc/src/test/java/org/apache/zeppelin/jdbc/SqlCompleterTest.java

Fixed, but still there are erros that I don't understand how to link to PR.

forget my previous message. I got wrong changes

jongyoul · 2017-01-20T14:06:20Z

There is a flaky test in the result:

WelcomePageSuite

Never mind the result. that test is being fixed now. LGTM

jongyoul · 2017-01-24T06:48:47Z

@sotnich Can you rebase it onto current master? Recently, we fixed some flaky tests including your case. It would make your CI green

sotnich · 2017-01-30T08:38:20Z

Made a rebase, now CI tests have only one error related with DistributedResourcePoolTest.
Think it's still not related to my PR.

cloverhearts · 2017-01-31T09:33:31Z

@sotnich
I was test.
LGTM.

I have a question.
Is it possible to only support pgsql? (actually, i am mysql/maria user)

sotnich · 2017-01-31T09:39:10Z

@cloverhearts
Please, clirify your question.
This PR supports every JDBC driver that can return schema and table names.
I have tested it on postgres, but I am sure it would correctly run on mysql and other relational databases.

astroshim · 2017-01-31T11:16:28Z

@sotnich Thank you for great improvement.
I tested mysql and postgresql and only postgresql is working expectedly like @cloverhearts.

Leemoonsoo · 2017-02-02T01:13:42Z

CI failure is not related to this change.
LGTM and merge to master if no further discussion.

sotnich · 2017-02-02T09:26:25Z

@cloverhearts @astroshim
Made a little change - now mysql also works as well as postgresql. I have tested it.

astroshim · 2017-02-03T09:35:22Z

@sotnich Great Job! but still doesn't work with mysql.

cloverhearts · 2017-02-03T09:47:54Z

Hello, I was check again.
but, does not work on mysql.
Is there anything I missed?

…on + sqlcompleter tests

…schemas (like mysql)

sotnich · 2017-02-09T09:11:25Z

@cloverhearts @astroshim, check it now, please. If it's still not working send me details: configuration of the interpreter and jdbc log file.

cloverhearts · 2017-02-10T04:59:56Z

@sotnich
now Well working!
good works!
Thank you for great feature!

Leemoonsoo · 2017-02-10T05:35:35Z

LGTM and merge to master if no more discussions.

…on + sqlcompleter tests ### What is this PR for? This PR changes autocompletion behaviour in jdbc interpeter. There are some changes: * [main change] autocompletion now depends on what are you typing. Now there are four types of competion: schema, table, column and keywords. If you typing new word then autocompetion suggests only keywords and schema names. If you are typing after schema name with point then you get list of tables in that schema. Also if you typing a name after point after a table name you will get a list of column names of this table. * autocomption now supports aliases in sql. If you write alias for a table in sql you will get a list of columns for an aliased table if you write down alias and point. * autocompletion now load keywords only in low case (otherwise there are so many keywords in a list of autocompletion that it is becomes uncomfortable) ### What type of PR is it? Improvement ### Todos * [ ] - sort names in the output of autocompletion * [ ] - list only schema names if we are typing a schema name (for example after keywork FROM) * [ ] - add description in autocompletion list for schema names - schema, for table names - table and so * [ ] - autocompletion must initialize on opening of interpeter (not only after execution of sql) * [ ] - update autocompletion schemas only after execute update sql, not after every sql ??? * [ ] - new option for postgresql interpreter: postgresql.completer.schema.filter. Filter schema names loaded into autocompletion (no more garbage schema names). ### What is the Jira issue? * Open an issue on Jira https://issues.apache.org/jira/browse/ZEPPELIN/ * Put link here, and add [ZEPPELIN-*Jira number*] in PR title, eg. [ZEPPELIN-533] ### How should this be tested? Outline the steps to test the PR here. ### Screenshots (if appropriate) https://issues.apache.org/jira/secure/attachment/12845228/auto1.JPG https://issues.apache.org/jira/secure/attachment/12845229/auto2.JPG https://issues.apache.org/jira/secure/attachment/12845230/auto3.JPG ### Questions: * Does the licenses files need update? No * Is there breaking changes for older versions? No * Does this needs documentation? Yes Author: Sotnichenko Sergey <[email protected]> Closes apache#1886 from sotnich/jdbc-1876 and squashes the following commits: 2db7ed8 [Sotnichenko Sergey] [ZEPPELIN-1876] add support for databases with only catalogs without schemas (like mysql) a048ef2 [Sotnichenko Sergey] [ZEPPELIN-1876] add support for databases with only catalogs without schemas (like mysql) f4b03df [Sotnichenko Sergey] [ZEPPELIN-1876] add support for databases with only catalogs without schemas (like mysql) 675c629 [Sotnichenko Sergey] [ZEPPELIN-1876] Adding licence header 9fac1d0 [Sotnichenko Sergey] [ZEPPELIN-1876] SQLException instead of Throwable 895c35a [Sotnichenko Sergey] [ZEPPELIN-1876] SQLException instead of Throwable 7d40166 [Sotnichenko Sergey] [ZEPPELIN-1876] improved comptetion with schema/table/column separation + sqlcompleter tests

felixcheung reviewed Jan 13, 2017

View reviewed changes

jongyoul reviewed Jan 18, 2017

View reviewed changes

sotnich force-pushed the jdbc-1876 branch from e99f1de to 32688a8 Compare January 30, 2017 07:11

sotnich force-pushed the jdbc-1876 branch from 32688a8 to cfa4649 Compare February 2, 2017 09:22

Sotnichenko Sergey added 7 commits February 9, 2017 11:40

[ZEPPELIN-1876] improved comptetion with schema/table/column separati…

7d40166

…on + sqlcompleter tests

[ZEPPELIN-1876] SQLException instead of Throwable

895c35a

[ZEPPELIN-1876] SQLException instead of Throwable

9fac1d0

[ZEPPELIN-1876] Adding licence header

675c629

[ZEPPELIN-1876] add support for databases with only catalogs without …

f4b03df

…schemas (like mysql)

[ZEPPELIN-1876] add support for databases with only catalogs without …

a048ef2

…schemas (like mysql)

[ZEPPELIN-1876] add support for databases with only catalogs without …

2db7ed8

…schemas (like mysql)

sotnich force-pushed the jdbc-1876 branch from fe3ae5b to 2db7ed8 Compare February 9, 2017 08:41

asfgit closed this in b579a64 Feb 11, 2017

Conversation

sotnich commented Jan 11, 2017

What is this PR for?

What type of PR is it?

Todos

What is the Jira issue?

How should this be tested?

Screenshots (if appropriate)

Questions:

Uh oh!

jongyoul commented Jan 11, 2017

Uh oh!

felixcheung Jan 13, 2017

Choose a reason for hiding this comment

Uh oh!

sotnich Jan 15, 2017

Choose a reason for hiding this comment

Uh oh!

sotnich commented Jan 18, 2017

Uh oh!

jongyoul Jan 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sotnich Jan 20, 2017

Choose a reason for hiding this comment

Uh oh!

jongyoul Jan 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jongyoul commented Jan 20, 2017

Uh oh!

jongyoul commented Jan 24, 2017

Uh oh!

sotnich commented Jan 30, 2017

Uh oh!

cloverhearts commented Jan 31, 2017

Uh oh!

sotnich commented Jan 31, 2017

Uh oh!

astroshim commented Jan 31, 2017

Uh oh!

Leemoonsoo commented Feb 2, 2017

Uh oh!

sotnich commented Feb 2, 2017

Uh oh!

astroshim commented Feb 3, 2017

Uh oh!

cloverhearts commented Feb 3, 2017

Uh oh!

sotnich commented Feb 9, 2017

Uh oh!

cloverhearts commented Feb 10, 2017

Uh oh!

Leemoonsoo commented Feb 10, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jongyoul Jan 18, 2017 •

edited

Loading

jongyoul Jan 20, 2017 •

edited

Loading