Off-by-one error in Row Filter node when using row ranges with variable override

MeikelBrandmeyer · October 11, 2019, 7:51am

I think, there is a problem with the row filter when using variable overrides to specify a row range. Here is a short example:

This is a simple little table and I filter rows two through to four. The result is as expected*.

Now, I simply override the range limits in the row filter with some variables, which have the same values, two and four respectively. But now I get:

Please note the subtle difference. This happens on KNIME 4.0.2.

To be honest: I’d rather have here a left-closed, right-open intervall instead.

PS: I’m sorry if this is not the right place for bug reports. But my google-fu was weak and I didn’t find a link to a bug tracker of sorts.

s.roughley · October 11, 2019, 7:59am

I’m pretty sure this is a known bug going back to the early days of knime - behind the scenes, the row indices are 0-based, but I think the thinking was that a 1-based numbering would be more intuitive to users, and so the displayed value in the node settings is actually the stored value + 1. You can see the offending source code here:

github.com

knime/knime-base/blob/0bb28c7e1da13ddb8c50e9474289a9001e817b59/org.knime.base/src/org/knime/base/node/preproc/filter/row/RowNoRowFilterPanel.java#L204


        throw new InvalidSettingsException("The RowNumberFilter range "
                + "cannot start at a row number less than 1.");
    }
    if ((last != RowNoRowFilter.EOT) && (last < first)) {
        throw new InvalidSettingsException("The end of the RowNumberFilter"
                + " range must be greater than the start.");
    }


    // the filter contains index values (starting from 0)
    // the spinner show the numbers, so we need to add 1 here.
    m_first.setValue(Long.valueOf(first + 1));
    m_tilEOT.setSelected(last == RowNoRowFilter.EOT); // en/disables
    // m_last
    if (last != RowNoRowFilter.EOT) {
        m_last.setValue(Long.valueOf(last + 1));
    }
    updateErrText();
}


/**
 * {@inheritDoc}

I think that is unfortunately one that has been there so long that it is not going to go away because it would break too many workflows

Steve

MeikelBrandmeyer · October 11, 2019, 8:02am

Well, then it should be documented at least in the node description, should it not?

s.roughley · October 11, 2019, 8:04am

Good idea - I’m going to tag @thor here (not sure who is the best KNIME team member for enhancement suggestions?)

Steve

ipazin · October 17, 2019, 3:38pm

Hi there @MeikelBrandmeyer,

welcome to KNIME Community Forum!

As @s.roughley said this is a known bug and I have added +1 to it. Additionally to mention new Row Filter node is planned that will also take care of this issue. In a mean time I will check about adding this info to node description.

Edit: Created a ticket to add this information into node description.

Br,
Ivan

system · April 17, 2020, 3:38am

This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.