Skip to content

Commit 05efed3

Browse files
authored
Fix SQL Vector_Builder to work with deeper structures (#13030)
This fixes a Stack Overflow error occurring when running Snowflake tests. The `Vector_Builder` that is used under the hood in the SQL builder was relying on non-tail recursion, meaning that if the structure to be built was nested too deeply, it could overflow the stack. This PR addresses that by rewriting the `build` method to keep a stack on the heap, thus making the main loop tail-recursive. It adds a tests of `Vector_Builder` that verifies it can build very large structures of various shapes. The test case that builds 4 vectors that total 24000 elements finishes in ~0.5s on my machine.
1 parent e41cb2f commit 05efed3

File tree

3 files changed

+77
-7
lines changed

3 files changed

+77
-7
lines changed

distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Vector_Builder.enso

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,16 @@ type Vector_Builder
5656
build : Vector Any
5757
build self =
5858
Vector.build initial_capacity=self.length vec_builder->
59-
go elem = case elem of
60-
Leaf vec -> vec_builder.append_vector_range vec
61-
Append l r _ ->
62-
go l
63-
go r
64-
go self
59+
go stack = case stack of
60+
List.Nil -> Nothing
61+
List.Cons elem rest ->
62+
case elem of
63+
Leaf vec ->
64+
vec_builder.append_vector_range vec
65+
@Tail_Call go rest
66+
Append l r _ ->
67+
@Tail_Call go (List.Cons l (List.Cons r rest))
68+
go (List.Cons self List.Nil)
6569

6670
## PRIVATE
6771

@@ -94,4 +98,3 @@ type Vector_Builder
9498
- vec: The vector to create a vector builder from.
9599
from_vector : Vector Any -> Vector_Builder Any
96100
from_vector vec = Leaf vec
97-

test/Table_Tests/src/Helpers/Main.enso

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,14 @@ import project.Helpers.Sorted_List_Index_Spec
66
import project.Helpers.SQL_Statement_Serialization
77
import project.Helpers.Unique_Naming_Strategy_Spec
88
import project.Helpers.Value_Type_Spec
9+
import project.Helpers.Vector_Builder_Spec
910

1011
add_specs suite_builder =
1112
Unique_Naming_Strategy_Spec.add_specs suite_builder
1213
Sorted_List_Index_Spec.add_specs suite_builder
1314
SQL_Statement_Serialization.add_specs suite_builder
1415
Value_Type_Spec.add_specs suite_builder
16+
Vector_Builder_Spec.add_specs suite_builder
1517

1618
main filter=Nothing =
1719
suite = Test.build suite_builder->
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
from Standard.Base import all
2+
3+
import Standard.Table.Internal.Vector_Builder.Vector_Builder
4+
5+
from Standard.Test import all
6+
7+
add_specs suite_builder =
8+
suite_builder.group "Vector_Builder" group_builder->
9+
# Tests added because early implementation of Vector_Builder was overflowing the stack for larger vectors
10+
group_builder.specify "should be able to concatenate lots of elements" <|
11+
vb1 = 0.up_to 1000 . fold Vector_Builder.empty acc-> i->
12+
acc ++ [i]
13+
v1 = vb1.build
14+
v1.should_be_a Vector
15+
v1.length . should_equal 1000
16+
v1.should_equal (Vector.new 1000 i->i)
17+
18+
vb2 = 0.up_to 1000 . fold Vector_Builder.empty acc-> i->
19+
Vector_Builder.from_vector [i] ++ acc
20+
v2 = vb2.build
21+
v2.length . should_equal 1000
22+
v2.should_equal (Vector.new 1000 i->i).reverse
23+
24+
vb3 = 0.up_to 10000 . fold Vector_Builder.empty acc-> i->
25+
if i % 2 == 0 then acc ++ ["."] else
26+
Vector_Builder.from_vector ["."] ++ acc
27+
v3 = vb3.build
28+
v3.length . should_equal 10000
29+
v3.should_equal (Vector.new 10000 _->".")
30+
31+
vb4 = vb1 ++ vb2 ++ vb3
32+
v4 = vb4.build
33+
v4.length . should_equal 12000
34+
v4.should_equal ((Vector.new 1000 i->i) + (Vector.new 1000 i->i).reverse + (Vector.new 10000 _->"."))
35+
36+
group_builder.specify "should keep correct ordering" <|
37+
vb1 = 0.up_to 100 . fold Vector_Builder.empty acc-> i->
38+
acc ++ [i, 10*i]
39+
v1 = vb1.build
40+
v1.should_be_a Vector
41+
v1.length . should_equal 200
42+
v1.should_equal (Vector.new 100 i->[i, 10*i]).flatten
43+
44+
vb2 = 0.up_to 100 . fold Vector_Builder.empty acc-> i->
45+
Vector_Builder.from_vector [i, 10*i] ++ acc
46+
v2 = vb2.build
47+
v2.length . should_equal 200
48+
ev2 = Vector.new 100 j->
49+
i = 99 - j
50+
[i, 10*i]
51+
v2.should_equal ev2.flatten
52+
53+
abc = Vector_Builder.from_vector ["a", "b", "c"]
54+
vb3 = 0.up_to 500 . fold Vector_Builder.empty acc-> i->
55+
if i % 2 == 0 then acc ++ abc else
56+
abc ++ acc
57+
v3 = vb3.build
58+
v3.length . should_equal 1500
59+
v3.should_equal (Vector.new 500 _->["a", "b", "c"]).flatten
60+
61+
62+
main filter=Nothing =
63+
suite = Test.build suite_builder->
64+
add_specs suite_builder
65+
suite.run_with_filter filter

0 commit comments

Comments
 (0)