hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ted Yu (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-10965) Automate detection of presence of Filter#filterRow()
Date Tue, 15 Apr 2014 17:12:21 GMT

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

Ted Yu commented on HBASE-10965:
--------------------------------

bq. You would change how we run filters to accommodate brokenly implemented filters?

Accommodating brokenly implemented filters is already in place. Please take a look at the
following method in HRegion#RegionScannerImpl:
{code}
    private boolean filterRow() throws IOException {
      // when hasFilterRow returns true, filter.filterRow() will be called automatically inside
      // filterRowCells(List<Cell> kvs) so we skip that scenario here.
      return filter != null && (!filter.hasFilterRow())
          && filter.filterRow();
    }
{code}
When filter.hasFilterRow() returns false, filter.filterRow() is still called.

bq. Are folks implementing filters w/o reading the Interface

Here is one example: phoenix-core/src/main/java/org/apache/phoenix/filter/MultiKeyValueComparisonFilter.java
{code}
    @Override
    public boolean filterRow() {
{code}
But hasFilterRow() is not defined. There're other files in Phoenix which exhibit the same
pattern.

bq. is it not clear on what needs overriding
>From hbase-client/src/main/java/org/apache/hadoop/hbase/filter/Filter.java (since 0.96):
{code}
  /**
   * Primarily used to check for conflicts with scans(such as scans that do not read a full
row at a
   * time).
   *
   * @return True if this filter actively uses filterRow(List) or filterRow().
   */
  abstract public boolean hasFilterRow();
{code}
The contract is clearly published but not all people follow it.

If a custom filter overrides filterRow(List) or filterRow(), it is easy to use reflection
to detect. Having hasFilterRow() method leaves room for inconsistency.

If we can reach consensus on whether hasFilterRow() method should be deprecated first and
then removed, I will address Lars' and Anoop's review comments in the next patch.

> Automate detection of presence of Filter#filterRow()
> ----------------------------------------------------
>
>                 Key: HBASE-10965
>                 URL: https://issues.apache.org/jira/browse/HBASE-10965
>             Project: HBase
>          Issue Type: Task
>            Reporter: Ted Yu
>            Assignee: Ted Yu
>         Attachments: 10965-v1.txt, 10965-v2.txt, 10965-v3.txt, 10965-v4.txt
>
>
> There is potential inconsistency between the return value of Filter#hasFilterRow() and
presence of Filter#filterRow().
> Filters may override Filter#filterRow() while leaving return value of Filter#hasFilterRow()
being false (inherited from FilterBase).
> This JIRA aims to remove the inconsistency by automatically detecting the presence of
overridden Filter#filterRow(). If filterRow() is implemented and not inherited from FilterBase,
it is equivalent to having hasFilterRow() return true.



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

Mime
View raw message