-
Notifications
You must be signed in to change notification settings - Fork 21
fix: Error on missing nested SELECT.from.SELECT.from.ref #1114
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
Conversation
I think we have to add a test for this, before this release it was ...(transformedQuery.SELECT.from.ref || /* subq in from */ transformedQuery.SELECT.from.target.name), however, i don't know how it worked ... |
@johannes-vogel before it was: ...(transformedQuery.SELECT.from.ref || /* subq in from */ [transformedQuery.SELECT.from.target.name]), 0828c25#diff-3406bdae94230d4664b3a51d7efa5f3807f2a43095a9089ff018c5a5faf7ca3aL803-L806 It worked because of ...[transformedQuery.SELECT.from.target.name]. |
This part I understood. However, a query is not designed to have a |
Exactly I wondered how that would have worked before. We really should have a test |
@johannes-vogel here is the answer:
|
SELECT.from never has a .target. |
it has a target if we select from another query. I have re-created the issue with the following bookshop query:
this produces a query which select from another query, which has path expressions in it's where clause: [
{
"ref": [
"title"
]
},
"!=",
{
"val": "bar"
},
"and",
{
"ref": [
"author",
"books",
"genre",
"name"
]
},
"=",
{
"val": "Drama"
}
] this leads to a recursive transformation of the subquery in from, resulting in multiple joins for the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is replaced by #1126
→ added tests and thorough fix over there
if a query selects from another query, we need to consider the case where the subquery has path expressions (and therefore joins). The best solution is then to drill down into the innermost query of the from clause, because we do not know the depth. It is also not a good idea to check for the `ref` of the innermost _untransformed_ query, as it is potentially a scoped query (as shown in one of the tests). Moreover, it is not possible to use the `ref` of the _transformed_ query because it might be replaced by join `args`. replaces #1114 fix #1112
This PR was replaced by #1126 |
🤖 I have created a release *beep* *boop* --- <details><summary>db-service: 1.20.0</summary> ## [1.20.0](db-service-v1.19.1...db-service-v1.20.0) (2025-04-17) ### Added * Result set streaming ([#702](#702)) ([2fe02ea](2fe02ea)) ### Fixed * **`expand`:** proper subquery `from` construction ([#1126](#1126)) ([e343e79](e343e79)), closes [#1114](#1114) [#1112](#1112) * Improved support for special characters in column names ([#1141](#1141)) ([ba04697](ba04697)) * **infer:** for localized queries, use `localized.<entity>` as `_target` ([#1140](#1140)) ([b08707b](b08707b)) </details> <details><summary>sqlite: 1.11.0</summary> ## [1.11.0](sqlite-v1.10.0...sqlite-v1.11.0) (2025-04-17) ### Added * Result set streaming ([#702](#702)) ([2fe02ea](2fe02ea)) </details> <details><summary>postgres: 1.14.0</summary> ## [1.14.0](postgres-v1.13.0...postgres-v1.14.0) (2025-04-17) ### Added * Result set streaming ([#702](#702)) ([2fe02ea](2fe02ea)) </details> <details><summary>hana: 1.9.0</summary> ## [1.9.0](hana-v1.8.1...hana-v1.9.0) (2025-04-17) ### Added * enable skip hana wrapping ([#1137](#1137)) ([5e6d61f](5e6d61f)) * Result set streaming ([#702](#702)) ([2fe02ea](2fe02ea)) ### Fixed * Improved support for special characters in column names ([#1141](#1141)) ([ba04697](ba04697)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: D045778 <[email protected]>
db-service
aggregation query throws error #1112 (comment) ?