From ceeb84a69ad123d64dd8699309346042374ab85f Mon Sep 17 00:00:00 2001 From: quinnj Date: Thu, 18 Oct 2018 14:28:18 -0600 Subject: [PATCH 1/6] For nested joins, there was an issue where calls to the inner most source got translated like :(a.(b.c)) while proper access should look like :((a.b).c). This commit adds an invert_getproperty_access translate function that performs the translation --- src/query_translation.jl | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/src/query_translation.jl b/src/query_translation.jl index 14d50802..b87ee1cd 100644 --- a/src/query_translation.jl +++ b/src/query_translation.jl @@ -124,7 +124,6 @@ function query_expression_translation_phase_1(qe) deleteat!(sub_query.args[end].args,6) translate_query(sub_query) - length(sub_query.args)==1 || throw(QueryException("@group ... into subquery too long", sub_query)) qe[1] = :( @from $x in $(sub_query.args[1]) ) @@ -387,13 +386,39 @@ function replace_transparent_identifier_in_anonym_func(ex::Expr, names_to_put_in end end +# takes :(a.(b.c)) and translates to :((a.b).c) +function invert_getproperty_accesses(ex::Expr) + if ex.args[2] isa QuoteNode + return ex + elseif ex.args[2].args[2] isa QuoteNode + return Expr(:., Expr(:., ex.args[1], QuoteNode(ex.args[2].args[1])), ex.args[2].args[2]) + else + return invert_getproperty_accesses(Expr(:., ex.args[1], QuoteNode(ex.args[2].args[1])), ex.args[2].args[2]) + end +end + +function invert_getproperty_accesses(inner::Expr, ex::Expr) + if ex.args[2] isa QuoteNode + return Expr(:., Expr(:., inner, QuoteNode(ex.args[1])), ex.args[2]) + else + inner = Expr(:., inner, QuoteNode(ex.args[1])) + return invert_getproperty_accesses(inner, ex.args[2]) + end +end + function find_names_to_put_in_scope(ex::Expr) names = [] for child_ex in ex.args[2:end] if isa(child_ex,Expr) && child_ex.head==:transparentidentifier child_names = find_names_to_put_in_scope(child_ex) for child_name in child_names - push!(names, (Expr(:., ex.args[1], QuoteNode(child_name[1])), child_name[2])) + c = child_name[1] + if c isa Symbol + xx = (Expr(:., ex.args[1], QuoteNode(c)), child_name[2]) + else + xx = (invert_getproperty_accesses(Expr(:., ex.args[1], c)), child_name[2]) + end + push!(names, xx) end elseif isa(child_ex, Symbol) push!(names,(ex.args[1],child_ex)) From b1a769157814314bde978ba370c4d7c8b8836f7e Mon Sep 17 00:00:00 2001 From: quinnj Date: Thu, 18 Oct 2018 20:26:09 -0600 Subject: [PATCH 2/6] Fix nested join accesses w/ more than 3 joins --- src/query_translation.jl | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/src/query_translation.jl b/src/query_translation.jl index b87ee1cd..be30dd9a 100644 --- a/src/query_translation.jl +++ b/src/query_translation.jl @@ -386,24 +386,12 @@ function replace_transparent_identifier_in_anonym_func(ex::Expr, names_to_put_in end end -# takes :(a.(b.c)) and translates to :((a.b).c) -function invert_getproperty_accesses(ex::Expr) - if ex.args[2] isa QuoteNode - return ex - elseif ex.args[2].args[2] isa QuoteNode - return Expr(:., Expr(:., ex.args[1], QuoteNode(ex.args[2].args[1])), ex.args[2].args[2]) - else - return invert_getproperty_accesses(Expr(:., ex.args[1], QuoteNode(ex.args[2].args[1])), ex.args[2].args[2]) - end -end - -function invert_getproperty_accesses(inner::Expr, ex::Expr) - if ex.args[2] isa QuoteNode - return Expr(:., Expr(:., inner, QuoteNode(ex.args[1])), ex.args[2]) - else - inner = Expr(:., inner, QuoteNode(ex.args[1])) - return invert_getproperty_accesses(inner, ex.args[2]) +function shift_access!(sym::Symbol, ex::Expr) + while ex.args[1] isa Expr + ex = ex.args[1] end + ex.args[1] = Expr(:., sym, QuoteNode(ex.args[1])) + return end function find_names_to_put_in_scope(ex::Expr) @@ -415,8 +403,11 @@ function find_names_to_put_in_scope(ex::Expr) c = child_name[1] if c isa Symbol xx = (Expr(:., ex.args[1], QuoteNode(c)), child_name[2]) + elseif c.args[1] isa Symbol + xx = (Expr(:., Expr(:., ex.args[1], QuoteNode(c.args[1])), c.args[2]), child_name[2]) else - xx = (invert_getproperty_accesses(Expr(:., ex.args[1], c)), child_name[2]) + shift_access!(ex.args[1], c) + xx = (c, child_name[2]) end push!(names, xx) end From 2247a3f153f63558e44f7c453c0daa88d7b76299 Mon Sep 17 00:00:00 2001 From: quinnj Date: Thu, 18 Oct 2018 21:19:18 -0600 Subject: [PATCH 3/6] Add a code comment and a few tests for shift_access --- src/query_translation.jl | 1 + 1 file changed, 1 insertion(+) diff --git a/src/query_translation.jl b/src/query_translation.jl index be30dd9a..b591b890 100644 --- a/src/query_translation.jl +++ b/src/query_translation.jl @@ -386,6 +386,7 @@ function replace_transparent_identifier_in_anonym_func(ex::Expr, names_to_put_in end end +# translates (:a, :((b.c).d)) to :(((a.b).c).d) function shift_access!(sym::Symbol, ex::Expr) while ex.args[1] isa Expr ex = ex.args[1] From 85b87d0cf2d440162ad568bc9621668a7f31b20d Mon Sep 17 00:00:00 2001 From: David Anthoff Date: Tue, 23 Oct 2018 21:17:20 -0700 Subject: [PATCH 4/6] Add test --- test/runtests.jl | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/runtests.jl b/test/runtests.jl index 4dfa13bb..919a5693 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -455,6 +455,18 @@ end @test q["Sally"]==5 @test q["Kirk"]==2 +q = @from i in source_df begin + @let j = i.name + @let k = i.children + @let l = i.age + @select {a=j, b=k, c=l} + @collect DataFrame +end + +@test q.a == ["John", "Sally", "Kirk"] +@test q.b == [3, 5, 2] +@test q.c == [23., 42., 59.] + @test @count(source_df)==3 @test @count(source_df, i->i.children>3)==1 From e1e24b5ef8c02feaf6c1eda0e9994eb2e934e546 Mon Sep 17 00:00:00 2001 From: quinnj Date: Tue, 23 Oct 2018 22:28:10 -0600 Subject: [PATCH 5/6] Add shift_access tests --- test/runtests.jl | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/test/runtests.jl b/test/runtests.jl index 919a5693..e8e0b253 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -32,6 +32,14 @@ end @test !Query.iscall(:(map(1,2,3,4)), :map, 3) end + @testset "shift_access!" begin + ex = Expr(:., :c, QuoteNode(:d)) + Query.shift_access!(:b, ex) + @test ex == :((b.c).d) + Query.shift_access!(:a, ex) + @test ex == :(((a.b).c).d) + end + source_df = DataFrame(name=["John", "Sally", "Kirk"], age=[23., 42., 59.], children=[3,5,2]) q = @from i in source_df begin @@ -455,18 +463,6 @@ end @test q["Sally"]==5 @test q["Kirk"]==2 -q = @from i in source_df begin - @let j = i.name - @let k = i.children - @let l = i.age - @select {a=j, b=k, c=l} - @collect DataFrame -end - -@test q.a == ["John", "Sally", "Kirk"] -@test q.b == [3, 5, 2] -@test q.c == [23., 42., 59.] - @test @count(source_df)==3 @test @count(source_df, i->i.children>3)==1 From 6222079a2c437719754a15577ee2734c43c4bc64 Mon Sep 17 00:00:00 2001 From: David Anthoff Date: Tue, 23 Oct 2018 21:33:32 -0700 Subject: [PATCH 6/6] Add tests --- test/runtests.jl | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/runtests.jl b/test/runtests.jl index e8e0b253..a7ac1249 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -463,6 +463,18 @@ end @test q["Sally"]==5 @test q["Kirk"]==2 +q = @from i in source_df begin + @let j = i.name + @let k = i.children + @let l = i.age + @select {a=j, b=k, c=l} + @collect DataFrame +end + +@test q.a == ["John", "Sally", "Kirk"] +@test q.b == [3, 5, 2] +@test q.c == [23., 42., 59.] + @test @count(source_df)==3 @test @count(source_df, i->i.children>3)==1