Summary
.copy() does not fully isolate a query's custom_join state. deepcopy(::SQLObjectQuery)
shallow-copies custom_join (copy(obj.custom_join)), so a query and its .copy() share the
inner per-join Dict objects by reference. Adding another on() / cjoin() on the same join
path to the copy then rewrites the original's join filters in place.
This is the custom_join sibling of the CTE aliasing fixed in #43. #43 fixed the CTE half (ctes
now deep-copied via _copy_ctes, dropping the transient "model"); custom_join was intentionally
left shallow because it is not mutated during execution, so it did not affect #43's
execution-focused acceptance criteria. This issue tracks closing the remaining build-time gap.
Verified evidence
deepcopy(::SQLObjectQuery) (src/querybuilder/types.jl) copies the outer dict but shares the
values:
custom_join=copy(obj.custom_join) # shallow: inner per-join Dicts shared by reference
on() mutates the shared inner dict in place (src/querybuilder/ctes.jl):
existing = get(q.custom_join, join_path, Dict{String,Any}())
existing["filters"] = vcat(existing_filters, parsed_filters) # in-place write into the shared inner dict
existing["join_type"] = join_type_normalized
q.custom_join[join_path] = existing
When join_path already exists, existing is the original's inner dict, so
existing["filters"] = … and existing["join_type"] = … overwrite the original's join
definition. (cjoin() on a duplicate path throws "Join path already exists", so the primary
trigger is a repeated/extended on() on the same path.)
Reproduction shape:
q = M.Result.objects
q.on("driverid", "driverid__nationality" => "British") # existing ON predicate on path "driverid"
q2 = q.copy()
q2.on("driverid", "driverid__nationality" => "Brazilian") # extends the SAME path on the copy
# q (the original) now also carries the "Brazilian" predicate — its join filters were mutated
# because q.custom_join["driverid"] and q2.custom_join["driverid"] are the same object.
Why it matters
.copy() should produce an independent query. #43 established this for CTE state; this is the same
promise for custom joins. It is lower severity than #43 because the trigger is narrower — it only
surfaces at build time (defining an additional join on a copy that shares a join path), never
during plain .list() / execution — but it is the same class of shared-mutable-state footgun and is
cheap to settle before publish while breaking changes are free.
Fix direction
Give custom_join the same per-entry independent copy that ctes got in #43 — a _copy_custom_join
helper mirroring _copy_ctes:
- fresh outer dict and a fresh inner
Dict per join path;
- copy
"filters" into a new vector (on() reassigns the vector, so a shallow copy of the vector
is sufficient — deep-copying the FilterType objects is not required for this bug);
- share the
"field" PormGField reference (it holds Model_Type → Module, which deepcopy cannot
traverse — the same constraint that forced the CTE "query"-only deep copy);
- carry scalar entries (
"join_type") by value.
Acceptance criteria
Notes
Summary
.copy()does not fully isolate a query'scustom_joinstate.deepcopy(::SQLObjectQuery)shallow-copies
custom_join(copy(obj.custom_join)), so a query and its.copy()share theinner per-join
Dictobjects by reference. Adding anotheron()/cjoin()on the same joinpath to the copy then rewrites the original's join filters in place.
This is the
custom_joinsibling of the CTE aliasing fixed in #43. #43 fixed the CTE half (ctesnow deep-copied via
_copy_ctes, dropping the transient"model");custom_joinwas intentionallyleft shallow because it is not mutated during execution, so it did not affect #43's
execution-focused acceptance criteria. This issue tracks closing the remaining build-time gap.
Verified evidence
deepcopy(::SQLObjectQuery)(src/querybuilder/types.jl) copies the outer dict but shares thevalues:
on()mutates the shared inner dict in place (src/querybuilder/ctes.jl):When
join_pathalready exists,existingis the original's inner dict, soexisting["filters"] = …andexisting["join_type"] = …overwrite the original's joindefinition. (
cjoin()on a duplicate path throws"Join path already exists", so the primarytrigger is a repeated/extended
on()on the same path.)Reproduction shape:
Why it matters
.copy()should produce an independent query. #43 established this for CTE state; this is the samepromise for custom joins. It is lower severity than #43 because the trigger is narrower — it only
surfaces at build time (defining an additional join on a copy that shares a join path), never
during plain
.list()/ execution — but it is the same class of shared-mutable-state footgun and ischeap to settle before publish while breaking changes are free.
Fix direction
Give
custom_jointhe same per-entry independent copy thatctesgot in #43 — a_copy_custom_joinhelper mirroring
_copy_ctes:Dictper join path;"filters"into a new vector (on()reassigns the vector, so a shallowcopyof the vectoris sufficient — deep-copying the
FilterTypeobjects is not required for this bug);"field"PormGFieldreference (it holdsModel_Type → Module, whichdeepcopycannottraverse — the same constraint that forced the CTE
"query"-only deep copy);"join_type") by value.Acceptance criteria
q.copy()yields distinct innercustom_joindicts:q2.object.custom_join[path] !== q.object.custom_join[path].on()on a copy does not alter the original'scustom_joinfilters orjoin_type.on(),.copy()it, add anotheron()onthe same path to the copy, assert the original's join definition is unchanged and the two render
different SQL.
cjoin+onbehavior unchanged (no execution-time regression).Notes
to keep the Shared mutable state in read/copy path: .list() mutates the live query, .copy() aliases CTE state #43 PR focused on the execution/CTE contract.
custom_joinis built bycjoin()/on()insrc/querybuilder/ctes.jl; the copy livesin
src/querybuilder/types.jl.