From 6ba08bc5bdcd4f41899a0fc7c8a60cbc3aed1102 Mon Sep 17 00:00:00 2001 From: James Cohan Date: Tue, 4 Aug 2015 13:28:16 -0400 Subject: [PATCH] Revert "SERVER-18427 Adds $log, $log10, $ln, $pow, and $exp aggregation expressions" This reverts commit f0541cc1f7f9cf1987aaf392dcb2328f71d6d232. --- jstests/aggregation/bugs/server18427.js | 111 ----------- src/mongo/db/pipeline/expression.cpp | 249 ------------------------ src/mongo/db/pipeline/expression.h | 26 --- 3 files changed, 386 deletions(-) delete mode 100644 jstests/aggregation/bugs/server18427.js diff --git a/jstests/aggregation/bugs/server18427.js b/jstests/aggregation/bugs/server18427.js deleted file mode 100644 index 46b5b8345a0c9..0000000000000 --- a/jstests/aggregation/bugs/server18427.js +++ /dev/null @@ -1,111 +0,0 @@ -// SERVER-18427: Add $log, $log10, $ln, $pow, and $exp aggregation expressions. - -// For assertErrorCode. -load('jstests/aggregation/extras/utils.js'); - -(function() { - 'use strict'; - var coll = db.log_exponential_expressions; - coll.drop(); - assert.writeOK(coll.insert({_id: 0})); - - // Helper for testing that op returns expResult. - function testOp(op, expResult) { - var pipeline = [{$project: {_id: 0, result: op}}]; - assert.eq(coll.aggregate(pipeline).toArray(), [{result: expResult}]); - } - - // $log, $log10, $ln. - - // Valid input: numeric/null/NaN, base positive and not equal to 1, arg positive. - testOp({$log: [10, 10]}, 1); - testOp({$log10: [10]}, 1); - testOp({$ln: [Math.E]}, 1); - // All types converted to doubles. - testOp({$log: [NumberLong("10"), NumberLong("10")]}, 1); - testOp({$log10: [NumberLong("10")]}, 1); - testOp({$ln: [NumberLong("1")]}, 0); - // LLONG_MAX is converted to a double. - testOp({$log: [NumberLong("9223372036854775807"), 10]}, 18.964889726830812); - // Null inputs result in null. - testOp({$log: [null, 10]}, null); - testOp({$log: [10, null]}, null); - testOp({$log10: [null]}, null); - testOp({$ln: [null]}, null); - // NaN inputs result in NaN. - testOp({$log: [NaN, 10]}, NaN); - testOp({$log: [10, NaN]}, NaN); - testOp({$log10: [NaN]}, NaN); - testOp({$ln: [NaN]}, NaN); - - // Invalid input: non-numeric/non-null, bases not positive or equal to 1, args not positive. - - // Args/bases must be numeric or null. - assertErrorCode(coll, [{$project: {log: {$log: ["string", 5]}}}], 28756); - assertErrorCode(coll, [{$project: {log: {$log: [5, "string"]}}}], 28757); - assertErrorCode(coll, [{$project: {log10: {$log10: ["string"]}}}], 28760); - assertErrorCode(coll, [{$project: {ln: {$ln: ["string"]}}}], 28754); - // Args/bases cannot equal 0. - assertErrorCode(coll, [{$project: {log: {$log: [0, 5]}}}], 28758); - assertErrorCode(coll, [{$project: {log: {$log: [5, 0]}}}], 28759); - assertErrorCode(coll, [{$project: {log10: {$log10: [0]}}}], 28761); - assertErrorCode(coll, [{$project: {ln: {$ln: [0]}}}], 28755); - // Args/bases cannot be negative. - assertErrorCode(coll, [{$project: {log: {$log: [-1, 5]}}}], 28758); - assertErrorCode(coll, [{$project: {log: {$log: [5, -1]}}}], 28759); - assertErrorCode(coll, [{$project: {log10: {$log10: [-1]}}}], 28761); - assertErrorCode(coll, [{$project: {ln: {$ln: [-1]}}}], 28755); - // Base can't equal 1. - assertErrorCode(coll, [{$project: {log: {$log: [5, 1]}}}], 28759); - - // $pow, $exp. - - // Valid input - numeric/null/NaN. - - // $pow -- if either input is a double return a double. - testOp({$pow: [10, 2]}, 100); - testOp({$pow: [1/2, -1]}, 2); - testOp({$pow: [-2, 2]}, 4); - testOp({$pow: [NumberInt("2"), 2]}, 4); - testOp({$pow: [-2, NumberInt("2")]}, 4); - - // If exponent is negative and base not -1, 0, or 1, return a double. - testOp({$pow: [NumberLong("2"), NumberLong("-1")]}, 1/2); - testOp({$pow: [NumberInt("4"), NumberInt("-1")]}, 1/4); - testOp({$pow: [NumberInt("4"), NumberLong("-1")]}, 1/4); - testOp({$pow: [NumberInt("1"), NumberLong("-2")]}, NumberLong("1")); - testOp({$pow: [NumberInt("-1"), NumberLong("-2")]}, NumberLong("1")); - - // If result would overflow a long, return a double. - testOp({$pow: [NumberInt("2"), NumberLong("63")]}, 9223372036854776000); - - // Result would be incorrect if double were returned. - testOp({$pow: [NumberInt("3"), NumberInt("35")]}, NumberLong("50031545098999707")); - - // Else if either input is a long, return a long. - testOp({$pow: [NumberInt("-2"), NumberLong("63")]}, NumberLong("-9223372036854775808")); - testOp({$pow: [NumberInt("4"), NumberLong("2")]}, NumberLong("16")); - testOp({$pow: [NumberLong("4"), NumberInt("2")]}, NumberLong("16")); - testOp({$pow: [NumberLong("4"), NumberLong("2")]}, NumberLong("16")); - - // Else return an int if it fits. - testOp({$pow: [NumberInt("4"), NumberInt("2")]}, 16); - - // $exp always returns doubles, since e is a double. - testOp({$exp: [NumberInt("-1")]}, 1/Math.E); - testOp({$exp: [NumberLong("1")]}, Math.E); - // Null input results in null. - testOp({$pow: [null, 2]}, null); - testOp({$pow: [1/2, null]}, null); - testOp({$exp: [null]}, null); - // NaN input results in NaN. - testOp({$pow: [NaN, 2]}, NaN); - testOp({$pow: [1/2, NaN]}, NaN); - testOp({$exp: [NaN]}, NaN); - - // Invalid inputs - non-numeric/non-null types, or 0 to a negative exponent. - assertErrorCode(coll, [{$project: {pow: {$pow: [0, NumberLong("-1")]}}}], 28764); - assertErrorCode(coll, [{$project: {pow: {$pow: ["string", 5]}}}], 28762); - assertErrorCode(coll, [{$project: {pow: {$pow: [5, "string"]}}}], 28763); - assertErrorCode(coll, [{$project: {exp: {$exp: ["string"]}}}], 28765); -}()); \ No newline at end of file diff --git a/src/mongo/db/pipeline/expression.cpp b/src/mongo/db/pipeline/expression.cpp index db3443362fe82..88a471d07922f 100644 --- a/src/mongo/db/pipeline/expression.cpp +++ b/src/mongo/db/pipeline/expression.cpp @@ -1154,26 +1154,6 @@ const char* ExpressionDivide::getOpName() const { return "$divide"; } -/* ----------------------- ExpressionExp ---------------------------- */ - -Value ExpressionExp::evaluateInternal(Variables* vars) const { - Value expVal = vpOperand[0]->evaluateInternal(vars); - if (expVal.nullish()) - return Value(BSONNULL); - - uassert(28765, - str::stream() << "$exp only supports numeric types, not " << typeName(expVal.getType()), - expVal.numeric()); - - // exp() always returns a double since e is a double. - return Value(exp(expVal.coerceToDouble())); -} - -REGISTER_EXPRESSION(exp, ExpressionExp::parse); -const char* ExpressionExp::getOpName() const { - return "$exp"; -} - /* ---------------------- ExpressionObject --------------------------- */ intrusive_ptr ExpressionObject::create() { @@ -2073,85 +2053,6 @@ const char* ExpressionIfNull::getOpName() const { return "$ifNull"; } -/* ----------------------- ExpressionLn ---------------------------- */ - -Value ExpressionLn::evaluateInternal(Variables* vars) const { - Value argVal = vpOperand[0]->evaluateInternal(vars); - if (argVal.nullish()) - return Value(BSONNULL); - - uassert(28754, - str::stream() << "$ln only supports numeric types, not " << typeName(argVal.getType()), - argVal.numeric()); - - double argDouble = argVal.coerceToDouble(); - uassert(28755, - str::stream() << "$ln's argument must be a positive number, but is " << argDouble, - argDouble > 0 || std::isnan(argDouble)); - return Value(std::log(argDouble)); -} - -REGISTER_EXPRESSION(ln, ExpressionLn::parse); -const char* ExpressionLn::getOpName() const { - return "$ln"; -} - -/* ----------------------- ExpressionLog ---------------------------- */ - -Value ExpressionLog::evaluateInternal(Variables* vars) const { - Value argVal = vpOperand[0]->evaluateInternal(vars); - Value baseVal = vpOperand[1]->evaluateInternal(vars); - if (argVal.nullish() || baseVal.nullish()) - return Value(BSONNULL); - - uassert(28756, - str::stream() << "$log's argument must be numeric, not " << typeName(argVal.getType()), - argVal.numeric()); - uassert(28757, - str::stream() << "$log's base must be numeric, not " << typeName(baseVal.getType()), - baseVal.numeric()); - - double argDouble = argVal.coerceToDouble(); - double baseDouble = baseVal.coerceToDouble(); - uassert(28758, - str::stream() << "$log's argument must be a positive number, but is " << argDouble, - argDouble > 0 || std::isnan(argDouble)); - uassert(28759, - str::stream() << "$log's base must be a positive number not equal to 1, but is " - << baseDouble, - (baseDouble > 0 && baseDouble != 1) || std::isnan(baseDouble)); - return Value(std::log(argDouble) / std::log(baseDouble)); -} - -REGISTER_EXPRESSION(log, ExpressionLog::parse); -const char* ExpressionLog::getOpName() const { - return "$log"; -} - -/* ----------------------- ExpressionLog10 ---------------------------- */ - -Value ExpressionLog10::evaluateInternal(Variables* vars) const { - Value argVal = vpOperand[0]->evaluateInternal(vars); - if (argVal.nullish()) - return Value(BSONNULL); - - uassert(28760, - str::stream() << "$log10 only supports numeric types, not " - << typeName(argVal.getType()), - argVal.numeric()); - - double argDouble = argVal.coerceToDouble(); - uassert(28761, - str::stream() << "$log10's argument must be a positive number, but is " << argDouble, - argDouble > 0 || std::isnan(argDouble)); - return Value(std::log10(argDouble)); -} - -REGISTER_EXPRESSION(log10, ExpressionLog10::parse); -const char* ExpressionLog10::getOpName() const { - return "$log10"; -} - /* ------------------------ ExpressionNary ----------------------------- */ intrusive_ptr ExpressionNary::optimize() { @@ -2327,156 +2228,6 @@ const char* ExpressionOr::getOpName() const { return "$or"; } -/* ----------------------- ExpressionPow ---------------------------- */ - -Value ExpressionPow::evaluateInternal(Variables* vars) const { - Value baseVal = vpOperand[0]->evaluateInternal(vars); - Value expVal = vpOperand[1]->evaluateInternal(vars); - if (baseVal.nullish() || expVal.nullish()) - return Value(BSONNULL); - - BSONType baseType = baseVal.getType(); - BSONType expType = expVal.getType(); - - uassert(28762, - str::stream() << "$pow's base must be numeric, not " << typeName(baseType), - baseVal.numeric()); - uassert(28763, - str::stream() << "$pow's exponent must be numeric, not " << typeName(expType), - expVal.numeric()); - - // pow() will cast args to doubles. - double baseNum = baseVal.coerceToDouble(); - double expNum = expVal.coerceToDouble(); - - uassert(28764, - "$pow cannot take a base of 0 and a negative exponent", - !(baseNum == 0 && expNum < 0)); - - // If either number is a double, return a double. - if (baseType == NumberDouble || expType == NumberDouble) { - return Value(pow(baseNum, expNum)); - } - - // base and exp are both integers. - - auto representableAsLong = [](long long base, long long exp) { - // If exp is greater than 63 and base is not -1, 0, or 1, the result will overflow. - // If exp is negative and the base is not -1 or 1, the result will be fractional. - if (exp < 0 || exp > 63) { - return std::abs(base) == 1 || base == 0; - } - - struct MinMax { - long long min; - long long max; - }; - - // Array indices correspond to exponents 0 through 63. The values in each index are the min - // and max bases, respectively, that can be raised to that exponent without overflowing a - // 64-bit int. For max bases, this was computed by solving for b in - // b = (2^63-1)^(1/exp) for exp = [0, 63] and truncating b. To calculate min bases, for even - // exps the equation used was b = (2^63-1)^(1/exp), and for odd exps the equation used was - // b = (-2^63)^(1/exp). Since the magnitude of long min is greater than long max, the - // magnitude of some of the min bases raised to odd exps is greater than the corresponding - // max bases raised to the same exponents. - - static const MinMax kBaseLimits[] = { - {std::numeric_limits::min(), std::numeric_limits::max()}, // 0 - {std::numeric_limits::min(), std::numeric_limits::max()}, - {-3037000499, 3037000499}, - {-2097152, 2097151}, - {-55108, 55108}, - {-6208, 6208}, - {-1448, 1448}, - {-512, 511}, - {-234, 234}, - {-128, 127}, - {-78, 78}, // 10 - {-52, 52}, - {-38, 38}, - {-28, 28}, - {-22, 22}, - {-18, 18}, - {-15, 15}, - {-13, 13}, - {-11, 11}, - {-9, 9}, - {-8, 8}, // 20 - {-8, 7}, - {-7, 7}, - {-6, 6}, - {-6, 6}, - {-5, 5}, - {-5, 5}, - {-5, 5}, - {-4, 4}, - {-4, 4}, - {-4, 4}, // 30 - {-4, 4}, - {-3, 3}, - {-3, 3}, - {-3, 3}, - {-3, 3}, - {-3, 3}, - {-3, 3}, - {-3, 3}, - {-3, 3}, - {-2, 2}, // 40 - {-2, 2}, - {-2, 2}, - {-2, 2}, - {-2, 2}, - {-2, 2}, - {-2, 2}, - {-2, 2}, - {-2, 2}, - {-2, 2}, - {-2, 2}, // 50 - {-2, 2}, - {-2, 2}, - {-2, 2}, - {-2, 2}, - {-2, 2}, - {-2, 2}, - {-2, 2}, - {-2, 2}, - {-2, 2}, - {-2, 2}, // 60 - {-2, 2}, - {-2, 2}, - {-2, 1}}; - - return base >= kBaseLimits[exp].min && base <= kBaseLimits[exp].max; - }; - - // If the result cannot be represented as a long, return a double. Otherwise if either number - // is a long, return a long. If both numbers are ints, then return an int if the result fits or - // a long if it is too big. - if (!representableAsLong(baseNum, expNum)) { - return Value(pow(baseNum, expNum)); - } - - long long baseLong = baseVal.getLong(); - long long expLong = expVal.getLong(); - long long result = 1; - // Use repeated multiplication, since pow() casts args to doubles which could result in loss of - // precision if arguments are very large. - for (int i = 0; i < expLong; i++) { - result *= baseLong; - } - - if (baseType == NumberLong || expType == NumberLong) { - return Value(result); - } - return Value::createIntOrLong(result); -} - -REGISTER_EXPRESSION(pow, ExpressionPow::parse); -const char* ExpressionPow::getOpName() const { - return "$pow"; -} - /* ------------------------- ExpressionSecond ----------------------------- */ Value ExpressionSecond::evaluateInternal(Variables* vars) const { diff --git a/src/mongo/db/pipeline/expression.h b/src/mongo/db/pipeline/expression.h index 5113c104686af..b4caa27ae52ef 100644 --- a/src/mongo/db/pipeline/expression.h +++ b/src/mongo/db/pipeline/expression.h @@ -639,12 +639,6 @@ class ExpressionDivide final : public ExpressionFixedArity }; -class ExpressionExp final : public ExpressionFixedArity { - Value evaluateInternal(Variables* vars) const final; - const char* getOpName() const final; -}; - - class ExpressionFieldPath final : public Expression { public: boost::intrusive_ptr optimize() final; @@ -772,21 +766,6 @@ class ExpressionLet final : public Expression { boost::intrusive_ptr _subExpression; }; -class ExpressionLn final : public ExpressionFixedArity { - Value evaluateInternal(Variables* vars) const final; - const char* getOpName() const final; -}; - -class ExpressionLog final : public ExpressionFixedArity { - Value evaluateInternal(Variables* vars) const final; - const char* getOpName() const final; -}; - -class ExpressionLog10 final : public ExpressionFixedArity { - Value evaluateInternal(Variables* vars) const final; - const char* getOpName() const final; -}; - class ExpressionMap final : public Expression { public: boost::intrusive_ptr optimize() final; @@ -1002,11 +981,6 @@ class ExpressionOr final : public ExpressionVariadic { } }; -class ExpressionPow final : public ExpressionFixedArity { - Value evaluateInternal(Variables* vars) const final; - const char* getOpName() const final; -}; - class ExpressionSecond final : public ExpressionFixedArity { public: