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: dont mistake non-key access with foreign key #642

Merged
merged 4 commits into from
May 16, 2024

Conversation

patricebender
Copy link
Member

@patricebender patricebender commented May 15, 2024

Consider this two path expressions:
classrooms.classroom.ID, classrooms.classroom.name

While the first path could be optimized to classrooms.classroom_ID, the second path is not a key access and hence must be transformed to classroom.name, where classroom is the nested join node, refer to this example:

    const query = CQL`SELECT from Pupils {
      ID
    } group by classrooms.classroom.ID, classrooms.classroom.name`
    const expected = CQL`SELECT from Pupils as Pupils
                        left join ClassroomsPupils as classrooms
                          on classrooms.pupil_ID = Pupils.ID
                        left join Classrooms as classroom
                          on classroom.ID = classrooms.classroom_ID
                        {
                          Pupils.ID
                        } group by classroom.ID, classroom.name`

As both paths navigate through the same join nodes, we do not (yet?) optimize the first path to the foreign key, but access the ID in the target of classroom. This is also the behavior of the cds-compiler and should not matter too much as the join node is produced through the second path anyway:

Pupils
  └── classrooms
        └── classroom
              ├── ID
              └── name

The problem was, that when we found the second path, we have already seen this path up to the n - 1 step before, and only the reference for the first path was adjusted to not be a foreign key access anymore.

fixes cap/issue#16001

@patricebender patricebender marked this pull request as ready for review May 16, 2024 13:42
@patricebender patricebender merged commit 2cd2349 into main May 16, 2024
4 checks passed
@patricebender patricebender deleted the patrice/joins-fix branch May 16, 2024 16:07
@cap-bots cap-bots mentioned this pull request May 16, 2024
johannes-vogel pushed a commit that referenced this pull request May 17, 2024
🤖 I have created a release *beep* *boop*
---


<details><summary>db-service: 1.9.1</summary>

##
[1.9.1](db-service-v1.9.0...db-service-v1.9.1)
(2024-05-16)


### Fixed

* dont mistake non-key access with foreign key
([#642](#642))
([2cd2349](2cd2349))
</details>

<details><summary>sqlite: 1.7.1</summary>

##
[1.7.1](sqlite-v1.7.0...sqlite-v1.7.1)
(2024-05-16)


### Fixed

* **deps:** update dependency better-sqlite3 to v10
([#636](#636))
([0cc60e7](0cc60e7))
</details>

<details><summary>hana: 0.4.0</summary>

##
[0.4.0](hana-v0.3.0...hana-v0.4.0)
(2024-05-16)


### Added

* Allow hex engine to be used
([#641](#641))
([bca0c01](bca0c01))


### Fixed

* Improve comparator check for combined and nested expressions
([#632](#632))
([8e1cb4b](8e1cb4b))
* Support multi byte characters
([#639](#639))
([4cfa77f](4cfa77f))


### Changed

* `@sap/hana-client` optional peer dependency
([#631](#631))
([89d7149](89d7149))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
@cap-bots cap-bots mentioned this pull request Jul 18, 2024
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.

2 participants