[SPARK] Avoid trigger execution when getting result schema

[wForget]

🔍 Description

Issue References 🔗

DataFrame.isEmpty may trigger execution again, we should avoid it.

Describe Your Solution 🔧

Types of changes 🔖

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Test Plan 🧪

Behavior Without This Pull Request ⚰️

Behavior With This Pull Request 🎉

Related Unit Tests

---

Checklist 📝

• This patch was not authored or co-authored using Generative Tooling

Be nice. Be informative.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 0% with 10 lines in your changes missing coverage. Please review.

Project coverage is 0.00%. Comparing base (7198c72) to head (265f0ec).
Report is 19 commits behind head on master.

Files with missing lines	Patch %	Lines
...kyuubi/engine/spark/operation/SparkOperation.scala	0.00%	5 Missing ⚠️
.../kyuubi/engine/spark/operation/ExecutePython.scala	0.00%	2 Missing ⚠️
...e/kyuubi/engine/spark/operation/ExecuteScala.scala	0.00%	2 Missing ⚠️
...ubi/engine/spark/operation/PlanOnlyStatement.scala	0.00%	1 Missing ⚠️

Additional details and impacted files

@@          Coverage Diff           @@
##           master   #6688   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files         684     684           
  Lines       42237   42277   +40     
  Branches     5755    5765   +10     
======================================
- Misses      42237   42277   +40     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[pan3793]

cc @iodone

[ulysses-you]

If I get it correctly, when we go into thie else if branch, that means the query is in planExcludes ? We return the result rather than the plan. Can we add a new flag to distinguish them ? I'm not sure if the result schema of planExcludes is always empty.

[wForget]

I think we don't need to add a flag to distinguish them, because we handle all cases：

if (result == null) {
    ...
else if (result.schema.isEmpty) {
    ...
} else {
    result.schema
}

[wForget]

And it is consistent with ExecuteStatement

kyuubi/externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/operation/ExecuteStatement.scala

Lines 53 to 58 in 635c793

	override protected def resultSchema: StructType = {
	if (result == null || result.schema.isEmpty) {
	new StructType().add("Result", "string")
	} else {
	result.schema
	}

[ulysses-you]

ah, I see. How about reduce the else if and else by using super.resultSchema() ?

[wForget]

Sounds good, thanks, I will change that

[iodone]

The logic here is no different from before?

[wForget]

It looks no different, result is never assigned in ExecutePython.

[bowenliang123]

Thanks, merged to master (1.10.0) and branch-1.9 (1.9.3).