[GitHub] [incubator-doris] gaodayue commented on a change in pull request #2772: Support unsigned BIGINT element for BITMAP type

[GitBox <gi...@apache.org>]

gaodayue commented on a change in pull request #2772: Support unsigned BIGINT element for BITMAP type
URL: https://github.com/apache/incubator-doris/pull/2772#discussion_r366869894
 
 

 ##########
 File path: be/src/util/bitmap_value.h
 ##########
 @@ -0,0 +1,1245 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#ifndef DORIS_BE_SRC_UTIL_BITMAP_VALUE_H
+#define DORIS_BE_SRC_UTIL_BITMAP_VALUE_H
+
+#include <algorithm>
+#include <cstdarg>
+#include <cstdio>
+#include <limits>
+#include <map>
+#include <new>
+#include <numeric>
+#include <roaring/roaring.hh>
+#include <stdexcept>
+#include <string>
+#include <utility>
+
+#include "common/logging.h"
+#include "util/coding.h"
+
+namespace doris {
+
+class Roaring64MapSetBitForwardIterator;
+
+// serialized bitmap := TypeCode(1), Payload
+// The format of payload depends on value of TypeCode which is defined below
+struct BitmapTypeCode {
+    enum type {
+        // An empty bitmap. Payload is 0 byte.
+        // added in 0.11
+        EMPTY = 0,
+        // A bitmap containing only one element that is in [0, UINT32_MAX]
+        // Payload := UInt32LittleEndian(4 byte)
+        // added in 0.11
+        SINGLE32 = 1,
+        // A bitmap whose maximum element is in [0, UINT32_MAX]
+        // Payload := the standard RoaringBitmap format described by
+        // https://github.com/RoaringBitmap/RoaringFormatSpec/
+        // added in 0.11
+        BITMAP32 = 2,
+        // A bitmap containing only one element that is in (UINT32_MAX, UINT64_MAX]
+        // Payload := UInt64LittleEndian(8 byte)
+        // (added in 0.12)
+        SINGLE64 = 3,
+        // A bitmap whose maximum element is in (UINT32_MAX, UINT64_MAX].
+        //
+        // To support 64-bits elements, all elements with the same high 32 bits are stored in a
+        // RoaringBitmap containing only the lower 32 bits. Thus we could use
+        // map<uint32_t, RoaringBitmap> to represent bitmap of 64-bits ints.
+        //
+        // Since there is no standard format for 64-bits RoaringBitmap, we define our own as below
+        // Payload := NumRoaring(vint64), { MapKey, MapValue }^NumRoaring
+        // - MapKey := the shared high 32 bits in UInt32LittleEndian(4 byte)
+        // - MapValue := the standard RoaringBitmap format
+        //
+        // added in 0.12
+        BITMAP64 = 4
+    };
+};
+
+// Forked from https://github.com/RoaringBitmap/CRoaring/blob/v0.2.60/cpp/roaring64map.hh
+// The main differences are
+// - we use a custom serialized format (see BitmapTypeCode::BITMAP32 and BitmapTypeCode::BITMAP64)
+// - added clear() and is32BitsEnough() member functions
+class Roaring64Map {
 
 Review comment:
   > I think we would better move Roaring64Map to a single file.
   
   Yeah I've thought about it but failed to find a strong reason to do so. One motivation for a separate file is to spot changes we made via diff. However it turns out to not work because of code style formatting. So I think documenting what we've changed is enough for us to maintain, not to mention the class itself is really easy to understand :)
   
   > Add mark which method is different from roaring64map.hh
   
   Yeah, I've tried to do so. Maybe not clear enough, let me try to improve it.
   
   > Besides, If we decide to keep ourself Roaring64Map, we should porting the UT of Roaring64Map at the same time.
   
   Sure, will port the UT as well

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org