[jira] [Commented] (NIFI-5312) QueryDatabaseTable updates state when an SQLException is thrown

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on NIFI-5312:
--------------------------------------

Github user mattyb149 commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/2868#discussion_r201458637
  
    --- Diff: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/QueryDatabaseTable.java
 ---
    @@ -543,5 +550,10 @@ public void processRow(ResultSet resultSet) throws 
IOException {
                     throw new IOException(e);
                 }
             }
    +
    +        @Override
    +        public void rollbackStateChanges() {
    +            this.newColMap = this.originalState;
    --- End diff --
    
    This only updates the reference, not the actual map pointed to by it (i.e. 
the one that's passed in). Maybe instead we use originalState as the reference 
to stateMap, and have "updatedStateMap" or something be the copy of the input 
map, and update that instead of the passed in map. If all succeeds, then we can 
call `originalState.putAll(updatedStateMap)`, if anything fails then the input 
map is intact.
    
    I found this when adding a unit test that illustrates the error (and fix), 
not sure if it's finished/correct but please feel free to incorporate it if you 
like:
    
    ```
    @Test
    public void testWithExceptionAfterSomeRowsProcessed() throws SQLException {
            // load test data to database
            final Connection con = ((DBCPService) 
runner.getControllerService("dbcp")).getConnection();
            Statement stmt = con.createStatement();
    
            try {
                stmt.execute("drop table TEST_NULL_INT");
            } catch (final SQLException sqle) {
                // Ignore, usually due to Derby not having DROP TABLE IF EXISTS
            }
    
            stmt.execute("create table TEST_NULL_INT (id integer not null, val1 
integer, val2 integer, constraint my_pk primary key (id))");
    
            stmt.execute("insert into TEST_NULL_INT (id, val1, val2) VALUES (1, 
NULL, 1)");
            stmt.execute("insert into TEST_NULL_INT (id, val1, val2) VALUES (2, 
1, 1)");
    
            runner.setIncomingConnection(false);
            runner.setProperty(QueryDatabaseTable.TABLE_NAME, "TEST_NULL_INT");
            
runner.setProperty(AbstractDatabaseFetchProcessor.MAX_VALUE_COLUMN_NAMES, "id");
    
            // Override adapter with one that fails after the first row is 
processed
            QueryDatabaseTable.dbAdapters.put(dbAdapter.getName(), new 
GenericDatabaseAdapter() {
                boolean fail = false;
                @Override
                public String getName() {
                    if(!fail) {
                        fail = true;
                        return super.getName();
                    }
                    throw new DataFileWriter.AppendWriteException(null);
                }
            });
            runner.run();
            
assertTrue(runner.getFlowFilesForRelationship(QueryDatabaseTable.REL_SUCCESS).isEmpty());
            // State should not have been updated
            runner.getStateManager().assertStateNotSet("test_null_int@!@id", 
Scope.CLUSTER);
    
            // Restore original (working) adapter and run again
            QueryDatabaseTable.dbAdapters.put(dbAdapter.getName(), dbAdapter);
            runner.run();
            
assertFalse(runner.getFlowFilesForRelationship(QueryDatabaseTable.REL_SUCCESS).isEmpty());
            runner.getStateManager().assertStateEquals("test_null_int@!@id", 
"2", Scope.CLUSTER);
        }
    ```


> QueryDatabaseTable updates state when an SQLException is thrown
> ---------------------------------------------------------------
>
>                 Key: NIFI-5312
>                 URL: https://issues.apache.org/jira/browse/NIFI-5312
>             Project: Apache NiFi
>          Issue Type: Bug
>         Environment: Ubuntu 16.04 
> Apache NiFi 1.5.0, 1.6.0 
> IBM DB2 for Linux, UNIX and Windows 10.5.0.7, 10.5.0.8 (1) 
> IBM Data Server Driver for JDBC and SQLJ, JDBC 4.0 Driver (db2jcc4.jar) 
> 4.19.26 / v10.5 FP6, 4.19.72 / v10.5 FP9 (2) 
> Notes: 
> (1) SELECT * FROM SYSIBMADM.ENV_INST_INFO 
> (2) java -cp ./db2jcc4.jar com.ibm.db2.jcc.DB2Jcc -version
>            Reporter: Marcio Sugar
>            Assignee: Peter Wicks
>            Priority: Major
>
> I noticed that when an SQLException is thrown, at least in the situation 
> described by NIFI-4926, QueryDatabaseTable still updates the state of the 
> Maximum-value Columns. It means that when something goes wrong, a potentially 
> big number of rows will be skipped pretty much silently. (Well, it will 
> appear in the Bulletin Board, but when the message disappears from the 
> Bulletin Board there will be no indication of the problem left. The processor 
> has no "terminate relationship" other than "Success".)



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)