Copilot commented on code in PR #17208:
URL: https://github.com/apache/pinot/pull/17208#discussion_r2525740593
##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/ArithmeticFunctions.java:
##########
@@ -201,6 +204,31 @@ public static double truncate(double a) {
return Math.signum(a) * Math.floor(Math.abs(a));
}
+ @ScalarFunction
+ public static double rand() {
+ return ThreadLocalRandom.current().nextDouble();
+ }
+
+ @ScalarFunction
+ public static double rand(long seed) {
+ return deterministicRand(seed);
+ }
+
+ private static double deterministicRand(long seed) {
+ // The XOR with 0x5DEECE66DL is inspired by java.util.Random's initial LCG
multiplier scramble,
+ // but unlike Random, we use a custom non-linear mix64 routine for
diffusion and reproducible output.
+ // This approach is chosen to provide deterministic, well-diffused
pseudo-random values, not to match Random's output.
+ // Reference for multiplier:
https://docs.oracle.com/javase/8/docs/api/java/util/Random.html
+ long mixed = mix64(seed ^ 0x5DEECE66DL);
Review Comment:
The comment explains the 'why' of using XOR with 0x5DEECE66DL and the custom
mix64 routine, but it should also briefly explain why we right-shift by 11 bits
(to extract the top 53 bits for IEEE 754 double precision mantissa). This would
improve clarity for future maintainers.
```suggestion
long mixed = mix64(seed ^ 0x5DEECE66DL);
// Right-shift by 11 bits to extract the top 53 bits for use as the
mantissa of an IEEE 754 double-precision value.
// This produces a double in [0,1) with full precision.
```
##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/ArithmeticFunctions.java:
##########
@@ -201,6 +204,33 @@ public static double truncate(double a) {
return Math.signum(a) * Math.floor(Math.abs(a));
}
+ @ScalarFunction
+ public static double rand() {
+ return ThreadLocalRandom.current().nextDouble();
+ }
+
+ @ScalarFunction
+ public static double rand(long seed) {
+ return deterministicRand(seed);
+ }
+
+ private static double deterministicRand(long seed) {
+ // The XOR with 0x5DEECE66DL is inspired by java.util.Random's initial LCG
multiplier scramble,
+ // but unlike Random, we use a custom non-linear mix64 routine for
diffusion and reproducible output.
+ // This approach is chosen to provide deterministic, well-diffused
pseudo-random values, not to match Random's output.
+ // Reference for multiplier:
https://docs.oracle.com/javase/8/docs/api/java/util/Random.html
+ long mixed = mix64(seed ^ 0x5DEECE66DL);
+ // Right-shift by 11 bits to extract the top 53 bits for use as the
mantissa of an IEEE 754 double-precision value.
+ // This produces a double in [0,1) with full precision.
+ return (mixed >>> 11) * DOUBLE_UNIT;
+ }
+
Review Comment:
[nitpick] The `rand(long seed)` method is a thin wrapper around
`deterministicRand`. Consider inlining the logic to reduce unnecessary
indirection and improve code clarity.
```suggestion
```
--
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]
