nnguyen168 commented on PR #55466:
URL: https://github.com/apache/spark/pull/55466#issuecomment-4555314554
@cloud-fan Thank you for the insightful analysis. 100% agree with your
assessment.
I went ahead and verified your structural scanner sketch against all test
cases:
- ✅ All 15 existing splitSemiColon unit tests from CliSuite SPARK-37906
- ✅ All 3 new tests from this PR
- ✅ The nested comment bug I reported (SELECT 1; /* outer /* inner */ */)
- ✅ Your newly identified edge case (/* a */ -- foo\n/* b */)
- ✅ Quoted strings with semicolons (SPARK-26321)
All 31 tests pass.
I'd like to help move this forward. Two options:
Option A: I can implement the structural refactor directly in this PR if
@yadavay-amzn would like to collaborate. I can prepare the Scala implementation
for both `SparkSQLCLIDriver.splitSemiColon` and
`StringUtils.splitSemiColonWithIndex`, along with comprehensive tests.
Option B: If @yadavay-amzn prefers to land the current fix as-is (addressing
the original bug), I can open a follow-up PR with the structural refactor to
consolidate both implementations and fix the remaining edge cases.
@yadavay-amzn What's your preference?
--
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]
