Skip to content

Commit eb52db9

Browse files
authored
Create default values for union during copying / runtime (#5772)
* Fix issue * Small fixes * ci: auto code format * Fix bug * Fix failing test --------- Co-authored-by: CI Bot <[email protected]>
1 parent 0ef7a29 commit eb52db9

File tree

2 files changed

+24
-11
lines changed

2 files changed

+24
-11
lines changed

src/common/types/value/value.cpp

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -172,13 +172,12 @@ Value Value::createDefaultValue(const LogicalType& dataType) {
172172
return Value(dataType.copy(), std::move(children));
173173
}
174174
case LogicalTypeID::MAP:
175-
case LogicalTypeID::LIST: {
176-
return Value(dataType.copy(), std::vector<std::unique_ptr<Value>>{});
177-
}
175+
case LogicalTypeID::LIST:
178176
case LogicalTypeID::UNION: {
179-
std::vector<std::unique_ptr<Value>> children;
180-
children.push_back(std::make_unique<Value>(createNullValue()));
181-
return Value(dataType.copy(), std::move(children));
177+
// We can't create a default value for the union since the
178+
// selected variant is runtime information. Default value
179+
// is initialized when copying (see Value::copyFromUnion).
180+
return Value(dataType.copy(), std::vector<std::unique_ptr<Value>>{});
182181
}
183182
case LogicalTypeID::NODE:
184183
case LogicalTypeID::REL:
@@ -732,19 +731,25 @@ void Value::copyFromUnion(const uint8_t* kuUnion) {
732731
// For union dataType, only one member can be active at a time. So we don't need to copy all
733732
// union fields into value.
734733
auto activeFieldIdx = UnionType::getInternalFieldIdx(*(union_field_idx_t*)unionValues);
735-
auto childValue = children[0].get();
736-
childValue->dataType = childrenTypes[activeFieldIdx]->copy();
734+
// Create default value now that we know the active field
735+
auto childValue = Value::createDefaultValue(*childrenTypes[activeFieldIdx]);
737736
auto curMemberIdx = 0u;
738737
// Seek to the current active member value.
739738
while (curMemberIdx < activeFieldIdx) {
740739
unionValues += storage::StorageUtils::getDataTypeSize(*childrenTypes[curMemberIdx]);
741740
curMemberIdx++;
742741
}
743742
if (NullBuffer::isNull(unionNullValues, activeFieldIdx)) {
744-
childValue->setNull(true);
743+
childValue.setNull(true);
744+
} else {
745+
childValue.setNull(false);
746+
childValue.copyFromRowLayout(unionValues);
747+
}
748+
if (children.empty()) {
749+
children.push_back(std::make_unique<Value>(std::move(childValue)));
750+
childrenSize = 1;
745751
} else {
746-
childValue->setNull(false);
747-
childValue->copyFromRowLayout(unionValues);
752+
children[0] = std::make_unique<Value>(std::move(childValue));
748753
}
749754
}
750755

test/test_files/function/union.test

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,14 @@
99
---- 1
1010
36
1111

12+
-LOG UnionValueOnNestedType
13+
-STATEMENT RETURN union_value(a := struct_pack(b := 123))
14+
---- 1
15+
{b: 123}
16+
-STATEMENT RETURN union_value(a := [12, 34])
17+
---- 1
18+
[12,34]
19+
1220
-LOG UnionValueOnExpr
1321
-STATEMENT MATCH (p:person)-[:knows]->(p1:person) return union_value(age := p.age) AS u1, union_value(age := p1.age) AS u2
1422
---- 14

0 commit comments

Comments
 (0)