kbendick commented on a change in pull request #4396:
URL: https://github.com/apache/iceberg/pull/4396#discussion_r836016333
##########
File path:
spark/v3.2/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/parser/extensions/IcebergSparkSqlExtensionsParser.scala
##########
@@ -176,7 +176,12 @@ class IcebergSparkSqlExtensionsParser(delegate:
ParserInterface) extends ParserI
}
private def isIcebergCommand(sqlText: String): Boolean = {
- val normalized =
sqlText.toLowerCase(Locale.ROOT).trim().replaceAll("\\s+", " ")
+ val normalized = sqlText.toLowerCase(Locale.ROOT).trim()
+ // Catch all SQL comments, including simple comments (that start with
--), as well as multiline comments
+ .replaceAll("(?ms)/\\*.*?\\*/|--.*?\\n", " ")
Review comment:
So I thought it seemed strange as well to use multi-line and single line
mode as well, but removing single line mode actually made the following test
case fail:
```sql
/* some multi-line
comment */ CALL …
```
So both single line and multi-line mode are needed. And the multiline one
like that, where the statement begins on the same line as the closing comment
line, is what DBT is adding (that people would like to keep).
It could be done with multiple regexes, but it wasn’t much more readable and
is also likely slower. Though the decision to use the regex as is was made
based on the fact that it works, it’s been thoroughly tested, and it can be
broke down into what’s needed and what’s not / explained relatively simply,
admittedly, the multiline / single line thing is kind of weird. But I think
it would be needed with multiple regex too as the same regex needs both
multiline mode and single line mode.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
