Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] Expose PPL Parser Errors Instead of Swallowing Them #400

Open
noCharger opened this issue Jun 27, 2024 · 0 comments
Open

[FEATURE] Expose PPL Parser Errors Instead of Swallowing Them #400

noCharger opened this issue Jun 27, 2024 · 0 comments
Labels
enhancement New feature or request Lang:PPL Pipe Processing Language support

Comments

@noCharger
Copy link
Collaborator

Is your feature request related to a problem?

Currently, when the PPL parser fails to parse a query, the FlintSparkPPLParser class swallows the PPL parser error and falls back to the Flint SQL parser. This behavior can lead to confusing error messages for users, as the actual PPL parser error is replaced by a different error from the SQL parser.

override def parsePlan(sqlText: String): LogicalPlan = {
try {
// if successful build ppl logical plan and translate to catalyst logical plan
val context = new CatalystPlanContext
planTrnasormer.visit(plan(pplParser, sqlText, false), context)
context.getPlan
} catch {
// Fall back to Spark parse plan logic if flint cannot parse
case _: ParseException | _: SyntaxCheckException => sparkParser.parsePlan(sqlText)
}
}
override def parseExpression(sqlText: String): Expression = sparkParser.parseExpression(sqlText)
override def parseTableIdentifier(sqlText: String): TableIdentifier =

What solution would you like?

  1. Modify the FlintSparkPPLParser class to accept a SparkSession object in its constructor:
 class FlintSparkPPLParser(sparkParser: ParserInterface, sparkSession: SparkSession) extends ParserInterface { // ...} 
  1. Update the FlintPPLSparkExtensions class to pass the SparkSession object to the FlintSparkPPLParser constructor:
class FlintPPLSparkExtensions extends (SparkSessionExtensions => Unit) {
    override def apply(extensions: SparkSessionExtensions): Unit = {
        extensions.injectParser { (spark, parser) =>
            new FlintSparkPPLParser(parser, spark)
        }
    }
}

Instead of swallowing the PPL parser errors, throw the exception directly if the language type is specified as PPL (from spark conf) in the parsePlan method of FlintSparkPPLParser.

What alternatives have you considered?
An alternative approach would be to pass the PPL exception along with the CatalystPlanContext and determine which exception to throw from downstream components. However, this approach might introduce additional complexity and make the code harder to maintain and understand.

@noCharger noCharger added enhancement New feature or request untriaged labels Jun 27, 2024
@seankao-az seankao-az self-assigned this Jun 27, 2024
@YANG-DB YANG-DB added the Lang:PPL Pipe Processing Language support label Aug 6, 2024
@YANG-DB YANG-DB moved this to Todo in PPL Commands Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Lang:PPL Pipe Processing Language support
Projects
Status: Todo
Development

No branches or pull requests

3 participants