barrkel commented on code in PR #3694:
URL: https://github.com/apache/calcite/pull/3694#discussion_r1495655052


##########
core/src/main/java/org/apache/calcite/runtime/CalciteResource.java:
##########
@@ -112,6 +112,9 @@ public interface CalciteResource {
   @BaseMessage("Illegal identifier '':''. Was expecting ''VALUE''")
   ExInst<CalciteException> illegalColon();
 
+  @BaseMessage("Illegal identifier '',''. Was expecting ''VALUE''")

Review Comment:
   Well `,` isn't a legal identifier - the message is not wrong. It's kind of a 
two-prong message; we were expecting an identifier, and for the identifier to 
be `VALUE`. We fell at the first hurdle because `,` isn't an identifier.
   
   (AFAICT `VALUE` is usually a keyword not an identifier but it's a bit 
squishy because it could easily be a non-reserved word.)
   
   A better message might be "Unexpected symbol" rather than "Illegal 
identifier", which has better symmetry with the expected clause following. I'll 
change both to that.



-- 
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: commits-unsubscr...@calcite.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to