julianhyde commented on code in PR #2823:
URL: https://github.com/apache/calcite/pull/2823#discussion_r887395596
##########
core/src/main/codegen/templates/Parser.jj:
##########
@@ -4528,17 +4528,50 @@ SqlLiteral DateTimeLiteral() :
return SqlParserUtil.parseTimestampLiteral(p, s.end(this));
}
|
- <DATE> { s = span(); } <QUOTED_STRING> {
- return SqlParserUtil.parseDateLiteral(token.image, s.end(this));
- }
+ <DATE> { s = span(); } ((<QUOTED_STRING> | <BIG_QUERY_QUOTED_STRING>)
Review Comment:
Can you use the same indentation for '{' and '|' as other code.
##########
core/src/main/java/org/apache/calcite/sql/parser/SqlParserUtil.java:
##########
@@ -139,38 +139,45 @@ public static java.sql.Timestamp parseTimestamp(String s)
{
return java.sql.Timestamp.valueOf(s);
}
- public static SqlDateLiteral parseDateLiteral(String s, SqlParserPos pos) {
- final String dateStr = parseString(s);
+ public static SqlDateLiteral parseDateLiteralWithoutQuote(String dateStr,
SqlParserPos pos) {
final Calendar cal =
DateTimeUtils.parseDateFormat(dateStr, Format.get().date,
DateTimeUtils.UTC_ZONE);
if (cal == null) {
throw SqlUtil.newContextException(pos,
- RESOURCE.illegalLiteral("DATE", s,
+ RESOURCE.illegalLiteral("DATE", dateStr,
RESOURCE.badFormat(DateTimeUtils.DATE_FORMAT_STRING).str()));
}
final DateString d = DateString.fromCalendarFields(cal);
return SqlLiteral.createDate(d, pos);
}
- public static SqlTimeLiteral parseTimeLiteral(String s, SqlParserPos pos) {
+ public static SqlDateLiteral parseDateLiteral(String s, SqlParserPos pos) {
final String dateStr = parseString(s);
+ return parseDateLiteralWithoutQuote(dateStr, pos);
+ }
+
+ public static SqlTimeLiteral parseTimeLiteralWithoutQuote(String dateStr,
SqlParserPos pos) {
Review Comment:
Why have a parameter `dateStr` for a function that parses a time literal?
That seems confusing. Should it be `timeStr`? Or just stick to `s`.
##########
core/src/main/codegen/templates/Parser.jj:
##########
@@ -4528,17 +4528,50 @@ SqlLiteral DateTimeLiteral() :
return SqlParserUtil.parseTimestampLiteral(p, s.end(this));
}
|
- <DATE> { s = span(); } <QUOTED_STRING> {
- return SqlParserUtil.parseDateLiteral(token.image, s.end(this));
- }
+ <DATE> { s = span(); } ((<QUOTED_STRING> | <BIG_QUERY_QUOTED_STRING>)
+ {
+ p = SqlParserUtil.stripQuotes(token.image, "'", "'", "\\'",
+ Casing.UNCHANGED);
+ return SqlParserUtil.parseDateLiteralWithoutQuote(p, s.end(this));
+ }
+ | <BIG_QUERY_DOUBLE_QUOTED_STRING>
Review Comment:
Oh I see. You opened a '(' several lines back and didn't indent. Confusing.
I suggest you add a rule (function) that matches <QUOTED_STRING>,
<BIG_QUERY_QUOTED_STRING> and <BIG_QUERY_DOUBLE_QUOTED_STRING> and use it in
all of these literals.
##########
babel/src/test/java/org/apache/calcite/test/BabelParserTest.java:
##########
@@ -191,10 +191,10 @@ class BabelParserTest extends SqlParserTest {
.ok("SELECT TIMESTAMP '1969-07-20 00:00:00'");
// PostgreSQL allows the following. We should too.
sql("select ^timestamp '1969-07-20 1:2'^")
- .fails("Illegal TIMESTAMP literal '1969-07-20 1:2': not in format "
+ .fails("Illegal TIMESTAMP literal 1969-07-20 1:2: not in format "
Review Comment:
Removing the single-quotes from the error message doesn't seem to be an
improvement.
##########
testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java:
##########
@@ -5420,8 +5441,8 @@ public void subTestIntervalYearPositive() {
*/
public void subTestIntervalYearToMonthPositive() {
// default precision
- expr("interval '1-2' year to month")
- .ok("INTERVAL '1-2' YEAR TO MONTH");
+ expr("interval '1:2' year to month")
+ .ok("INTERVAL '1:2' YEAR TO MONTH");
Review Comment:
Did you intend to change the syntax from hyphen to colon? I believe hyphen
was correct already.
##########
core/src/main/codegen/templates/Parser.jj:
##########
@@ -4528,17 +4528,50 @@ SqlLiteral DateTimeLiteral() :
return SqlParserUtil.parseTimestampLiteral(p, s.end(this));
}
|
- <DATE> { s = span(); } <QUOTED_STRING> {
- return SqlParserUtil.parseDateLiteral(token.image, s.end(this));
- }
+ <DATE> { s = span(); } ((<QUOTED_STRING> | <BIG_QUERY_QUOTED_STRING>)
+ {
+ p = SqlParserUtil.stripQuotes(token.image, "'", "'", "\\'",
+ Casing.UNCHANGED);
+ return SqlParserUtil.parseDateLiteralWithoutQuote(p, s.end(this));
+ }
+ | <BIG_QUERY_DOUBLE_QUOTED_STRING>
Review Comment:
Why do we need this BIG_QUERY_DOUBLE_QUOTED_STRING case?
--
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]
