proost commented on code in PR #543:
URL: https://github.com/apache/datasketches-cpp/pull/543#discussion_r4248084213
##########
theta/include/compact_theta_sketch_parser.hpp:
##########
@@ -41,6 +41,31 @@ class compact_theta_sketch_parser {
static void check_v4_entry_bits(uint8_t entry_bits);
static void check_v4_num_entries_bytes(uint8_t num_entries_bytes);
+ /**
+ * Checks the first 8 bytes of a serial version 3 compact sketch that has
the empty flag set.
+ * Accepts the current 8-byte form (preamble longs 1), and the legacy
24-byte form
+ * (preamble longs 3) written by Java before 1.0.0 for sketches with p < 1.
+ * The legacy form must also carry the expected seed hash.
+ * The caller must check that a legacy image is 24 bytes long and has no
entries.
+ * @param pre0 the first 8 bytes of the image, little-endian
+ * @param expected_seed_hash the seed hash computed from the expected seed
+ * @return the number of preamble longs: 1 or 3
+ * @throw std::invalid_argument if the image is not a valid empty compact
sketch
+ */
+ static uint8_t check_empty_v3(uint64_t pre0, uint16_t expected_seed_hash);
Review Comment:
For the code consistency, how to change signature to "void"?
##########
theta/include/compact_theta_sketch_parser.hpp:
##########
@@ -41,6 +41,31 @@ class compact_theta_sketch_parser {
static void check_v4_entry_bits(uint8_t entry_bits);
static void check_v4_num_entries_bytes(uint8_t num_entries_bytes);
+ /**
+ * Checks the first 8 bytes of a serial version 3 compact sketch that has
the empty flag set.
+ * Accepts the current 8-byte form (preamble longs 1), and the legacy
24-byte form
+ * (preamble longs 3) written by Java before 1.0.0 for sketches with p < 1.
+ * The legacy form must also carry the expected seed hash.
+ * The caller must check that a legacy image is 24 bytes long and has no
entries.
+ * @param pre0 the first 8 bytes of the image, little-endian
+ * @param expected_seed_hash the seed hash computed from the expected seed
+ * @return the number of preamble longs: 1 or 3
+ * @throw std::invalid_argument if the image is not a valid empty compact
sketch
+ */
+ static uint8_t check_empty_v3(uint64_t pre0, uint16_t expected_seed_hash);
+
+ // The MASK selects which bits of the first 8 bytes of an empty image are
examined.
+ // The TEST gives the required values of the examined bits; it must lie
within the MASK.
+ // Examined: preamble longs, serial version 3, sketch type 3, bytes 3 and 4
= 0 (not used by compact);
+ // flags read-only, empty, compact set; single-item and reserved bits 0, 6,
7 clear.
+ // The ordered flag is ignored: C++ before 3.3.0 and Java before 1.0.0 wrote
empty images without it.
+ // The reserved flag bits must be zero for serial version 3. Any future use
of them requires a new one.
+ // The seed hash is ignored in the 8-byte form: it may be 0 or the seed
hash, depending on the writer.
+ // These must stay the same as in Java EmptyCompactSketch.
+ static const uint64_t EMPTY_SKETCH_MASK = 0x0000EFFFFFFFFFFFULL;
Review Comment:
are there reasons not to be private?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
