Skip to content

Commit

Permalink
fix literal coredump
Browse files Browse the repository at this point in the history
  • Loading branch information
marin-ma committed Mar 19, 2024
1 parent 31493e1 commit ffeaacc
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import org.apache.spark.sql.catalyst.{AggregateFunctionRewriteRule, FlushableHas
import org.apache.spark.sql.catalyst.analysis.FunctionRegistry.FunctionBuilder
import org.apache.spark.sql.catalyst.catalog.BucketSpec
import org.apache.spark.sql.catalyst.catalog.CatalogTypes.TablePartitionSpec
import org.apache.spark.sql.catalyst.expressions.{Alias, Ascending, Attribute, Cast, CreateNamedStruct, ElementAt, Expression, ExpressionInfo, Generator, GetArrayItem, GetMapValue, GetStructField, If, Inline, IsNaN, JsonTuple, Literal, Murmur3Hash, NamedExpression, NaNvl, PosExplode, Round, StringSplit, StringTrim}
import org.apache.spark.sql.catalyst.expressions.{Alias, Ascending, Attribute, Cast, CreateNamedStruct, ElementAt, Expression, ExpressionInfo, Generator, GetArrayItem, GetMapValue, GetStructField, If, Inline, IsNaN, JsonTuple, Literal, Murmur3Hash, NamedExpression, NaNvl, PosExplode, Round, SortOrder, StringSplit, StringTrim}
import org.apache.spark.sql.catalyst.expressions.aggregate.{AggregateExpression, HLLAdapter}
import org.apache.spark.sql.catalyst.optimizer.BuildSide
import org.apache.spark.sql.catalyst.plans.JoinType
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,5 +136,6 @@ class VeloxLiteralSuite extends VeloxWholeStageTransformerSuite {

test("Literal Fallback") {
validateFallbackResult("SELECT struct(cast(null as struct<a: string>))")
validateFallbackResult("SELECT array(struct(1, 'a'), null)")
}
}
14 changes: 9 additions & 5 deletions cpp/velox/substrait/SubstraitToVeloxExpr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,15 @@ MapVectorPtr makeMapVector(const VectorPtr& keyVector, const VectorPtr& valueVec
RowVectorPtr makeRowVector(
const std::vector<VectorPtr>& children,
std::vector<std::string>&& names,
size_t length,
facebook::velox::memory::MemoryPool* pool) {
std::vector<std::shared_ptr<const Type>> types;
types.resize(children.size());
for (int i = 0; i < children.size(); i++) {
types[i] = children[i]->type();
}
const size_t vectorSize = children.empty() ? 0 : children.front()->size();
auto rowType = ROW(std::move(names), std::move(types));
return std::make_shared<RowVector>(pool, rowType, BufferPtr(nullptr), vectorSize, children);
return std::make_shared<RowVector>(pool, rowType, BufferPtr(nullptr), length, children);
}

ArrayVectorPtr makeEmptyArrayVector(memory::MemoryPool* pool, const TypePtr& elementType) {
Expand All @@ -76,7 +76,7 @@ MapVectorPtr makeEmptyMapVector(memory::MemoryPool* pool, const TypePtr& keyType
}

RowVectorPtr makeEmptyRowVector(memory::MemoryPool* pool) {
return makeRowVector({}, {}, pool);
return makeRowVector({}, {}, 0, pool);
}

template <typename T>
Expand Down Expand Up @@ -423,7 +423,8 @@ VectorPtr SubstraitVeloxExprConverter::literalsToVector(
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_);
return VELOX_DYNAMIC_SCALAR_TYPE_DISPATCH_ALL(
constructFlatVector, kind, elementAtFunc, childSize, veloxType, pool_);
}
case ::substrait::Expression_Literal::LiteralTypeCase::kIntervalDayToSecond:
return constructFlatVector<TypeKind::BIGINT>(elementAtFunc, childSize, INTERVAL_DAY_TIME(), pool_);
Expand Down Expand Up @@ -487,6 +488,9 @@ VectorPtr SubstraitVeloxExprConverter::literalsToVector(
}

RowVectorPtr SubstraitVeloxExprConverter::literalsToRowVector(const ::substrait::Expression::Literal& structLiteral) {
if (structLiteral.has_null()) {
VELOX_NYI("NULL for struct type is not supported.");
}
auto childSize = structLiteral.struct_().fields().size();
if (childSize == 0) {
return makeEmptyRowVector(pool_);
Expand Down Expand Up @@ -537,7 +541,7 @@ RowVectorPtr SubstraitVeloxExprConverter::literalsToRowVector(const ::substrait:
}
}
}
return makeRowVector(vectors, std::move(names), pool_);
return makeRowVector(vectors, std::move(names), 1, pool_);
}

core::TypedExprPtr SubstraitVeloxExprConverter::toVeloxExpr(
Expand Down

0 comments on commit ffeaacc

Please sign in to comment.