Skip to content

Commit

Permalink
Fixing review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@arundpanicker
arundpanicker committed Sep 17, 2023
1 parent 9b08171 commit 6da0ad9
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 54 deletions.
1 change: 0 additions & 1 deletion velox/docs/functions/presto/math.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ Mathematical Functions
Returns a cryptographically secure random value in the range 0.0 <= x < 1.0.

.. function:: secure_random(lower, upper) → [same as input]
:noindex:

Returns a cryptographically secure random value in the range lower <= x < upper, where lower < upper.

Expand Down
12 changes: 0 additions & 12 deletions velox/functions/prestosql/Rand.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,53 +66,41 @@ struct SecureRandFunction {

FOLLY_ALWAYS_INLINE void
call(double& out, const double lower, const double upper) {
VELOX_USER_CHECK_GE(lower, 0.0, "lower bound must be positive");
VELOX_USER_CHECK_GT(upper, 0.0, "upper bound must be positive");
VELOX_USER_CHECK_GT(
upper, lower, "upper bound must be greater than lower bound");
out = folly::Random::secureRandDouble(lower, upper);
}

FOLLY_ALWAYS_INLINE void
call(float& out, const float lower, const float upper) {
VELOX_USER_CHECK_GE(lower, 0.0, "lower bound must be positive");
VELOX_USER_CHECK_GT(upper, 0.0, "upper bound must be positive");
VELOX_USER_CHECK_GT(
upper, lower, "upper bound must be greater than lower bound");
out = float(folly::Random::secureRandDouble(lower, upper));
}

FOLLY_ALWAYS_INLINE void
call(int64_t& out, const int64_t lower, const int64_t upper) {
VELOX_USER_CHECK_GE(lower, 0, "lower bound must be positive");
VELOX_USER_CHECK_GT(upper, 0, "upper bound must be positive");
VELOX_USER_CHECK_GT(
upper, lower, "upper bound must be greater than lower bound");
out = folly::Random::secureRand64(lower, upper);
}

FOLLY_ALWAYS_INLINE void
call(int32_t& out, const int32_t lower, const int32_t upper) {
VELOX_USER_CHECK_GE(lower, 0, "lower bound must be positive");
VELOX_USER_CHECK_GT(upper, 0, "upper bound must be positive");
VELOX_USER_CHECK_GT(
upper, lower, "upper bound must be greater than lower bound");
out = folly::Random::secureRand32(lower, upper);
}

FOLLY_ALWAYS_INLINE void
call(int16_t& out, const int16_t lower, const int16_t upper) {
VELOX_USER_CHECK_GE(lower, 0, "lower bound must be positive");
VELOX_USER_CHECK_GT(upper, 0, "upper bound must be positive");
VELOX_USER_CHECK_GT(
upper, lower, "upper bound must be greater than lower bound");
out = int16_t(folly::Random::secureRand32(lower, upper));
}

FOLLY_ALWAYS_INLINE void
call(int8_t& out, const int8_t lower, const int8_t upper) {
VELOX_USER_CHECK_GE(lower, 0, "lower bound must be positive");
VELOX_USER_CHECK_GT(upper, 0, "upper bound must be positive");
VELOX_USER_CHECK_GT(
upper, lower, "upper bound must be greater than lower bound");
out = int8_t(folly::Random::secureRand32(lower, upper));
Expand Down
66 changes: 25 additions & 41 deletions velox/functions/prestosql/tests/RandTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class SecureRandTest : public functions::test::FunctionBaseTest {
};
};

TEST_F(SecureRandTest, zeroArgTest) {
TEST_F(SecureRandTest, zeroArg) {
auto result =
evaluateOnce<double>("secure_random()", makeRowVector(ROW({}), 1));
EXPECT_LT(result, 1.0);
Expand All @@ -93,18 +93,18 @@ TEST_F(SecureRandTest, zeroArgTest) {
EXPECT_GE(result, 0.0);
}

TEST_F(SecureRandTest, int64Test) {
TEST_F(SecureRandTest, int64Args) {
auto result =
secureRand<int64_t>((int64_t)2147532562, (int64_t)4611791058295013614);
secureRand<int64_t>((int64_t)-2147532562, (int64_t)4611791058295013614);
EXPECT_LT(result, 4611791058295013614);
EXPECT_GE(result, 2147532562);
EXPECT_GE(result, -2147532562);

result = secureRand<int64_t>((int64_t)0, (int64_t)46117910582950136);
EXPECT_LT(result, 46117910582950136);
EXPECT_GE(result, 0);
}

TEST_F(SecureRandTest, int32Test) {
TEST_F(SecureRandTest, int32Args) {
auto result = secureRand<int32_t>((int32_t)8765432, (int32_t)2145613151);
EXPECT_LT(result, 2145613151);
EXPECT_GE(result, 8765432);
Expand All @@ -114,17 +114,17 @@ TEST_F(SecureRandTest, int32Test) {
EXPECT_GE(result, 0);
}

TEST_F(SecureRandTest, int16Test) {
auto result = secureRand<int16_t>((int16_t)100, (int16_t)23286);
TEST_F(SecureRandTest, int16Args) {
auto result = secureRand<int16_t>((int16_t)-100, (int16_t)23286);
EXPECT_LT(result, 23286);
EXPECT_GE(result, 100);
EXPECT_GE(result, -100);

result = secureRand<int16_t>((int16_t)0, (int16_t)23286);
EXPECT_LT(result, 23286);
EXPECT_GE(result, 0);
}

TEST_F(SecureRandTest, int8Test) {
TEST_F(SecureRandTest, int8Args) {
auto result = secureRand<int8_t>((int8_t)10, (int8_t)120);
EXPECT_LT(result, 120);
EXPECT_GE(result, 10);
Expand All @@ -134,7 +134,7 @@ TEST_F(SecureRandTest, int8Test) {
EXPECT_GE(result, 0);
}

TEST_F(SecureRandTest, doubleTest) {
TEST_F(SecureRandTest, doubleArgs) {
auto result = secureRand<double>((double)10.5, (double)120.7895);
EXPECT_LT(result, 120.7895);
EXPECT_GE(result, 10.5);
Expand All @@ -144,10 +144,10 @@ TEST_F(SecureRandTest, doubleTest) {
EXPECT_GE(result, 0.0);
}

TEST_F(SecureRandTest, floatTest) {
auto result = secureRand<float>((float)10.5, (float)120.7);
TEST_F(SecureRandTest, floatArgs) {
auto result = secureRand<float>((float)-10.5, (float)120.7);
EXPECT_LT(result, 120.7);
EXPECT_GE(result, 10.5);
EXPECT_GE(result, -10.5);

result = secureRand<float>((float)0.0, (float)120.7);
EXPECT_LT(result, 120.7);
Expand All @@ -156,54 +156,38 @@ TEST_F(SecureRandTest, floatTest) {

TEST_F(SecureRandTest, invalidBounds) {
VELOX_ASSERT_THROW(
secureRand<int64_t>((int64_t)-5, (int64_t)10),
"lower bound must be positive");
VELOX_ASSERT_THROW(
secureRand<int64_t>((int64_t)0, (int64_t)-1),
"upper bound must be positive");
secureRand<int64_t>((int64_t)-5, (int64_t)-10),
"upper bound must be greater than lower bound");
VELOX_ASSERT_THROW(
secureRand<int64_t>((int64_t)15, (int64_t)10),
"upper bound must be greater than lower bound");
VELOX_ASSERT_THROW(
secureRand<int32_t>((int32_t)-5, (int32_t)10),
"lower bound must be positive");
VELOX_ASSERT_THROW(
secureRand<int32_t>((int32_t)0, (int32_t)-1),
"upper bound must be positive");
secureRand<int32_t>((int32_t)5, (int32_t)-10),
"upper bound must be greater than lower bound");
VELOX_ASSERT_THROW(
secureRand<int32_t>((int32_t)15, (int32_t)10),
"upper bound must be greater than lower bound");
VELOX_ASSERT_THROW(
secureRand<int16_t>((int16_t)-5, (int16_t)10),
"lower bound must be positive");
VELOX_ASSERT_THROW(
secureRand<int16_t>((int16_t)0, (int16_t)-1),
"upper bound must be positive");
secureRand<int16_t>((int16_t)-5, (int16_t)-10),
"upper bound must be greater than lower bound");
VELOX_ASSERT_THROW(
secureRand<int16_t>((int16_t)15, (int16_t)10),
"upper bound must be greater than lower bound");
VELOX_ASSERT_THROW(
secureRand<int8_t>((int8_t)-5, (int8_t)10),
"lower bound must be positive");
VELOX_ASSERT_THROW(
secureRand<int8_t>((int8_t)0, (int8_t)-1),
"upper bound must be positive");
secureRand<int8_t>((int8_t)5, (int8_t)-10),
"upper bound must be greater than lower bound");
VELOX_ASSERT_THROW(
secureRand<int8_t>((int8_t)15, (int8_t)10),
"upper bound must be greater than lower bound");
VELOX_ASSERT_THROW(
secureRand<double>(-5.7, 10.7), "lower bound must be positive");
VELOX_ASSERT_THROW(
secureRand<double>(0.0, -1.0), "upper bound must be positive");
secureRand<double>(-5.7, -10.7),
"upper bound must be greater than lower bound");
VELOX_ASSERT_THROW(
secureRand<double>(15.6, 10.1),
"upper bound must be greater than lower bound");
VELOX_ASSERT_THROW(
secureRand<float>((float)-5.7, (float)10.7),
"lower bound must be positive");
VELOX_ASSERT_THROW(
secureRand<float>((float)0.0, (float)-1.0),
"upper bound must be positive");
secureRand<float>((float)-5.7, (float)-10.7),
"upper bound must be greater than lower bound");
VELOX_ASSERT_THROW(
secureRand<float>((float)15.6, (float)10.1),
"upper bound must be greater than lower bound");
Expand Down

0 comments on commit 6da0ad9

Please sign in to comment.