xiedeyantu commented on code in PR #4703:
URL: https://github.com/apache/calcite/pull/4703#discussion_r2646587723
##########
site/_docs/reference.md:
##########
@@ -3075,6 +3075,8 @@ In the following:
| b | TO_CODE_POINTS(string) | Converts *string* to an
array of integers that represent code points or extended ASCII character values
| o p r h | TO_DATE(string, format) | Converts *string* to
a date using the format *format*
| o p r | TO_TIMESTAMP(string, format) | Converts *string* to a
timestamp using the format *format*
+| p | AGE(timestamp) | Returns the difference between the current timestamp
and the specified timestamp
+| p | AGE(timestamp1, timestamp2) | Returns the difference between timestamp1
and timestamp2
Review Comment:
I saw the syntax `| b | TRUNC(numeric1 [, integer2 ])` near this line, and I
think this form should be used.
##########
core/src/main/java/org/apache/calcite/util/BuiltInMethod.java:
##########
@@ -980,7 +980,9 @@ public enum BuiltInMethod {
ImmutableBitSet.class),
FUNCTIONAL_DEPENDENCY_DETERMINANTS(FunctionalDependency.class,
"determinants",
ImmutableBitSet.class),
- FUNCTIONAL_DEPENDENCY_GET_FDS(FunctionalDependency.class, "getFDs");
+ FUNCTIONAL_DEPENDENCY_GET_FDS(FunctionalDependency.class, "getFDs"),
+ AGE(SqlFunctions.class, "age", long.class, long.class),
+ AGE_ONE_PARAM(SqlFunctions.class, "age", long.class);
Review Comment:
I'm wondering if we could refer to the implementation of
SqlSubstringFunction, since SUBSTRING can also accept two or three parameters.
Is it possible to avoid declaring two functions based on the number of input
parameters here?
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -1027,6 +1028,8 @@ void populate2() {
define(FORMAT_TIME, datetimeFormatImpl);
define(FORMAT_TIMESTAMP, datetimeFormatImpl);
+ defineReflective(AGE_PG, BuiltInMethod.AGE.method,
BuiltInMethod.AGE_ONE_PARAM.method);
Review Comment:
If we can support variable input parameters in a single execution function,
then we wouldn't need to use `defineReflective` here, would we? Also, I noticed
that DuckDB also supports this function; could we support it as well, and stop
using the name `AGE_PG`?
```
duckdb> SELECT AGE(timestamp '2023-12-25 08:08:08.080', timestamp
'2020-01-01') FROM (VALUES (1)) t;
┌────────────────────────────────────────────────────────────────────────────────────┐
│ age(CAST('2023-12-25 08:08:08.080' AS TIMESTAMP), CAST('2020-01-01' AS
TIMESTAMP)) │
╞════════════════════════════════════════════════════════════════════════════════════╡
│ 0 years 47 mons 24 days 8 hours 8 mins 8.080000000 secs
│
└────────────────────────────────────────────────────────────────────────────────────┘
```
--
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]
