[tarantool-patches] Re: [PATCH v1 1/4] box: introduce new helpers in column_mask.h

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

> 1. COLUMN_MASK_BIT will return 0, when >= 64. Please, fix it
> to return the last bit in such a case. Otherwise it is not
> COLUMN_MASK_BIT, but just <<.

fixed:
+ uint64_t mask = (uint64_t) 1 << (fieldno < 63 ? fieldno : 63);
+ return (column_mask & mask) != 0;

> 2. Not sure if it is worth macrosing, because anyway column
> mask every where is uint64_t. If we want to hide a size,
> we should introduce column_mask_t type.
I also think so now. Let's introduce such macro in sqlInt with Bitmask.

So we have new helpers column_mask_is_overflowed, column_mask_fieldno_is_set,
column_mask32_set_fieldno, column_mask32_fieldno_is_set,
column_mask32_is_overflowed.

==================================

Refactored column_mask.h definitions: introduced a new routine
column_mask_is_overflowed, column_mask_is_set. Also a couple of
routines for 32-bit bitmask.
We need this helpers in further refactoring.

Needed for #3571
---
 src/box/column_mask.h        | 66 ++++++++++++++++++++++++++++++++++++
 test/unit/column_mask.c      | 33 +++++++++++++++++-
 test/unit/column_mask.result | 10 +++++-
 3 files changed, 107 insertions(+), 2 deletions(-)

diff --git a/src/box/column_mask.h b/src/box/column_mask.h
index d71911d46..f447d8386 100644
--- a/src/box/column_mask.h
+++ b/src/box/column_mask.h
@@ -94,6 +94,31 @@ column_mask_set_range(uint64_t *column_mask, uint32_t first_fieldno_in_range)
 	}
 }
 
+/**
+ * Test if overflow flag set in bitmask.
+ * @param column_mask Mask to test.
+ * @retval true If mask overflowed, false otherwise.
+ */
+static inline bool
+column_mask_is_overflowed(uint64_t column_mask)
+{
+	return column_mask & ((uint64_t)1 << 63);
+}
+
+/**
+ * Test a bit in the bitmask corresponding to a column fieldno.
+ * @param column_mask Mask to test.
+ * @param fieldno Fieldno number to test (index base must be 0).
+ * @retval true If bit corresponding to a column fieldno.
+ * @retval false if bit is not set or fieldno > 64.
+ */
+static inline bool
+column_mask_fieldno_is_set(uint64_t column_mask, uint32_t fieldno)
+{
+	uint64_t mask = (uint64_t) 1 << (fieldno < 63 ? fieldno : 63);
+	return (column_mask & mask) != 0;
+}
+
 /**
  * True if the update operation does not change the key.
  * @param key_mask Key mask.
@@ -109,4 +134,45 @@ key_update_can_be_skipped(uint64_t key_mask, uint64_t update_mask)
 	return (key_mask & update_mask) == 0;
 }
 
+/**
+ * Set a bit in the 32-bit bitmask corresponding to a single
+ * changed column.
+ * @param column_mask Mask to update.
+ * @param fieldno     Updated fieldno (index base must be 0).
+ */
+static inline void
+column_mask32_set_fieldno(uint32_t *column_mask, uint32_t fieldno)
+{
+	if (fieldno >= 31)
+		*column_mask |= ((uint32_t) 1) << 31;
+	else
+		*column_mask |= ((uint32_t) 1) << fieldno;
+}
+
+/**
+ * Test a bit in the 32-bit bitmask corresponding to a column
+ * fieldno.
+ * @param column_mask Mask to test.
+ * @param fieldno Fieldno number to test (index base must be 0).
+ * @retval true If bit corresponding to a column fieldno.
+ * @retval false if bit is not set or fieldno > 64.
+ */
+static inline bool
+column_mask32_fieldno_is_set(uint32_t column_mask, uint32_t fieldno)
+{
+	uint32_t mask = (uint32_t) 1 << (fieldno < 31 ? fieldno : 31);
+	return (column_mask & mask) != 0;
+}
+
+/**
+ * Test if overflow flag set in 32-bit bitmask.
+ * @param column_mask Mask to test.
+ * @retval true If mask overflowed, false otherwise.
+ */
+static inline bool
+column_mask32_is_overflowed(uint32_t column_mask)
+{
+	return column_mask & ((uint32_t) 1 << 31);
+}
+
 #endif
diff --git a/test/unit/column_mask.c b/test/unit/column_mask.c
index 7859626f8..0d5c04fd4 100644
--- a/test/unit/column_mask.c
+++ b/test/unit/column_mask.c
@@ -236,13 +236,44 @@ basic_test()
 				    column_masks[i]);
 }
 
+static inline void
+helpers_test()
+{
+	uint64_t bitmask = 0;
+	column_mask_set_fieldno(&bitmask, 33);
+	is(column_mask_fieldno_is_set(bitmask, 33), true,
+	   "64 bitmask internal bit");
+	is(column_mask_is_overflowed(bitmask), false,
+	   "64 bitmask is not overflowed");
+
+	column_mask_set_fieldno(&bitmask, 74);
+	is(column_mask_fieldno_is_set(bitmask, 74), true,
+	   "64 bitmask external bit");
+	is(column_mask_is_overflowed(bitmask), true,
+	   "64 bitmask is overflowed");
+
+	uint32_t bitmask32 = 0;
+	column_mask32_set_fieldno(&bitmask32, 23);
+	is(column_mask32_fieldno_is_set(bitmask32, 23), true,
+	   "32 bitmask internal bit");
+	is(column_mask32_is_overflowed(bitmask32), false,
+	   "32 bitmask is not overflowed");
+
+	column_mask32_set_fieldno(&bitmask32, 54);
+	is(column_mask32_fieldno_is_set(bitmask32, 54), true,
+	   "32 bitmask external bit");
+	is(column_mask32_is_overflowed(bitmask32), true,
+	   "32 bitmask is overflowed");
+}
+
 int
 main()
 {
 	header();
-	plan(27);
+	plan(35);
 
 	basic_test();
+	helpers_test();
 
 	footer();
 	check_plan();
diff --git a/test/unit/column_mask.result b/test/unit/column_mask.result
index 9309e6cdc..91d1f1feb 100644
--- a/test/unit/column_mask.result
+++ b/test/unit/column_mask.result
@@ -1,5 +1,5 @@
 	*** main ***
-1..27
+1..35
 ok 1 - check result length
 ok 2 - tuple update is correct
 ok 3 - column_mask is correct
@@ -27,4 +27,12 @@ ok 24 - column_mask is correct
 ok 25 - check result length
 ok 26 - tuple update is correct
 ok 27 - column_mask is correct
+ok 28 - 64 bitmask internal bit
+ok 29 - 64 bitmask is not overflowed
+ok 30 - 64 bitmask external bit
+ok 31 - 64 bitmask is overflowed
+ok 32 - 32 bitmask internal bit
+ok 33 - 32 bitmask is not overflowed
+ok 34 - 32 bitmask external bit
+ok 35 - 32 bitmask is overflowed
 	*** main: done ***
-- 
2.20.1