Uploaded image for project: 'Apache IoTDB'
  1. Apache IoTDB
  2. IOTDB-1997

Fix incorrect AggregationResult generation in cluster DESC query

Attach filesAttach ScreenshotVotersWatch issueWatchersCreate sub-taskLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    Description

      This bug can reappear by replaying the test case IoTDBAggregationIT.firstTest in a 3 nodes with 2 replicas cluster.

      The main reason:

      Consider this query in the above IT

       

      "SELECT first_value(s0),first_value(s1),first_value(s2),first_value(s3) "
          + "FROM root.vehicle.d0 WHERE time >= 1500 AND time <= 9000 order by time desc" 

      And see the codes in https://github.com/apache/iotdb/blob/e08525072a88e6a99ee2026352fb133f40e7c3ee/cluster/src/main/java/org/apache/iotdb/cluster/query/LocalQueryExecutor.java#L693-L738

       

      If we query by DESC, the ascending == false, and we get the AggregationResult is FirstValueDescAggrResult, which should read data in decending order and calculate result.

      Howerver, in the AggregationExecutor.aggregateOneSeries, the reader generated is also a decending one, it will return a descending read BatchData when calling AggregationResult.updateResultFromPageData(). 

      Now let's investigate the implementation of FirstValueDescAggrResult.updateResultFromPageData(BatchData). It 'treats' the input BatchData as an ascending read sequence one(it will return the first data without any loop), which is a contradiction with the reader generated, thus the bug occurs.

      Here is a brief explain:

      The page data in order: 1,2,3

      The generated decending reader page data with the read sequence: 3,2,1

      And the FirstValueDescAggrResult will return the first one data, '3', which is different from our expected '1'

      So I think the best solution is

      1. Fix FirstValueDescAggrResult implementation.

      2. put the AggregationResult in an ascResultList and descResultList according to the method AggregationResult.isAscending().

      And we should add a comment to the abstract class AggregationResult interface.

       

      What's more, 

       

      Attachments

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            ericpai Eric Pai
            ericpai Eric Pai
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment