Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: losing aliasMap in 'partial' subqueries #2405

Merged
merged 3 commits into from
May 17, 2023

Conversation

falkenhawk
Copy link
Contributor

@falkenhawk falkenhawk commented Apr 22, 2023

ported from ovos#13

This fixes an issue with 'partial' subqueries that they lose aliasMap from q._context (QueryBuilder's "internal context") while generating nested where sql part. QueryBuilder.subqueryOf() used to grab context from "parent" query, but it doesn't anymore. (since 2019-02-13) 524e0be

In our case, we were getting SQL errors where alias was necessary to make it work, but it was missing

ER_NON_UNIQ_ERROR: Column 'event_id' in IN/ALL/ANY subquery is ambiguous

Basically reintroducing what was deleted in 524e0be (to fix #1205) but wrap merging context with a condition that it should only apply to "partial" subqueries + reverse the sequence that isPartial() is set before subqueryOf()

This fixes an issue with 'partial' suibqueries that they lose `aliasMap` from `q._context`
(QueryBuilder's "internal context") while generating nested `where` sql part.
`QueryBuilder.subqueryOf()` used to grab context from "parent" query, but it doesn't anymore. (since 2019-02-13)
Vincit@524e0be

Basically reintroducing what was deleted in Vincit@524e0be but wrap merging context with a condition that it should only apply to "partial" subqueries + reverse the sequence that `isPartial()` is set before `subqueryOf()`
lib/utils/objectUtils.js Outdated Show resolved Hide resolved
Co-authored-by: Elan Ruusamäe <glen@pld-linux.org>
@lehni
Copy link
Collaborator

lehni commented Apr 24, 2023

@falkenhawk great thanks! this looks reasonable. Adding a test would be good to prevent a future regression?

with error: `no such column: model2.model2_prop2`
@falkenhawk
Copy link
Contributor Author

@lehni I've added a test which fails with

DBError: select `m1`.* from `model2` as `m1` where (`id_col` = (select `m2`.`id_col` from `model2` as `m2` where `m2`.`model2_prop2` = `model2`.`model2_prop2`)) order by `m1`.`model2_prop2` asc - SQLITE_ERROR: no such column: model2.model2_prop2

on the master branch.

Perhaps the ${query.tableRefFor(query.modelClass())}.model2_prop2 reference look weird, but it is useful e.g. for modifiers which add a where condition with subquery, referencing a column from parent query, but the alias which parent query uses is not known and cannot be hardcoded in the modifier.

@lehni lehni merged commit af48971 into Vincit:master May 17, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Subqueries break upgrading from 1.4.0 to 1.5.3
3 participants