MechanicalRabbit / FunSQL.jl

Julia library for compositional construction of SQL queries
https://mechanicalrabbit.github.io/FunSQL.jl
Other
150 stars 6 forks source link

Bug: CTEs are not aliased when used in the query #33

Closed ananis25 closed 2 years ago

ananis25 commented 2 years ago

In the compiled SQL string, the CTE tables are not aliased. This results in an invalid query when doing something like a self join.

Repro

using FunSQL:
    SQLTable, 
    From, Join, Get, As, Select, With,
    render;

const tab = 
    SQLTable(:foo, columns=[:x, :y])

q = From(:foo_plus) |>
    Join(:foo_plus_dup => From(:foo_plus), on = Get.x .== Get.foo_plus_dup.x2) |>
    With(:foo_plus => From(tab) |> Select(Get.x, Get.x .+ 1 |> As(:x2)))
print(render(q))

#=>
WITH "foo_plus_1" ("x", "x2") AS (
  SELECT
    "foo_1"."x",
    ("foo_1"."x" + 1) AS "x2"
  FROM "foo" AS "foo_1"
)
SELECT
  "foo_plus_1"."x",
  "foo_plus_1"."x2"
FROM "foo_plus_1"
JOIN "foo_plus_1" ON ("foo_plus_1"."x" = "foo_plus_1"."x2")
=#

Possible Fix

A From node referencing a CTE subquery is specialized to a FromReference node during the annotation pass. Looking at the assemble routine for FromTable, an alias is created everytime the node is encountered. We could port over the same logic to FromReference.

# current impl
function assemble(n::FromReferenceNode, refs, ctx)
    cte_a = ctx.cte_map[n.over]
    a = unwrap_repl(cte_a.a)

    # remove
    # c = FROM(over = ID(over = cte_a.schema, name = cte_a.name))
    # subs = make_subs(a, cte_a.name)

    # add
    alias = allocate_alias(ctx, cte_a.name)
    c = FROM(over = ID(over = cte_a.schema, name = alias))
    subs = make_subs(a, alias)

    trns = Pair{SQLNode, SQLClause}[]
    for ref in refs
        push!(trns, ref => subs[ref])
    end
    repl, cols = make_repl_cols(trns)
    return Assemblage(c, cols = cols, repl = repl)
end

I would commit a PR but I'm not yet comfortable with the Julia workflow.

xitology commented 2 years ago

Good catch! Fixed in https://github.com/MechanicalRabbit/FunSQL.jl/commit/5583be18c9674b0f74f57fbb636d0a099201ab06.

ananis25 commented 2 years ago

@xitology - curious why the extra aliasing is needed when translating the Knot nodes too (Link ). Could you please mention an example where it would cause a bug?

xitology commented 2 years ago

I believe it's impossible to trigger this bug with a Knot node. Knot is an intermediate node (created by Iterate) that generates a WITH RECURSIVE clause. I updated assemble for KnotNode for consistency so that the generated SQL is the same for both non-recursive WITH and WITH RECURSIVE clauses. However since it is impossible to refer to the Iterate node using From, you cannot use Join to duplicate the node and trigger this bug.