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

[VL] Noisy wrong fallback message after case-class refactor #5880

Closed
Yohahaha opened this issue May 27, 2024 · 4 comments · Fixed by #7113
Closed

[VL] Noisy wrong fallback message after case-class refactor #5880

Yohahaha opened this issue May 27, 2024 · 4 comments · Fixed by #7113
Labels
bug Something isn't working triage

Comments

@Yohahaha
Copy link
Contributor

Backend

VL (Velox)

Bug description

== Fallback Summary ==
(1) LocalTableScan: Gluten does not touch it or does not support it
(7) NoopLeaf: Gluten does not touch it or does not support it
(8) WriteFiles: Gluten does not touch it or does not support it

== Physical Plan ==
Execute InsertIntoHadoopFsRelationCommand (11)
+- VeloxColumnarWriteFiles (10)
   :- ^ WriteFilesExecTransformer (5)
   :  +- ^ InputIteratorTransformer (4)
   :     +- ^ InputAdapter (3)
   :        +- ^ RowToVeloxColumnar (2)
   :           +- ^ LocalTableScan (1)
   +- RowToVeloxColumnar (9)
      +- WriteFiles (8)
         +- NoopLeaf (7)
24/05/27 17:32:18 WARN [main] GlutenFallbackReporter: Validation failed for plan: Execute InsertIntoHadoopFsRelationCommand[QueryId=0], due to: at least one of its children has empty output; at least one of its children has empty output

Each huge refactoring of plan conversion framework bring new concepts for user/developer, and not only need lots time to understanding new code structure and new naming, but also introduce misleading fallback messages.

In above case, I need to check code and UI more carefully to ensure the fallback log from GlutenFallbackReporter is wrong, but I really dont want to.

#5698
#5480

Spark version

None

Spark configurations

No response

System information

No response

Relevant logs

No response

@zhztheplayer
Copy link
Member

I see there is already a list can be used to add exclusions. I can help update that list if you think is needed.

@Yohahaha
Copy link
Contributor Author

I see there is already a list can be used to add exclusions. I can help update that list if you think is needed.

that's great, thank you!

@FelixYBW
Copy link
Contributor

Can we create an exception for each fallback reason? then put the message in the exception? @zhztheplayer Let's have a sync on this.

@zhztheplayer
Copy link
Member

A proposed fix was included in #5698. @Yohahaha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants