[GitHub] zeppelin pull request #2347: [Bug Fix][ZEPPELIN-2554] sql parser fix (backsl...

classic Classic list List threaded Threaded
31 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin pull request #2347: [Bug Fix][ZEPPELIN-2554] sql parser fix (backsl...

prabhjyotsingh
GitHub user tinkoff-dwh opened a pull request:

    https://github.com/apache/zeppelin/pull/2347

    [Bug Fix][ZEPPELIN-2554] sql parser fix (backslash)

    ### What is this PR for?
    Fix of parser to correctly parse backslash in quotes.
   
    ### What type of PR is it?
    Bug Fix
   
    ### 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?
    Execute `select '\n', ';'`
   
    ### Screenshots (if appropriate)
    before
    ![before](https://cloud.githubusercontent.com/assets/25951039/26098731/14562fa6-3a42-11e7-8361-5869cbfb42d3.png)
    text is parsed as 2 queries `select '\n', '` and `'`
   
   
    after
    ![after](https://cloud.githubusercontent.com/assets/25951039/26098738/18adbaa6-3a42-11e7-97c9-9412de556883.png)
   
   
    ### Questions:
    * Does the licenses files need update? no
    * Is there breaking changes for older versions? no
    * Does this needs documentation? no


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/tinkoff-dwh/zeppelin ZEPPELIN-2554

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zeppelin/pull/2347.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 #2347
   
----
commit 3951aa7053e3b33a0288eac879506845b03a1ca7
Author: Tinkoff DWH <[hidden email]>
Date:   2017-05-16T07:29:56Z

    [ZEPPELIN-2554] sql parser fix (backslash)

commit af508f1ca4fbd6765175d01a454b54893054f3bc
Author: Tinkoff DWH <[hidden email]>
Date:   2017-05-16T09:19:51Z

    [ZEPPELIN-2554] fix tests

----


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2347: [Bug Fix][ZEPPELIN-2554] sql parser fix (backslash)

prabhjyotsingh
Github user zjffdu commented on the issue:

    https://github.com/apache/zeppelin/pull/2347
 
    \cc @prabhjyotsingh


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2347: [Bug Fix][ZEPPELIN-2554] sql parser fix (backslash)

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user zjffdu commented on the issue:

    https://github.com/apache/zeppelin/pull/2347
 
    This is for multiple line sql support, seems this is the only way. @prabhjyotsingh has more context.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2347: [Bug Fix][ZEPPELIN-2554] sql parser fix (backslash)

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user prabhjyotsingh commented on the issue:

    https://github.com/apache/zeppelin/pull/2347
 
    Thanks, @zjffdu. @tinkoff-dwh  will try to review this 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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2347: [Bug Fix][ZEPPELIN-2554] sql parser fix (backslash)

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user felixcheung commented on the issue:

    https://github.com/apache/zeppelin/pull/2347
 
    Perhaps multiple queries was the reason we tried to parse the text (we really need to document these things better) - though I'm not sure how it would work exactly? Do you get multiple table in result if there are multiple queries in one paragraph? Does it even handle the multiple results correctly since that was added after JDBC interpreter is implemented?
   
    In any case I do think it is much cleaner not to mess with the content.
   
    Perhaps default to single query with a switch to fallback to old code and see if we get any feedback on 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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2347: [Bug Fix][ZEPPELIN-2554] sql parser fix (backslash)

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user prabhjyotsingh commented on the issue:

    https://github.com/apache/zeppelin/pull/2347
 
    @felixcheung  +1 for defaulting to the single query.
   
    And for all of your questions answer is yes.
    ![1845](https://cloud.githubusercontent.com/assets/674497/21741685/1cbfb62e-d504-11e6-92c5-0e5a52436ae5.gif)


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2347: [Bug Fix][ZEPPELIN-2554] sql parser fix (backslash)

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user felixcheung commented on the issue:

    https://github.com/apache/zeppelin/pull/2347
 
    hmm, I see, 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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2347: [Bug Fix][ZEPPELIN-2554] sql parser fix (backslash)

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user tinkoff-dwh commented on the issue:

    https://github.com/apache/zeppelin/pull/2347
 
    @prabhjyotsingh
    fixed problems


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2347: [Bug Fix][ZEPPELIN-2554] sql parser fix (backslash)

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user prabhjyotsingh commented on the issue:

    https://github.com/apache/zeppelin/pull/2347
 
    Tested this on local, looks OK.
   
    Can you also implement this comment https://github.com/apache/zeppelin/pull/2347#issuecomment-302747859, i.e. default to single query with a switch to make multiple queries work?


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2347: [Bug Fix][ZEPPELIN-2554] sql parser fix (backslash)

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user tinkoff-dwh commented on the issue:

    https://github.com/apache/zeppelin/pull/2347
 
    @prabhjyotsingh
    checkbox (for paragraph) which indicates to parse the request or not?


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2347: [Bug Fix][ZEPPELIN-2554] sql parser fix (backslash)

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user prabhjyotsingh commented on the issue:

    https://github.com/apache/zeppelin/pull/2347
 
    IMO property in settings of interpreter makes more sense, with a bit of a documentation on how to enable/disable it.
    cc: @felixcheung


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2347: [Bug Fix][ZEPPELIN-2554] sql parser fix (backslash)

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user felixcheung commented on the issue:

    https://github.com/apache/zeppelin/pull/2347
 
    yes for interpreter settings


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2347: [Bug Fix][ZEPPELIN-2554] sql parser fix (backslash)

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user tinkoff-dwh commented on the issue:

    https://github.com/apache/zeppelin/pull/2347
 
    @felixcheung   @prabhjyotsingh
    have a problem with single mode, if user execute multiple queries (all text executes as single query) then result set is incorrect.
   
    I think this setting is redundant. If the user wants to execute single query, he can write a single 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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2347: [Bug Fix][ZEPPELIN-2554] sql parser fix (backslash)

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user prabhjyotsingh commented on the issue:

    https://github.com/apache/zeppelin/pull/2347
 
    Which I think is fine, because in that case, we will not try to use this logic, and at least there will be a workaround if the user is facing any problem because of this issue, and we can have a document for 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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2347: [Bug Fix][ZEPPELIN-2554] sql parser fix (backslash)

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user tinkoff-dwh commented on the issue:

    https://github.com/apache/zeppelin/pull/2347
 
    single mode is default and the user will always get an incorrect result if he writes multiple queries to in paragraph


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2347: [Bug Fix][ZEPPELIN-2554] sql parser fix (backslash)

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user prabhjyotsingh commented on the issue:

    https://github.com/apache/zeppelin/pull/2347
 
    Which is fine, with the document there will be a workaround.
    And the case that you are talking is true for Postgres, but for Hive if you will try to execute multiple SQLs in one statement then it fails.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2347: [Bug Fix][ZEPPELIN-2554] sql parser fix (backslash)

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user tinkoff-dwh commented on the issue:

    https://github.com/apache/zeppelin/pull/2347
 
    I think is better without this parameter, what to do with the incorrect result. if there are problems, then think how to solve it. Postgres by default, and many use it and all users will have incorrect data


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2347: [Bug Fix][ZEPPELIN-2554] sql parser fix (backslash)

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user prabhjyotsingh commented on the issue:

    https://github.com/apache/zeppelin/pull/2347
 
    IMO there should be a workaround if the logic fails.
   
    And for handling error scenarios, here is what I think will happen.
    Before recommendation:
    ![screen shot 2017-05-23 at 12 11 45 pm](https://cloud.githubusercontent.com/assets/674497/26341426/4eb2b114-3fb1-11e7-8328-9ddfe70a96fd.png)
   
    After recommendation:
    ![screen shot 2017-05-23 at 12 12 31 pm](https://cloud.githubusercontent.com/assets/674497/26341427/4eb3f75e-3fb1-11e7-97cb-70d8953d9158.png)



---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2347: [Bug Fix][ZEPPELIN-2554] sql parser fix (backslash)

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user tinkoff-dwh commented on the issue:

    https://github.com/apache/zeppelin/pull/2347
 
    if all quieries is valid then the result will return from the first. is it correct?


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2347: [Bug Fix][ZEPPELIN-2554] sql parser fix (backslash)

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user prabhjyotsingh commented on the issue:

    https://github.com/apache/zeppelin/pull/2347
 
    Yes, correct.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
12