Add GRACE PERIOD to SHOW CREATE MATERIALIZED VIEW #27529

piotrrzysko · 2025-12-02T07:14:33Z

Description

This PR adds support for displaying the GRACE PERIOD clause in the output of SHOW CREATE MATERIALIZED VIEW.

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text:

## Section
* Add `GRACE PERIOD` to `SHOW CREATE MATERIALIZED VIEW` output. ({issue}`issuenumber`)

core/trino-main/src/main/java/io/trino/util/DateTimeUtils.java

findepi · 2025-12-02T14:07:39Z

core/trino-main/src/main/java/io/trino/util/DateTimeUtils.java


-        PeriodFormatterBuilder builder = new PeriodFormatterBuilder();
+        PeriodFormatterBuilder builder = new PeriodFormatterBuilder()
+                .printZeroIfSupported();


why so? code comment

findepi · 2025-12-02T14:07:48Z

core/trino-main/src/main/java/io/trino/util/DateTimeUtils.java

                    break;
                }
                builder.appendLiteral(":");
+                builder.minimumPrintedDigits(2);


why so? code comment

findepi · 2025-12-02T14:09:30Z

core/trino-main/src/test/java/io/trino/util/TestDateTimeUtils.java

+    @Test
+    void testDayTimeIntervalRoundTrip()
+    {
+        testDayTimeIntervalRoundTrip("0", SECOND, Optional.empty());


i'd rather have this take SQL interval string as input to test roundtrip.
That would be testing at the level at which things must be stable, and without adding too many layers IMO.

findepi · 2025-12-02T14:11:24Z

core/trino-main/src/main/java/io/trino/util/DateTimeUtils.java

+        if (period.getDays() > 0) {
+            startField = DAY;
+            if (period.getSeconds() > 0 || period.getMillis() > 0) {
+                endField = Optional.of(SECOND);
+                value = INTERVAL_DAY_SECOND_FORMATTER.print(period);
+            }
+            else if (period.getMinutes() > 0) {
+                endField = Optional.of(MINUTE);
+                value = INTERVAL_DAY_MINUTE_FORMATTER.print(period);
+            }
+            else if (period.getHours() > 0) {
+                endField = Optional.of(HOUR);
+                value = INTERVAL_DAY_HOUR_FORMATTER.print(period);
+            }
+            else {
+                endField = Optional.empty();
+                value = INTERVAL_DAY_FORMATTER.print(period);
+            }
+        }
+        else if (period.getHours() > 0) {
+            startField = HOUR;
+            if (period.getSeconds() > 0 || period.getMillis() > 0) {
+                endField = Optional.of(SECOND);
+                value = INTERVAL_HOUR_SECOND_FORMATTER.print(period);
+            }
+            else if (period.getMinutes() > 0) {
+                endField = Optional.of(MINUTE);
+                value = INTERVAL_HOUR_MINUTE_FORMATTER.print(period);
+            }
+            else {
+                endField = Optional.empty();
+                value = INTERVAL_HOUR_FORMATTER.print(period);
+            }
+        }
+        else if (period.getMinutes() > 0) {
+            startField = MINUTE;
+            if (period.getSeconds() > 0 || period.getMillis() > 0) {
+                endField = Optional.of(SECOND);
+                value = INTERVAL_MINUTE_SECOND_FORMATTER.print(period);
+            }
+            else {
+                endField = Optional.empty();
+                value = INTERVAL_MINUTE_FORMATTER.print(period);
+            }
+        }
+        else {
+            startField = SECOND;
+            endField = Optional.empty();
+            value = INTERVAL_SECOND_FORMATTER.print(period);
+        }


This looks terribly verbose, but i don't know how to improve it. ok to keep

One option is to reuse the function we use for casts:

trino/client/trino-client/src/main/java/io/trino/client/IntervalDayTime.java

Lines 52 to 73 in 518b244

public static String formatMillis(long millis)

{

if (millis == Long.MIN_VALUE) {

return LONG_MIN_VALUE;

}

String sign = "";

if (millis < 0) {

sign = "-";

millis = -millis;

}

long day = millis / MILLIS_IN_DAY;

millis %= MILLIS_IN_DAY;

long hour = millis / MILLIS_IN_HOUR;

millis %= MILLIS_IN_HOUR;

long minute = millis / MILLIS_IN_MINUTE;

millis %= MILLIS_IN_MINUTE;

long second = millis / MILLIS_IN_SECOND;

millis %= MILLIS_IN_SECOND;

return format("%s%d %02d:%02d:%02d.%03d", sign, day, hour, minute, second, millis);

}

but then the output is verbose:

trino/core/trino-main/src/test/java/io/trino/type/TestIntervalDayTime.java

Lines 431 to 434 in 518b244

assertThat(assertions.expression("CAST(a AS varchar)")

.binding("a", "INTERVAL '12' DAY"))

.hasType(VARCHAR)

.isEqualTo("12 00:00:00.000");

Keep in mind this is coming: #27539

Keep in mind this is coming: #27539

Thanks for the link!

i skimmed the PR and it looks mostly backwards compatible.
The code in this PR takes java time interval and formats it as a SQL literal, so it should work both before and after the same.

One option is to reuse the function we use for casts

Sounds much simpler. How would that look like @piotrrzysko ?
Would we still output CREATE MV statement that can be pasted back into CLI and run?

A prototype: piotrrzysko@962fe50

I added tests that demonstrate the formatted result can be parsed back into the original input (expressed in milliseconds, since the original format is not retained).

The only downside I see in this approach, as mentioned earlier, is that the formatted values are not succinct (e.g., INTERVAL '1' HOUR becomes INTERVAL '0 01:00:00.000' DAY TO SECOND).

I agree that succinctness of the output is not critical in this case. Please take a look at the latest fixup commit. Since we cannot guarantee that the interval fields are preserved either way, it seems simplest to always use the widest range.

One thing worth pointing out is that INTERVAL '1' HOUR and INTERVAL '...' DAY TO SECOND are actually different types in standard SQL. We don't distinguish between them yet, but it's something I'm trying to fix.

So this means that for GRACE PERIOD we will store the original interval fields and precision, right? If so, this is even better, because then we will explicitly know which formatter should be used.

Eventually, yes.

One thing worth pointing out is that INTERVAL '1' HOUR and INTERVAL '...' DAY TO SECOND are actually different types in standard SQL.

great

So this means that for GRACE PERIOD we will store the original interval fields and precision, right?

i am against that. in our simplified world, 1 day = 24 hours = 24 * 60 minutes = 24 * 60 * 60 seconds
furthermore, for MV functionality, it doesn't at all matter how the grace period time duration was specified, so i don't see why we would want to go extra complexity to preserve that

anyhow, all this certainly does not matter from the perspective of this PR. it's undoubtedly a bug fix and way forward. let's merge it

findepi · 2025-12-05T08:59:22Z

please rebase

core/trino-main/src/main/java/io/trino/util/DateTimeUtils.java

core/trino-main/src/main/java/io/trino/sql/rewrite/ShowQueriesRewrite.java

cla-bot bot added the cla-signed label Dec 2, 2025

piotrrzysko force-pushed the show_create_grace_period branch from 8701367 to 7e3c950 Compare December 2, 2025 08:30

piotrrzysko marked this pull request as ready for review December 2, 2025 11:14

piotrrzysko requested review from findepi and martint December 2, 2025 11:14

findepi approved these changes Dec 2, 2025

View reviewed changes

piotrrzysko force-pushed the show_create_grace_period branch from 7e3c950 to 7b1cf65 Compare December 8, 2025 07:34

findepi reviewed Dec 8, 2025

View reviewed changes

core/trino-main/src/main/java/io/trino/util/DateTimeUtils.java Outdated Show resolved Hide resolved

core/trino-main/src/main/java/io/trino/util/DateTimeUtils.java Show resolved Hide resolved

Add GRACE PERIOD to SHOW CREATE MATERIALIZED VIEW

2c69b65

piotrrzysko force-pushed the show_create_grace_period branch from 7b1cf65 to 2c69b65 Compare December 8, 2025 16:10

findepi reviewed Dec 8, 2025

View reviewed changes

core/trino-main/src/main/java/io/trino/sql/rewrite/ShowQueriesRewrite.java Outdated Show resolved Hide resolved

Apply suggestion from @findepi

26ce1dc

findepi approved these changes Dec 8, 2025

View reviewed changes

findepi merged commit a57636b into trinodb:master Dec 8, 2025
99 checks passed

github-actions bot added this to the 479 milestone Dec 8, 2025

	public static String formatMillis(long millis)
	{
	if (millis == Long.MIN_VALUE) {
	return LONG_MIN_VALUE;
	}
	String sign = "";
	if (millis < 0) {
	sign = "-";
	millis = -millis;
	}

	long day = millis / MILLIS_IN_DAY;
	millis %= MILLIS_IN_DAY;
	long hour = millis / MILLIS_IN_HOUR;
	millis %= MILLIS_IN_HOUR;
	long minute = millis / MILLIS_IN_MINUTE;
	millis %= MILLIS_IN_MINUTE;
	long second = millis / MILLIS_IN_SECOND;
	millis %= MILLIS_IN_SECOND;

	return format("%s%d %02d:%02d:%02d.%03d", sign, day, hour, minute, second, millis);
	}

	assertThat(assertions.expression("CAST(a AS varchar)")
	.binding("a", "INTERVAL '12' DAY"))
	.hasType(VARCHAR)
	.isEqualTo("12 00:00:00.000");

Add GRACE PERIOD to SHOW CREATE MATERIALIZED VIEW #27529

Add GRACE PERIOD to SHOW CREATE MATERIALIZED VIEW #27529

Conversation

piotrrzysko commented Dec 2, 2025

Description

Additional context and related issues

Release notes

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

piotrrzysko Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

findepi commented Dec 5, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

piotrrzysko Dec 8, 2025 •

edited

Loading