Dandandan commented on a change in pull request #1095:
URL: https://github.com/apache/arrow-datafusion/pull/1095#discussion_r726308905



##########
File path: datafusion/src/physical_plan/hyperloglog/mod.rs
##########
@@ -0,0 +1,295 @@
+// 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.
+
+//! # HyperLogLog
+//!
+//! `hyperloglog` is a module that contains a modified version
+//! of [redis's 
implementation](https://github.com/redis/redis/blob/4930d19e70c391750479951022e207e19111eb55/src/hyperloglog.c)
+//! with some modification based on strong assumption of usage
+//! within datafusion, so that [`approx_distinct`] function can
+//! be efficiently implemented.
+//!
+//! Specifically, like Redis's version, this HLL structure uses
+//! 2**14 = 16384 registers, which means the standard error is
+//! 1.04/(16384**0.5) = 0.8125%. Unlike Redis, the register takes
+//! up full [`u8`] size instead of a raw int* and thus saves some
+//! tricky bit shifting techniques used in the original version.
+//! This results in a memory usage increase from 12Kib to 16Kib.
+//! Also only the dense version is adopted, so there's no automatic
+//! conversion, largely to simplify the code.
+//!
+//! This module also borrows some code structure from 
[pdatastructs.rs](https://github.com/crepererum/pdatastructs.rs/blob/3997ed50f6b6871c9e53c4c5e0f48f431405fc63/src/hyperloglog.rs).
+
+// TODO remove this when hooked up with the rest
+#![allow(dead_code)]
+
+use ahash::AHasher;
+use std::hash::{Hash, Hasher};
+use std::marker::PhantomData;
+
+/// The greater is P, the smaller the error.
+const HLL_P: usize = 14_usize;
+/// the number of bits of the hash value used determining the number of 
leading zeros
+const HLL_Q: usize = 64_usize - HLL_P;
+const NUM_REGISTERS: usize = 1_usize << HLL_P;
+/// mask to obtain index into the registers
+const HLL_P_MASK: u64 = (NUM_REGISTERS as u64) - 1;
+
+#[derive(Clone, Debug)]
+pub(crate) struct HyperLogLog<T>
+where
+    T: Hash + ?Sized,
+{
+    registers: [u8; NUM_REGISTERS],
+    phantom: PhantomData<T>,
+}
+
+impl<T> HyperLogLog<T>
+where
+    T: Hash + ?Sized,
+{
+    /// Creates a new, empty HyperLogLog.
+    pub fn new() -> Self {
+        let registers = [0; NUM_REGISTERS];
+        Self {
+            registers,
+            phantom: PhantomData,
+        }
+    }
+
+    /// choice of hash function: ahash is already an dependency
+    /// and it fits the requirements of being a 64bit hash with
+    /// reasonable performance.
+    #[inline]
+    fn hash_value(&self, obj: &T) -> u64 {
+        let mut hasher = AHasher::default();

Review comment:
       For now maybe a fixed `randomstate` would be OK-ish? In the join / 
aggregate algorithms we are also doing this. @houqp do you have an opinion on 
this? I guess usages like ROAPI have a higher chance of being vulnerable to DoS 
attacks by exposing an API to the end user. We should do something like what 
@crepererum suggests to make it work for e.g. ballista.
   There are of course more parts of the query engine you could abuse for DoS 
attacks, like generating cross joins, having a very large number of columns, 
etc. so maybe it makes more sense to spend some time in being able to e.g. 
specify and handle timeouts for query execution.




-- 
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: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to