You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by "Thejas M Nair (JIRA)" <ji...@apache.org> on 2014/08/15 19:39:19 UTC

[jira] [Comment Edited] (HIVE-7680) Do not throw SQLException for HiveStatement getMoreResults and setEscapeProcessing(false)

    [ https://issues.apache.org/jira/browse/HIVE-7680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14098799#comment-14098799 ] 

Thejas M Nair edited comment on HIVE-7680 at 8/15/14 5:39 PM:
--------------------------------------------------------------

Thanks for the extensive research and experiments [~apivovarov] ! 
I spend some more time reading up on this. Returning -1 instead of 0 for getUpdateCount might be a better behavior. It does look like better behavior than what we have. But the really correct behavior (when statement.execute indicates it is not a ResultSet), seems to be returning 0 the first time and returning -1 in the subsequent calls. According to what I read, statements that don't update rows such as "create table" are expected to return 0.

This is would be easy to implement using another variable in HiveStatement.

Other related changes that could potentially be made along with this is -
* getMoreResults returning appropriate value instead of throwing exception (Returns value of stmtHandle.isHasResultSet() the first time it is called, then false for subsequent calls)
* getResultSet returning the ResultSet only the first time it is called





was (Author: thejas):
Thanks for the extensive research and experiments [~apivovarov] ! 
I spend some more time reading up on this. Returning -1 instead of 0 for getUpdateCount might be a better behavior. It does look like better behavior than what we have. But the really correct behavior (when statement.execute indicates it is not a ResultSet), seems to be returning 0 the first time and returning -1 in the subsequent calls.

This is would be easy to implement using another variable in HiveStatement.

Other related changes that could potentially be made along with this is -
* getMoreResults returning appropriate value instead of throwing exception (Returns value of stmtHandle.isHasResultSet() the first time it is called, then false for subsequent calls)
* getResultSet returning the ResultSet only the first time it is called




> Do not throw SQLException for HiveStatement getMoreResults and setEscapeProcessing(false)
> -----------------------------------------------------------------------------------------
>
>                 Key: HIVE-7680
>                 URL: https://issues.apache.org/jira/browse/HIVE-7680
>             Project: Hive
>          Issue Type: Bug
>          Components: JDBC
>    Affects Versions: 0.13.1
>            Reporter: Alexander Pivovarov
>            Assignee: Alexander Pivovarov
>            Priority: Minor
>         Attachments: HIVE-7680.patch
>
>
> 1. Some JDBC clients call method setEscapeProcessing(false)  (e.g. SQL Workbench)
> Looks like setEscapeProcessing(false) should do nothing.So, lets do  nothing instead of throwing SQLException
> 2. getMoreResults is needed in case Statements returns several ReseltSet.
> Hive does not support Multiple ResultSets. So this method can safely always return false.
> 3. getUpdateCount. Currently this method always returns 0. Hive cannot tell us how many rows were inserted. According to JDBC spec it should return " -1 if the current result is a ResultSet object or there are no more results" 
> if this method returns 0 then in case of execution insert statement JDBC client shows "0 rows were inserted" which is not true.
> if this method returns -1 then JDBC client runs insert statements and  shows that it was executed successfully, no result were returned. 
> I think the latter behaviour is more correct.
> 4. Some methods in Statement class should throw SQLFeatureNotSupportedException if they are not supported.  Current implementation throws SQLException instead which means database access error.



--
This message was sent by Atlassian JIRA
(v6.2#6252)