[GitHub] [spark] rubenfiszel commented on a change in pull request #21334: [SPARK-24345][SQL]Improve ParseError stop location when offending symbol is a token

[GitBox <gi...@apache.org>]

rubenfiszel commented on a change in pull request #21334: [SPARK-24345][SQL]Improve ParseError stop location when offending symbol is a token
URL: https://github.com/apache/spark/pull/21334#discussion_r272194042
 
 

 ##########
 File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/ParseDriver.scala
 ##########
 @@ -203,8 +203,17 @@ case object ParseErrorListener extends BaseErrorListener {
       charPositionInLine: Int,
       msg: String,
       e: RecognitionException): Unit = {
-    val position = Origin(Some(line), Some(charPositionInLine))
-    throw new ParseException(None, msg, position, position)
+    val (start, stop) = offendingSymbol match {
+      case token: CommonToken =>
+        val start = Origin(Some(line), Some(token.getCharPositionInLine))
 
 Review comment:
   From a pure code point of view, it's not equivalent since it is using the token.getCharPositionInline instead of the method arg.
   
   It might be equivalent but that would require an invariant to hold (method getCharPositionInline == token.getCharPositionInLine)  that seems unnecessary since the intent of this specific case is to leverage the informations from the CommonToken directly.
   
   The difference between CommonToken and other types of offending symbols is that it is clear for CommonToken where is the stop.
   
   We use this internally on our fork of spark to get nice language-server-protocol errors that are correctly delimited.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org