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-6345] Intervals with more than 100 years are not supported #3743

Merged
merged 1 commit into from
Apr 17, 2024
Merged

[CALCITE-6345] Intervals with more than 100 years are not supported #3743

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion core/src/main/java/org/apache/calcite/sql/SqlIntervalQualifier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ public boolean isWeek() {

public int getStartPrecision(RelDataTypeSystem typeSystem) {
if (startPrecision == RelDataType.PRECISION_NOT_SPECIFIED) {
return typeSystem.getDefaultPrecision(typeName());
return typeSystem.getMaxPrecision(typeName());
} else {
return startPrecision;
}
Expand Down
6 changes: 4 additions & 2 deletions core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2262,6 +2262,8 @@ void testLikeAndSimilarFails() {
expr("INTERVAL '5' MONTH").assertInterval(is(5L));
expr("INTERVAL '3-2' YEAR TO MONTH").assertInterval(is(38L));
expr("INTERVAL '-5-4' YEAR TO MONTH").assertInterval(is(-64L));
expr("INTERVAL '100-2' YEAR TO MONTH").assertInterval(is(1202L));
expr("INTERVAL '1000-2' YEAR TO MONTH").assertInterval(is(12002L));
}

@Test void testIntervalMillisConversion() {
Expand Down Expand Up @@ -2381,9 +2383,9 @@ private IntervalTest.Fixture2 getFixture2(SqlValidatorFixture f2) {
.columnType("TIME(0) NOT NULL");

expr("interval '1' day + interval '1' DAY(4)")
.columnType("INTERVAL DAY(4) NOT NULL");
.columnType("INTERVAL DAY NOT NULL");
expr("interval '1' day(5) + interval '1' DAY")
.columnType("INTERVAL DAY(5) NOT NULL");
.columnType("INTERVAL DAY NOT NULL");
expr("interval '1' day + interval '1' HOUR(10)")
.columnType("INTERVAL DAY TO HOUR NOT NULL");
expr("interval '1' day + interval '1' MINUTE")
Expand Down
26 changes: 12 additions & 14 deletions testkit/src/main/java/org/apache/calcite/test/IntervalTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -871,7 +871,7 @@ public void subTestIntervalYearNegative() {
// Field value out of range
// (default, explicit default, alt, neg alt, max, neg max)
f.wholeExpr("INTERVAL '100' YEAR")
Copy link
Contributor

Choose a reason for hiding this comment

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

A small question, whether to put the supported cases separately in an executable single test instead of deleting them directly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is the suggestion to convert these into positive tests? I can do that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, recommend keeping the test case as positive tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added the removed tests back as positive tests, but I have kept them in the same place in the code.
I could move them to separate functions too if that's clearer.

.fails("Interval field value 100 exceeds precision of YEAR\\(2\\) field.*");
.columnType("INTERVAL YEAR NOT NULL");
f.wholeExpr("INTERVAL '100' YEAR(2)")
.fails("Interval field value 100 exceeds precision of YEAR\\(2\\) field.*");
f.wholeExpr("INTERVAL '1000' YEAR(3)")
Expand Down Expand Up @@ -936,7 +936,7 @@ public void subTestIntervalYearToMonthNegative() {
// (default, explicit default, alt, neg alt, max, neg max)
// plus >max value for mid/end fields
f.wholeExpr("INTERVAL '100-0' YEAR TO MONTH")
.fails("Interval field value 100 exceeds precision of YEAR\\(2\\) field.*");
.columnType("INTERVAL YEAR TO MONTH NOT NULL");
f.wholeExpr("INTERVAL '100-0' YEAR(2) TO MONTH")
.fails("Interval field value 100 exceeds precision of YEAR\\(2\\) field.*");
f.wholeExpr("INTERVAL '1000-0' YEAR(3) TO MONTH")
Expand Down Expand Up @@ -1124,7 +1124,7 @@ public void subTestIntervalMonthNegative() {
// Field value out of range
// (default, explicit default, alt, neg alt, max, neg max)
f.wholeExpr("INTERVAL '100' MONTH")
.fails("Interval field value 100 exceeds precision of MONTH\\(2\\) field.*");
.columnType("INTERVAL MONTH NOT NULL");
f.wholeExpr("INTERVAL '100' MONTH(2)")
.fails("Interval field value 100 exceeds precision of MONTH\\(2\\) field.*");
f.wholeExpr("INTERVAL '1000' MONTH(3)")
Expand Down Expand Up @@ -1180,7 +1180,7 @@ public void subTestIntervalDayNegative() {
// Field value out of range
// (default, explicit default, alt, neg alt, max, neg max)
f.wholeExpr("INTERVAL '100' DAY")
.fails("Interval field value 100 exceeds precision of DAY\\(2\\) field.*");
.columnType("INTERVAL DAY NOT NULL");
f.wholeExpr("INTERVAL '100' DAY(2)")
.fails("Interval field value 100 exceeds precision of DAY\\(2\\) field.*");
f.wholeExpr("INTERVAL '1000' DAY(3)")
Expand Down Expand Up @@ -1245,7 +1245,7 @@ public void subTestIntervalDayToHourNegative() {
// (default, explicit default, alt, neg alt, max, neg max)
// plus >max value for mid/end fields
f.wholeExpr("INTERVAL '100 0' DAY TO HOUR")
.fails("Interval field value 100 exceeds precision of DAY\\(2\\) field.*");
.columnType("INTERVAL DAY TO HOUR NOT NULL");
f.wholeExpr("INTERVAL '100 0' DAY(2) TO HOUR")
.fails("Interval field value 100 exceeds precision of DAY\\(2\\) field.*");
f.wholeExpr("INTERVAL '1000 0' DAY(3) TO HOUR")
Expand Down Expand Up @@ -1323,7 +1323,7 @@ public void subTestIntervalDayToMinuteNegative() {
// (default, explicit default, alt, neg alt, max, neg max)
// plus >max value for mid/end fields
f.wholeExpr("INTERVAL '100 0:0' DAY TO MINUTE")
.fails("Interval field value 100 exceeds precision of DAY\\(2\\) field.*");
.columnType("INTERVAL DAY TO MINUTE NOT NULL");
f.wholeExpr("INTERVAL '100 0:0' DAY(2) TO MINUTE")
.fails("Interval field value 100 exceeds precision of DAY\\(2\\) field.*");
f.wholeExpr("INTERVAL '1000 0:0' DAY(3) TO MINUTE")
Expand Down Expand Up @@ -1511,8 +1511,7 @@ public void subTestIntervalHourNegative() {
// Field value out of range
// (default, explicit default, alt, neg alt, max, neg max)
f.wholeExpr("INTERVAL '100' HOUR")
.fails("Interval field value 100 exceeds precision of "
+ "HOUR\\(2\\) field.*");
.columnType("INTERVAL HOUR NOT NULL");
f.wholeExpr("INTERVAL '100' HOUR(2)")
.fails("Interval field value 100 exceeds precision of "
+ "HOUR\\(2\\) field.*");
Expand Down Expand Up @@ -1580,7 +1579,7 @@ public void subTestIntervalHourToMinuteNegative() {
// (default, explicit default, alt, neg alt, max, neg max)
// plus >max value for mid/end fields
f.wholeExpr("INTERVAL '100:0' HOUR TO MINUTE")
.fails("Interval field value 100 exceeds precision of HOUR\\(2\\) field.*");
.columnType("INTERVAL HOUR TO MINUTE NOT NULL");
f.wholeExpr("INTERVAL '100:0' HOUR(2) TO MINUTE")
.fails("Interval field value 100 exceeds precision of HOUR\\(2\\) field.*");
f.wholeExpr("INTERVAL '1000:0' HOUR(3) TO MINUTE")
Expand Down Expand Up @@ -1659,8 +1658,7 @@ public void subTestIntervalHourToSecondNegative() {
// (default, explicit default, alt, neg alt, max, neg max)
// plus >max value for mid/end fields
f.wholeExpr("INTERVAL '100:0:0' HOUR TO SECOND")
.fails("Interval field value 100 exceeds precision of "
+ "HOUR\\(2\\) field.*");
.columnType("INTERVAL HOUR TO SECOND NOT NULL");
f.wholeExpr("INTERVAL '100:0:0' HOUR(2) TO SECOND")
.fails("Interval field value 100 exceeds precision of "
+ "HOUR\\(2\\) field.*");
Expand Down Expand Up @@ -1736,7 +1734,7 @@ public void subTestIntervalMinuteNegative() {
// Field value out of range
// (default, explicit default, alt, neg alt, max, neg max)
f.wholeExpr("INTERVAL '100' MINUTE")
.fails("Interval field value 100 exceeds precision of MINUTE\\(2\\) field.*");
.columnType("INTERVAL MINUTE NOT NULL");
f.wholeExpr("INTERVAL '100' MINUTE(2)")
.fails("Interval field value 100 exceeds precision of MINUTE\\(2\\) field.*");
f.wholeExpr("INTERVAL '1000' MINUTE(3)")
Expand Down Expand Up @@ -1809,7 +1807,7 @@ public void subTestIntervalMinuteToSecondNegative() {
// (default, explicit default, alt, neg alt, max, neg max)
// plus >max value for mid/end fields
f.wholeExpr("INTERVAL '100:0' MINUTE TO SECOND")
.fails("Interval field value 100 exceeds precision of MINUTE\\(2\\) field.*");
.columnType("INTERVAL MINUTE TO SECOND NOT NULL");
f.wholeExpr("INTERVAL '100:0' MINUTE(2) TO SECOND")
.fails("Interval field value 100 exceeds precision of MINUTE\\(2\\) field.*");
f.wholeExpr("INTERVAL '1000:0' MINUTE(3) TO SECOND")
Expand Down Expand Up @@ -1883,7 +1881,7 @@ public void subTestIntervalSecondNegative() {
// Field value out of range
// (default, explicit default, alt, neg alt, max, neg max)
f.wholeExpr("INTERVAL '100' SECOND")
.fails("Interval field value 100 exceeds precision of SECOND\\(2\\) field.*");
.columnType("INTERVAL SECOND NOT NULL");
f.wholeExpr("INTERVAL '100' SECOND(2)")
.fails("Interval field value 100 exceeds precision of SECOND\\(2\\) field.*");
f.wholeExpr("INTERVAL '1000' SECOND(3)")
Expand Down
Loading