[GLUTEN-5643] Fix the failure when the pre-project of GenerateExec falls back #6167
Conversation
|
Run Gluten Clickhouse CI |
3 similar comments
|
Run Gluten Clickhouse CI |
|
Run Gluten Clickhouse CI |
|
Run Gluten Clickhouse CI |
marin-ma
force-pushed
the
fix-generator-fallback
branch
from
cbecf17 to
2496f2a
Compare
|
Run Gluten Clickhouse CI |
1 similar comment
|
Run Gluten Clickhouse CI |
marin-ma
changed the title
|
@xumingming Could you help to review? Thanks! |
Sure, will do the first round of review in the recent days. |
@marin-ma I understand Project might fallback(some function not supported?), but I don't understand why we should consider ValueStreamNode as valid, can you elaborate? |
|
@xumingming |
| } else { | ||
| auto name = child->outputType()->names()[index]; | ||
| auto field = child->outputType()->childAt(index); | ||
| std::cout << "name: " << name << "type: " << field->toString() << std::endl; |
There was a problem hiding this comment.
nit: field -> type, remove the std::cout?
| VELOX_CHECK( | ||
| projNode != nullptr && projNode->names().size() > requiredChildOutput.size(), | ||
| std::dynamic_pointer_cast<const core::ProjectNode>(childNode) != nullptr || | ||
| std::dynamic_pointer_cast<const ValueStreamNode>(childNode) != nullptr, | ||
| "injectedProject is true, but the Project is missing or does not have the corresponding projection field") |
There was a problem hiding this comment.
The error message need to be updated to reflect the code change.
|
Run Gluten Clickhouse CI |
Currently, if GenerateExec needs a pre-project, only ProjectNode is allowed in the native substrait -> Velox PlanNode transformation. Query would fail if the pre-project of GenerateExec falls back due to native validation failure. We should consider
ValueStreamNodeas valid in case of pre-project fallback.Introduce
VeloxDummyExpressionfor test purpose. An expression wrapped byVeloxDummyExpressioncan mock the case when a certain expression is supported by Gluten, but it fails on native validation.