[SPARK-29761][SQL] do not output leading 'interval' in CalendarInterval.toString

[cloud-fan]

What changes were proposed in this pull request?

remove the leading "interval" in CalendarInterval.toString.

Why are the changes needed?

Although it's allowed to have "interval" prefix when casting string to int, it's not recommended.

This is also consistent with pgsql:

cloud0fan=# select interval '1' day;
interval ----------
1 day
(1 row)

Does this PR introduce any user-facing change?

yes, when display a dataframe with interval type column, the result is different.

How was this patch tested?

updated tests.

[cloud-fan]

cc @MaxGekk@dongjoon-hyun@maropu

[maropu]

Ur, I see, this change looks reasonable. Actually, it seems pgSQL does so;

postgres=# select cast('1 day' as interval);
interval ----------
1 day
(1 row)

[yaooqinn]

I have been confusing this weird prefix for a long time，but how about we use sql standard output style？

[cloud-fan]

AFAIK pgsql has configs to change the interval output string. We can think about it later.

[SparkQA]

Test build #113271 has finished for PR 26401 at commit 5fb2fe6.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #113273 has finished for PR 26401 at commit e08c60b.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[MaxGekk]

nit: Less .append() calls:

Suggested change

	sb.append(value).append(' ').append(unit).append('s').append(' ');
	sb.append(value).append(' ').append(unit).append("s ");

[MaxGekk]

Not important but you could leave the code as is, and just do trim() on the result.

[SparkQA]

Test build #113278 has finished for PR 26401 at commit dc094a7.

• This patch passes all tests.
• This patch does not merge cleanly.
• This patch adds no public classes.

[dongjoon-hyun]

Thank you for pinging me, @cloud-fan . I'm +1 for this, but it seems that we need a migration guide in order to avoid surprises because the output change might affect the old Spark application which expects that prefix. (cc @gatorsmile )

This PR already shows that kind of side effects.

- df.selectExpr(s"d - $i"),
+ df.selectExpr(s"d - INTERVAL'$i'"),

[cloud-fan]

This PR already shows that kind of side effects.

The particular case you pointed out is not a breaking change. It relies on CalendarInterval.toString which is a private class.

it does change the result when casting interval to string. Let me add a migration guide in case users rely on that string.

[SparkQA]

Test build #113297 has finished for PR 26401 at commit d68d974.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[cloud-fan]

retest this please

[SparkQA]

Test build #113303 has finished for PR 26401 at commit d68d974.

• This patch fails due to an unknown error code, -9.
• This patch merges cleanly.
• This patch adds no public classes.

[cloud-fan]

retest this please

[SparkQA]

Test build #113306 has finished for PR 26401 at commit d68d974.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #113357 has finished for PR 26401 at commit c1f3473.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
• case class LocalShuffleReaderExec(child: SparkPlan) extends UnaryExecNode
• class ContinuousRecordEndpoint(buckets: Seq[Seq[UnsafeRow]], lock: Object)

[cloud-fan]

thanks for the review, merging to master!