Skip to content

Conversation

@MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Jul 29, 2022

What changes were proposed in this pull request?

  1. Replace Option[QueryContext] by Array[QueryContext] in Spark exceptions like in SparkRuntimeException.
  2. Pass SQLQueryContext to QueryExecutionErrors functions instead of Option[SQLQueryContext].
  3. Add the methods getContextOrNull() and getContextOrNullCode() to SupportQueryContext to get a SQL query context or null (if it is missed) of an expression.

Why are the changes needed?

  1. The changes will allow to chain multiple error contexts in Spark's exception. For instance, if user's query refers a view v1, v1 refers another view v2, and v2 does a division. The error contexts will be: sql fragment of v2 that does division -> sql fragment of v1 that refers v2 -> sql fragment of your query that refers v1.
  2. Passing SQLQueryContext to QueryExecutionErrors directly simplifies codegen code because it allows to avoid construction of Scala objects like scala.None.

Does this PR introduce any user-facing change?

Yes, this PR changes user-facing exceptions.

How was this patch tested?

By running the modified test suites:

$ build/sbt "test:testOnly *DecimalExpressionSuite"

and potentially affected tests:

$ build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite"

@MaxGekk MaxGekk changed the title [WIP][SQL] Put QueryContext to array instead of Option [SPARK-39923][SQL] Put QueryContext to array instead of Option Jul 29, 2022
@MaxGekk MaxGekk marked this pull request as ready for review July 29, 2022 11:44
@MaxGekk MaxGekk requested a review from cloud-fan July 29, 2022 11:44
@MaxGekk
Copy link
Member Author

MaxGekk commented Jul 29, 2022

@cloud-fan @gengliangwang @anchovYu Could you review this PR, please.

@MaxGekk MaxGekk changed the title [SPARK-39923][SQL] Put QueryContext to array instead of Option [SPARK-39923][SQL] Multiple query contexts in Spark exceptions Aug 1, 2022
@MaxGekk
Copy link
Member Author

MaxGekk commented Aug 1, 2022

@cloud-fan @gengliangwang @anchovYu Could you review this PR, please.

@MaxGekk
Copy link
Member Author

MaxGekk commented Aug 1, 2022

Merging to master. Thank you, @cloud-fan @gengliangwang for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants