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:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
