[ 
https://issues.apache.org/jira/browse/DERBY-4338?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12748316#action_12748316
 ] 

Knut Anders Hatlen commented on DERBY-4338:
-------------------------------------------

Thanks for catching those additional cases and incorporating them into the 
test. The patch looks good to me.

Some small nits:

You may be able to simplify isolateAnyInitialIdentifier() by using the JDK's 
regex library. I think the code below is equivalent to the code in the patch.

    private final static Pattern INITIAL_IDENTIFIER = 
Pattern.compile("(\\p{Alpha}+).*");
    private String isolateAnyInitialIdentifier (String sql) {
        Matcher m = INITIAL_IDENTIFIER.matcher(sql);
        if (m.matches()) {
            return m.group(1);
        }
        return sql;
    }

In the tests there are some calls to check the return value from 
getUpdateCount() on this form:

    assertTrue(ps.getUpdateCount() == 0);

It's probably better to use assertEquals(0, ps.getUpdateCount()) to get more 
information in case of a failure.

There are a couple of occurrences of getConnection().createStatement() and 
getConnection().prepareStatement(). The getConnection() part should be dropped 
so that we use the methods in BaseJDBCTestCase that automatically close the 
statements.

Perhaps the calls to executeQuery() in testInitialComment_derby4338() and 
testWrongKeywordLexing_derby4338() should be wrapped in 
JDBC.assertDrainResults() so that we don't leave any results sets open?

A comment in testInitialComment_derby4338() stops in the middle of a sentence: 
// Change to 0 when DERBY-4362 is fixed. Before both

> Network client raises error "executeQuery method can not be used for update" 
> when sql is preceded by /* */ comments
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: DERBY-4338
>                 URL: https://issues.apache.org/jira/browse/DERBY-4338
>             Project: Derby
>          Issue Type: Bug
>          Components: Network Client
>    Affects Versions: 10.4.1.3, 10.4.2.0, 10.5.1.1, 10.5.2.0, 10.5.3.0
>            Reporter: Will Gomes
>            Assignee: Dag H. Wanvik
>             Fix For: 10.6.0.0
>
>         Attachments: derby-4338-a.diff, derby-4338-a.stat, derby-4338-b.diff, 
> derby-4338-b.stat, derby-4338-c.diff, derby-4338-c.stat
>
>
> Network derby client does not properly detect a sql select statement preceded 
> by /* */ comments.  As a result the sql appears to be detected as an update 
> statement, and results in  the following error:
>  org.apache.derby.client.am.SqlException: executeQuery method can not be used 
> for update.
>       at 
> org.apache.derby.client.am.Statement.checkForAppropriateSqlMode(Unknown 
> Source)
>       at org.apache.derby.client.am.PreparedStatement.flowExecute(Unknown 
> Source)
>       at org.apache.derby.client.am.PreparedStatement.executeQueryX(Unknown 
> Source)
> The problem appears to be in Statment.parseSqlAndSetSqlModes(), which only 
> appears to check for "--" style comments.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to