uros-b commented on PR #58826: URL: https://github.com/apache/spark/pull/58826#issuecomment-5683109294
One blocker item tho: SPARK-56147 tests encode the opposite spec [SPARK-56147](https://github.com/apache/spark/pull/54946) treated this as intentional. CliSuite (~Ln 624–653) currently says so in comments: ``` test("SPARK-37471: spark-sql support nested bracketed comment ") { // ... (Note `/*+` is a hint marker, not a nested comment opener, so nesting must use `/*`.) ... testRetry("SPARK-37555: spark-sql should pass last unclosed comment to backend") { runCliWithin(1.minute)( // A fully closed bracketed comment. `/*+` inside it is a hint marker, // not a nested comment opener, so the comment closes at the first `*/`; // the comment is skipped and the trailing `SELECT 1` executes. "/* SELECT /*+ HINT() 4; */ SELECT 1;".stripMargin -> "1", ``` With this PR, "/* SELECT /*+ HINT() 4; */ SELECT 1;" is an unclosed outer comment, not SELECT 1. Hive CliSuite will fail (it may only show up in -Phive jobs). Those comments and expected results need to be rewritten as part of this change, and the PR body should say this reverses SPARK-56147’s interpretation: /*+ is a hint only as a top-level token, not inside an already-open /*. That is the right language rule (ANSI nesting; hints are not comments), but it is a behavior change relative to what master currently documents. -- 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]
