Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[CALCITE-6325] Add LOG function (enabled in Mysql and Spark library) #3732

Closed
wants to merge 7 commits into from
Closed

[CALCITE-6325] Add LOG function (enabled in Mysql and Spark library) #3732

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4487985
[CALCITE-6325] Add LOG function (enabled in Mysql, Spark library)
caicancai Mar 1, 2024
719dc85
[CALCITE-6325] Add LOG function (enabled in Mysql, Spark library)
caicancai Mar 14, 2024
4b946b9
[CALCITE-6325] Optimize LogImplementor
caicancai Mar 15, 2024
adcbe2e
[CALCITE-6325] Add LOG function (enabled in Mysql library)
caicancai Mar 19, 2024
22a8767
[CALCITE-6325] Modify to MYSQL unified style
caicancai Mar 25, 2024
3a5f076
[CALCITE-6325] Add LOG function (enabled in Mysql Style library)
caicancai Apr 1, 2024
a21896b
[CALCITE-6325] Add LOG function (enabled in Mysql and Spark library)
caicancai Apr 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 15 additions & 9 deletions core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@
import static org.apache.calcite.sql.fun.SqlLibraryOperators.LOG2;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.LOGICAL_AND;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.LOGICAL_OR;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.LOG_MYSQL_SPARK;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.LPAD;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.MAP;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.MAP_CONCAT;
Expand Down Expand Up @@ -642,11 +643,13 @@ Builder populate() {
defineMethod(EXP, BuiltInMethod.EXP.method, NullPolicy.STRICT);
defineMethod(POWER, BuiltInMethod.POWER.method, NullPolicy.STRICT);
defineMethod(ABS, BuiltInMethod.ABS.method, NullPolicy.STRICT);
defineMethod(LOG2, BuiltInMethod.LOG2.method, NullPolicy.STRICT);

map.put(LN, new LogImplementor());
map.put(LOG, new LogImplementor());
map.put(LOG10, new LogImplementor());
map.put(LN, new LogImplementor(BuiltInMethod.LOG.method));
map.put(LOG, new LogImplementor(BuiltInMethod.LOG.method));
map.put(LOG10, new LogImplementor(BuiltInMethod.LOG.method));

caicancai marked this conversation as resolved.
Show resolved Hide resolved
map.put(LOG_MYSQL_SPARK, new LogImplementor(BuiltInMethod.LOG_MYSQL_SPARK.method));
map.put(LOG2, new LogImplementor(BuiltInMethod.LOG_MYSQL_SPARK.method));

defineReflective(RAND, BuiltInMethod.RAND.method,
BuiltInMethod.RAND_SEED.method);
Expand Down Expand Up @@ -4127,14 +4130,14 @@ private static class LogicalNotImplementor extends AbstractRexCallImplementor {
* <p>Handles all logarithm functions using log rules to determine the
* appropriate base (i.e. base e for LN).
*/
private static class LogImplementor extends AbstractRexCallImplementor {
LogImplementor() {
super("log", NullPolicy.STRICT, true);
private static class LogImplementor extends MethodImplementor {
LogImplementor(Method method) {
super(method, NullPolicy.STRICT, true);
caicancai marked this conversation as resolved.
Show resolved Hide resolved
}

@Override Expression implementSafe(final RexToLixTranslator translator,
final RexCall call, final List<Expression> argValueList) {
return Expressions.call(BuiltInMethod.LOG.method, args(call, argValueList));
return Expressions.call(method, args(call, argValueList));
}

private static List<Expression> args(RexCall call,
Expand All @@ -4145,10 +4148,13 @@ private static List<Expression> args(RexCall call,
case "LOG":
if (argValueList.size() == 2) {
caicancai marked this conversation as resolved.
Show resolved Hide resolved
return list.append(argValueList.get(1));
} else {
return list.append(Expressions.constant(Math.exp(1)));
}
// fall through
case "LN":
return list.append(Expressions.constant(Math.exp(1)));
case "LOG2":
return list.append(Expressions.constant(2));
case "LOG10":
return list.append(Expressions.constant(BigDecimal.TEN));
default:
Expand Down
25 changes: 18 additions & 7 deletions core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2788,15 +2788,26 @@ public static double log(BigDecimal d0, BigDecimal d1) {
return Math.log(d0.doubleValue()) / Math.log(d1.doubleValue());
}

/** SQL {@code LOG2(number)} function applied to double values. */
public static @Nullable Double log2(double number) {
return (number <= 0) ? null : log(number, 2);
/** SQL {@code LOGMySqlSpark(number, number2)} function applied to double values. */
caicancai marked this conversation as resolved.
Show resolved Hide resolved
public static @Nullable Double logMysqlSpark(double number, double number2) {
return (number <= 0) ? null : log(number, number2);
}

/** SQL {@code LOG2(number)} function applied to
* BigDecimal values. */
public static @Nullable Double log2(BigDecimal number) {
return log2(number.doubleValue());
/** SQL {@code LOGMySqlSpark(number, number2)} function applied to
* double and BigDecimal values. */
public static @Nullable Double logMysqlSpark(double number, BigDecimal number2) {
return logMysqlSpark(number, number2.doubleValue());
}

/** SQL {@code LOGMySqlSpark(number, number2)} function applied to
* BigDecimal and double values. */
public static @Nullable Double logMysqlSpark(BigDecimal number, double number2) {
return logMysqlSpark(number.doubleValue(), number2);
}

/** SQL {@code LOGMySqlSpark(number, number2)} function applied to double values. */
public static @Nullable Double logMysqlSpark(BigDecimal number, BigDecimal number2) {
return logMysqlSpark(number.doubleValue(), number2.doubleValue());
}

// MOD
Expand Down
8 changes: 8 additions & 0 deletions core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2205,6 +2205,14 @@ private static RelDataType deriveTypeMapFromEntries(SqlOperatorBinding opBinding
OperandTypes.NUMERIC_OPTIONAL_NUMERIC,
SqlFunctionCategory.NUMERIC);

/** The "LOG(numeric, numeric1)" function. Returns the base numeric1 logarithm of numeric. */
caicancai marked this conversation as resolved.
Show resolved Hide resolved
@LibraryOperator(libraries = {MYSQL, SPARK})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the PR title just has MySQL.

Copy link
Member Author

@caicancai caicancai Apr 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tanclary How should I reasonably describe this PR title?
Add LOG function (enabled in MYSQL, Spark library) or Add LOG function (enabled in MYSQL* library).

Before I was Add LOG function (enabled in MYSQL, Spark library), I thought you asked me to change it to Add LOG function (enabled in MYSQL library), it seems I didn’t understand what you meant, sorry

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use Mysql_Style to describe it. Do you think it is acceptable?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. Please look at other similar function implementations to see how they handle multiple libraries.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other functions do not specifically adapt their log functions to the mysql and spark dialects like the log function.

Copy link
Member Author

@caicancai caicancai Apr 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tanclary I found similar cases [CALCITE-5642] Add SHA256, SHA512 functions (enabled in BigQuery and PostgreSQL libraries), so I should change back to Add LOG function (enabled in MYSQL, Spark library)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Style" doesn't add much. The issue is you can't name a functionLOG_MYSQL if that function is also used for Spark. Does that make sense?

Copy link
Member Author

@caicancai caicancai Apr 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOG_MYSQL_SPARK?

Copy link
Member Author

@caicancai caicancai Apr 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I wrote at the beginning was LOG_MYSQL_SPARK. It seems that I have always misunderstood what you meant. Sorry.

At present, we have some differences regarding some specifications, and the logic of the code itself is fine. If I still have problems after I modify it, I hope you can tell me how it should be modified because I really can't understand what you mean. Thank you very much. I will be very grateful. (please submit a commit and @caicancai ).

I'm sorry for my behavior of ping you multiple times, I have to admit it was not a polite way

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tanclary PTAL, thank you

public static final SqlFunction LOG_MYSQL_SPARK =
SqlBasicFunction.create("LOG",
ReturnTypes.DOUBLE_FORCE_NULLABLE,
OperandTypes.NUMERIC_OPTIONAL_NUMERIC,
SqlFunctionCategory.NUMERIC);

/** The "LOG2(numeric)" function. Returns the base 2 logarithm of numeric. */
@LibraryOperator(libraries = {MYSQL, SPARK})
public static final SqlFunction LOG2 =
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/apache/calcite/util/BuiltInMethod.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ public enum BuiltInMethod {
SAFE_MULTIPLY(SqlFunctions.class, "safeMultiply", double.class, double.class),
SAFE_SUBTRACT(SqlFunctions.class, "safeSubtract", double.class, double.class),
LOG(SqlFunctions.class, "log", long.class, long.class),
LOG2(SqlFunctions.class, "log2", long.class),
LOG_MYSQL_SPARK(SqlFunctions.class, "logMysqlSpark", long.class, long.class),
SEC(SqlFunctions.class, "sec", double.class),
SECH(SqlFunctions.class, "sech", double.class),
SIGN(SqlFunctions.class, "sign", long.class),
Expand Down
1 change: 1 addition & 0 deletions site/_docs/reference.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2785,6 +2785,7 @@ In the following:
| b f s | LENGTH(string) | Equivalent to `CHAR_LENGTH(string)`
| h s | LEVENSHTEIN(string1, string2) | Returns the Levenshtein distance between *string1* and *string2*
| b | LOG(numeric1 [, numeric2 ]) | Returns the logarithm of *numeric1* to base *numeric2*, or base e if *numeric2* is not present
| m s | LOG(numeric1 [, numeric2 ]) | Returns the logarithm of *numeric1* to base *numeric2*, or base e if *numeric2* is not present
caicancai marked this conversation as resolved.
Show resolved Hide resolved
| m s | LOG2(numeric) | Returns the base 2 logarithm of *numeric*
| b o s | LPAD(string, length [, pattern ]) | Returns a string or bytes value that consists of *string* prepended to *length* with *pattern*
| b | TO_BASE32(string) | Converts the *string* to base-32 encoded form and returns an encoded string
Expand Down
53 changes: 53 additions & 0 deletions testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6293,6 +6293,12 @@ void checkRegexpExtract(SqlOperatorFixture f0, FunctionAlias functionAlias) {
isWithin(-2.0, 0.000001));
f.checkNull("log(cast(null as real), 10)");
f.checkNull("log(10, cast(null as real))");
f.checkScalarApprox("log(2.71828)", "DOUBLE NOT NULL",
isWithin(1.0, 0.000001));
f.checkScalarApprox("log(2.71828)", "DOUBLE NOT NULL",
isWithin(0.999999327, 0.0000001));
f.checkNull("log(cast(null as real), 10)");
f.checkNull("log(10, cast(null as real))");
}

/** Test case for
Expand Down Expand Up @@ -6332,6 +6338,53 @@ void checkRegexpExtract(SqlOperatorFixture f0, FunctionAlias functionAlias) {
f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
}

/** Test case for
* <a href="https://issues.apache.org/jira/browse/CALCITE-6325">[CALCITE-6325]
* Add LOG function (enabled in MYSQL, Spark library)</a>. */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: MySQL*, and can you make that consistent across this PR (commit message, jira, etc.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what it means, sorry

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please write it as "MySQL", that's how they style it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add LOG function as MySql style or Add LOG function (enabled in MYSQL*) ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please write it as "MySQL"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I was sick during this period and was delayed for some time.

@Test void testLogMysqlSparkFunc() {
final SqlOperatorFixture f0 = fixture();
f0.checkFails("^log(100, 10)^",
"No match found for function signature LOG\\(<NUMERIC>, <NUMERIC>\\)", false);
f0.setFor(SqlLibraryOperators.LOG_MYSQL_SPARK);
final Consumer<SqlOperatorFixture> consumer = f -> {
f.checkScalarApprox("log(10, 10)", "DOUBLE",
isWithin(1.0, 0.000001));
f.checkScalarApprox("log(64, 8)", "DOUBLE",
isWithin(2.0, 0.000001));
f.checkScalarApprox("log(27,3)", "DOUBLE",
isWithin(3.0, 0.000001));
f.checkScalarApprox("log(100, 10)", "DOUBLE",
isWithin(2.0, 0.000001));
f.checkScalarApprox("log(10, 100)", "DOUBLE",
isWithin(0.5, 0.000001));
f.checkScalarApprox("log(cast(10e6 as double), 10)", "DOUBLE",
isWithin(7.0, 0.000001));
f.checkScalarApprox("log(cast(10e8 as float), 10)", "DOUBLE",
isWithin(9.0, 0.000001));
f.checkScalarApprox("log(cast(10e-3 as real), 10)", "DOUBLE",
isWithin(-2.0, 0.000001));
f.checkNull("log(cast(null as real), 10)");
f.checkNull("log(10, cast(null as real))");
f.checkNull("log(0, 2)");
f.checkNull("log(0,-2)");
f.checkNull("log(0, +0.0)");
f.checkNull("log(0, 0.0)");
f.checkNull("log(null)");
f.checkNull("log(cast(null as real))");
f.checkScalarApprox("log(2.71828)", "DOUBLE",
isWithin(1.0, 0.000001));
f.checkScalarApprox("log(2.71828)", "DOUBLE",
isWithin(0.999999327, 0.0000001));
f.checkNull("log(cast(null as real))");
f.checkNull("log(cast(null as real))");
f.checkNull("log(0)");
f.checkNull("log(+0.0)");
f.checkNull("log(null)");
f.checkNull("log(cast(null as real))");
};
f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
}

@Test void testRandFunc() {
final SqlOperatorFixture f = fixture();
f.setFor(SqlStdOperatorTable.RAND, VmName.EXPAND);
Expand Down
Loading