philo-he commented on code in PR #13035:
URL: https://github.com/apache/gluten/pull/13035#discussion_r4236655368
##########
cpp/velox/substrait/SubstraitToVeloxExpr.cc:
##########
@@ -498,37 +521,52 @@ VectorPtr SubstraitVeloxExprConverter::literalsToVector(
vector_size_t childSize,
std::function<::substrait::Expression::Literal(vector_size_t /* idx */)>
elementAtFunc) {
auto childTypeCase = childLiteral.literal_type_case();
- switch (childTypeCase) {
- case ::substrait::Expression_Literal::LiteralTypeCase::kNull: {
- auto veloxType = SubstraitParser::parseType(childLiteral.null());
- auto kind = veloxType->kind();
- return VELOX_DYNAMIC_SCALAR_TYPE_DISPATCH_ALL(
- constructFlatVector, kind, elementAtFunc, childSize, veloxType,
pool_);
+ auto child = elementAtFunc(0);
+ if (child.has_null()) {
+ auto nullType = SubstraitParser::parseType(child.null());
+ if (nullType) {
+ switch (nullType->kind()) {
+ case TypeKind::ARRAY:
+ childTypeCase =
::substrait::Expression_Literal::LiteralTypeCase::kList;
+ break;
+ case TypeKind::ROW:
+ childTypeCase =
::substrait::Expression_Literal::LiteralTypeCase::kStruct;
+ break;
+ case TypeKind::MAP:
+ childTypeCase =
::substrait::Expression_Literal::LiteralTypeCase::kMap;
+ break;
+ default:
+ break;
+ }
}
+ }
+ switch (childTypeCase) {
case
::substrait::Expression_Literal::LiteralTypeCase::kIntervalDayToSecond:
return constructFlatVector<TypeKind::BIGINT>(elementAtFunc, childSize,
INTERVAL_DAY_TIME(), pool_);
// Handle EmptyList and List together since the children could be either
case.
case ::substrait::Expression_Literal::LiteralTypeCase::kEmptyList:
- case ::substrait::Expression_Literal::LiteralTypeCase::kList: {
- ArrayVectorPtr elements;
+ case ::substrait::Expression_Literal::LiteralTypeCase::kList: {
Review Comment:
This should violate code format. Could you fix it?
##########
cpp/velox/substrait/SubstraitToVeloxExpr.cc:
##########
@@ -567,24 +611,37 @@ VectorPtr SubstraitVeloxExprConverter::literalsToVector(
}
return rowVector;
}
- default:
+ default: {
+ // if null, get null type instead of next element type
auto veloxType = getScalarType(elementAtFunc(0));
if (veloxType) {
- auto kind = veloxType->kind();
- return VELOX_DYNAMIC_SCALAR_TYPE_DISPATCH(
- constructFlatVector, kind, elementAtFunc, childSize, veloxType,
pool_);
+ // Handle UNKNOWN type (untyped nulls like array(null)) by creating
Review Comment:
Spark actually parse it as NullType. "untyped" seems a bit ambiguous,
perhaps, consider to rephrase the comment like the following:
// Handle UNKNOWN elements (mapped from Spark NullType, as in array(null))
// by creating an all-null vector.
##########
cpp/velox/substrait/SubstraitToVeloxExpr.cc:
##########
@@ -498,37 +521,52 @@ VectorPtr SubstraitVeloxExprConverter::literalsToVector(
vector_size_t childSize,
std::function<::substrait::Expression::Literal(vector_size_t /* idx */)>
elementAtFunc) {
auto childTypeCase = childLiteral.literal_type_case();
- switch (childTypeCase) {
- case ::substrait::Expression_Literal::LiteralTypeCase::kNull: {
- auto veloxType = SubstraitParser::parseType(childLiteral.null());
- auto kind = veloxType->kind();
- return VELOX_DYNAMIC_SCALAR_TYPE_DISPATCH_ALL(
- constructFlatVector, kind, elementAtFunc, childSize, veloxType,
pool_);
+ auto child = elementAtFunc(0);
+ if (child.has_null()) {
Review Comment:
We can directly use `childLiteral.has_null()`. Then, no need to create child
variable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
