Copilot commented on code in PR #4703:
URL: https://github.com/apache/calcite/pull/4703#discussion_r2649494667
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -2774,4 +2774,33 @@ private static RelDataType
deriveTypeMapFromEntries(SqlOperatorBinding opBinding
public static final SqlFunction RANDOM = SqlStdOperatorTable.RAND
.withName("RANDOM")
.withOperandTypeChecker(OperandTypes.NILADIC);
+
+
+ /**
+ * AGE function for PostgreSQL.
+ * Returns the interval between two timestamps in DAY TO SECOND format.
Review Comment:
The Javadoc comment incorrectly states "Returns the interval between two
timestamps in DAY TO SECOND format". This is misleading because the function
actually returns a formatted VARCHAR string (e.g., "3 years 11 mons 24 days 0
hours 0 mins 0.0 secs"), not an interval type or a DAY TO SECOND interval
format. The comment should accurately describe the return format.
```suggestion
* Returns a human-readable VARCHAR describing the interval between
* one or two timestamps (for example,
* "3 years 11 mons 24 days 0 hours 0 mins 0.0 secs").
```
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -2774,4 +2774,33 @@ private static RelDataType
deriveTypeMapFromEntries(SqlOperatorBinding opBinding
public static final SqlFunction RANDOM = SqlStdOperatorTable.RAND
.withName("RANDOM")
.withOperandTypeChecker(OperandTypes.NILADIC);
+
+
Review Comment:
There is an extra blank line between the RANDOM function definition and the
AGE function documentation. Per Java style conventions and consistency with the
rest of the file, there should be only one blank line between field/method
declarations.
```suggestion
```
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -2774,4 +2774,33 @@ private static RelDataType
deriveTypeMapFromEntries(SqlOperatorBinding opBinding
public static final SqlFunction RANDOM = SqlStdOperatorTable.RAND
.withName("RANDOM")
.withOperandTypeChecker(OperandTypes.NILADIC);
+
+
+ /**
+ * AGE function for PostgreSQL.
+ * Returns the interval between two timestamps in DAY TO SECOND format.
+ *
+ * @see <a
href="https://www.postgresql.org/docs/current/functions-datetime.html";>PostgreSQL
AGE</a>
+ */
+ @LibraryOperator(libraries = {POSTGRESQL}, exceptLibraries = {REDSHIFT})
+ public static final SqlBasicFunction AGE =
+ SqlBasicFunction.create("AGE",
+ (SqlOperatorBinding opBinding) -> {
+ RelDataType varcharType =
opBinding.getTypeFactory().createSqlType(SqlTypeName.VARCHAR);
+ // Check if any input parameter is null
+ boolean nullable = false;
+ for (int i = 0; i < opBinding.getOperandCount(); i++) {
+ if (opBinding.isOperandNull(i, true)) {
+ nullable = true;
+ break;
+ }
+ }
+ // Return VARCHAR type since we're returning a formatted string
+ return
opBinding.getTypeFactory().createTypeWithNullability(varcharType, nullable);
+ },
Review Comment:
The type inference logic uses a custom implementation to check nullability,
but this can be simplified by using the standard ReturnTypes.VARCHAR_NULLABLE
constant, which automatically handles nullability based on operand types. The
current manual loop through operands using isOperandNull checks runtime null
values during type inference, which is not the standard pattern used in
Calcite. Instead, use ReturnTypes.VARCHAR_NULLABLE which uses
SqlTypeTransforms.TO_NULLABLE to properly infer nullability from operand types.
```suggestion
ReturnTypes.VARCHAR_NULLABLE,
```
##########
site/_docs/reference.md:
##########
@@ -3075,6 +3075,7 @@ 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(timestamp1 [, timestamp2 ]) | Returns the difference
between timestamps. With one argument, returns the difference between the
current timestamp and the specified timestamp. With two arguments, returns the
difference between timestamp1 and timestamp2
Review Comment:
The documentation description for the AGE function is unclear about the
return type and format. The function returns a formatted string like "3 years
11 mons 24 days 0 hours 0 mins 0.0 secs", not an interval type. The description
should clarify this. Also, for the single-argument version, it should specify
that it returns the difference between the current timestamp (at midnight UTC)
and the specified timestamp, not just "current timestamp".
```suggestion
| p | AGE(timestamp1 [, timestamp2 ]) | Returns a formatted
string representing the difference between timestamps (for example, "3 years 11
mons 24 days 0 hours 0 mins 0.0 secs"), not an interval type. With one
argument, returns the difference between the current timestamp at midnight UTC
and the specified timestamp. With two arguments, returns the difference between
*timestamp1* and *timestamp2*
```
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -7445,4 +7448,95 @@ private enum PartToExtract {
AUTHORITY,
USERINFO;
}
+
+ /** SQL {@code AGE(timestamp1, timestamp2)} function. */
+ private static String age(long timestamp1, long timestamp2) {
+ // Convert timestamps to ZonedDateTime objects using UTC to avoid timezone
issues
+ Instant instant1 = Instant.ofEpochMilli(timestamp1);
+ Instant instant2 = Instant.ofEpochMilli(timestamp2);
+
+ ZonedDateTime dateTime1 = ZonedDateTime.ofInstant(instant1,
ZoneOffset.UTC);
+ ZonedDateTime dateTime2 = ZonedDateTime.ofInstant(instant2,
ZoneOffset.UTC);
+
+ // Check if the original timestamps are in the correct order
+ boolean isNegative = timestamp1 < timestamp2;
+
+ // Ensure dateTime1 is later than dateTime2 for consistent calculation
+ if (dateTime1.isBefore(dateTime2)) {
+ ZonedDateTime temp = dateTime1;
+ dateTime1 = dateTime2;
+ dateTime2 = temp;
+ }
+
+ // Calculate period (years, months, days)
+ Period period = Period.between(dateTime2.toLocalDate(),
dateTime1.toLocalDate());
+
+ // Calculate duration (hours, minutes, seconds, milliseconds)
+ Duration duration = Duration.between(dateTime2, dateTime1);
+
+ // Adjust for possible day overflow when time part is negative
+ if (dateTime1.toLocalTime().isBefore(dateTime2.toLocalTime())) {
+ period = period.minusDays(1);
+ duration = duration.plusDays(1);
+ }
+
+ // Extract components
+ int years = period.getYears();
+ int months = period.getMonths();
+ int days = period.getDays();
+
+ long totalHours = duration.toHours();
+ long totalMinutes = duration.toMinutes();
+ long totalSeconds = duration.getSeconds();
+ long totalMillis = duration.toMillis();
+
+ long hours = totalHours % 24;
+ long minutes = totalMinutes % 60;
+ long seconds = totalSeconds % 60;
+ long millis = totalMillis % 1000;
+
+ // Apply negative sign if needed
+ if (isNegative) {
+ years = -years;
+ months = -months;
+ days = -days;
+ hours = -hours;
+ minutes = -minutes;
+ seconds = -seconds;
+ }
+
+ StringBuilder sb = new StringBuilder();
+ sb.append(years).append(" years ")
+ .append(months).append(" mons ")
+ .append(days).append(" days ")
+ .append(hours).append(" hours ")
+ .append(minutes).append(" mins ")
+ .append(String.format(Locale.ROOT, "%.1f secs", seconds + millis /
1000.0));
+ return sb.toString();
+ }
+
+ /** SQL {@code AGE(timestamp1, timestamp2)} function. Supports 1 or 2
timestamp arguments. */
+ public static String age(long... timestamps) {
+ if (timestamps.length == 0) {
+ throw new IllegalArgumentException("AGE function requires at least one
timestamp argument");
+ }
+
+ if (timestamps.length == 1) {
+ // Single parameter version: calculate age relative to current time
+ long timestamp = timestamps[0];
+ // Get current date at midnight in UTC
+ long currentTimestamp = Instant.now()
+ .atZone(ZoneOffset.UTC)
+ .truncatedTo(ChronoUnit.DAYS)
+ .toInstant()
+ .toEpochMilli();
+ // Call the two-parameter version with current timestamp and input
timestamp
+ return age(currentTimestamp, timestamp);
+ } else if (timestamps.length == 2) {
+ // Two parameter version: calculate age between two timestamps
+ return age(timestamps[0], timestamps[1]);
+ } else {
Review Comment:
The single-argument version of the AGE function truncates the current
timestamp to midnight (using truncatedTo(ChronoUnit.DAYS)), which means the
time component is always set to 00:00:00. This behavior differs from
PostgreSQL's age function, which uses the actual current timestamp including
the time component. This could lead to unexpected results when users compare
with PostgreSQL behavior.
```suggestion
// Use the actual current timestamp (including time component) in UTC
long currentTimestamp = Instant.now().toEpochMilli();
// Call the two-parameter version with current timestamp and input
timestamp
return age(currentTimestamp, timestamp);
} else if (timestamps.length == 2) {
// Two parameter version: calculate age between two timestamps
return age(timestamps[0], timestamps[1]);
} else {
} else if (timestamps.length == 2) {
// Two parameter version: calculate age between two timestamps
return age(timestamps[0], timestamps[1]);
} else {
```
##########
babel/src/test/java/org/apache/calcite/test/BabelTest.java:
##########
@@ -490,4 +496,45 @@ private void checkSqlResult(String funLibrary, String
query, String result) {
.query(query)
.returns(result);
}
+
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7337";>[CALCITE-7337]
+ * Add age function (enabled in Postgresql library)</a>. */
+ @Test void testAgeFunction() {
+ checkSqlResult("postgresql",
+ "SELECT AGE(timestamp '2023-12-25', timestamp '2020-01-01') FROM
(VALUES (1)) t",
+ "EXPR$0=3 years 11 mons 24 days 0 hours 0 mins 0.0 secs\n");
+
+ checkSqlResult("postgresql",
+ "SELECT AGE(timestamp '2023-01-01', timestamp '2023-01-01') FROM
(VALUES (1)) t",
+ "EXPR$0=0 years 0 mons 0 days 0 hours 0 mins 0.0 secs\n");
+
+ checkSqlResult("postgresql",
+ "SELECT AGE(timestamp '2020-01-01', timestamp '2023-12-25') FROM
(VALUES (1)) t",
+ "EXPR$0=-3 years -11 mons -24 days 0 hours 0 mins 0.0 secs\n");
+
+ checkSqlResult("postgresql",
+ "SELECT AGE(timestamp '2023-02-01', timestamp '2023-01-31') FROM
(VALUES (1)) t",
+ "EXPR$0=0 years 0 mons 1 days 0 hours 0 mins 0.0 secs\n");
+
+ checkSqlResult("postgresql",
+ "SELECT AGE(timestamp '2023-12-26 14:30:00', timestamp '2023-12-25
14:30:00') FROM (VALUES (1)) t",
+ "EXPR$0=0 years 0 mons 1 days 0 hours 0 mins 0.0 secs\n");
+
+ checkSqlResult("postgresql",
+ "SELECT AGE(timestamp '2023-12-25 00:00:00', timestamp '2020-01-01
23:59:59') FROM (VALUES (1)) t",
+ "EXPR$0=3 years 11 mons 23 days 0 hours 0 mins 1.0 secs\n");
+
+ LocalDate date = LocalDate.parse("2023-12-25");
+ Instant instant = date.atStartOfDay(ZoneOffset.UTC).toInstant();
+ long timestampMillis = instant.toEpochMilli();
+ LocalDateTime now =
LocalDateTime.now(ZoneOffset.UTC).truncatedTo(ChronoUnit.DAYS);
+ long currentTimestamp = now.toInstant(ZoneOffset.UTC).toEpochMilli();
+ String ageFunctionResult = SqlFunctions.age(currentTimestamp,
timestampMillis);
+ checkSqlResult("postgresql",
+ "SELECT AGE(timestamp '2023-12-25') FROM (VALUES (1)) t",
+ "EXPR$0=" + ageFunctionResult + "\n");
+ }
+
+
Review Comment:
There are two consecutive blank lines at the end of the test method. Per
Java style conventions and consistency with the rest of the codebase, there
should be only one blank line between methods.
```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]
