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: reject all path expressions w/o foreign keys #806

Merged
merged 17 commits into from
Sep 25, 2024
Merged

Conversation

patricebender
Copy link
Member

reject all path expressions which navigate along a managed association which does not provide foreign keys. In this case, we can't construct join conditions, where exists subqueries or correlated subqueries for expands.

For a draft enabled model:

```
namespace my;

entity Books {
  key ID : Integer;
  title  : String;
  stock  : Integer;
  author : Association to Authors;
}

entity Authors {
  key ID : Integer;
  name   : String;
  alive  : Boolean;
}

service CatalogService {
    @odata.draft.enabled
    @readonly entity Books as projection on my.Books;

    @readonly entity Authors as projection on my.Authors;
}
```

`my.Books` will have a field:
```
        "IsActiveEntity": {
          "type": "cds.Boolean",
          "key": true,
          "default": {
            "val": true
          },
          "@UI.Hidden": true
        },
```

which is marked as virtual by the lean draft implementation.
This virtual key must not be part of the primary key matching
which is done for a path expression in a `UPDATE.where`:

https://github.com/cap-js/cds-dbs/blob/982b8b796da4b577c5641039d2036816209c0437/db-service/test/cqn4sql/UPDATE.test.js#L73-L97

TODO:

- [] test this
reject all path expressions which navigate along a managed association
which does not provide foreign keys. In this case, we can't construct
join conditions, where exists subqueries or correlated subqueries for expands.
@patricebender patricebender marked this pull request as draft September 11, 2024 15:17
Base automatically changed from patrice/draft to main September 12, 2024 07:24
@patricebender patricebender marked this pull request as ready for review September 13, 2024 13:02
db-service/test/cqn4sql/model/keyless.cds Outdated Show resolved Hide resolved
title : String;
stock : Integer;
author : Association to Authors;
authorWithExplicitForeignKey: Association to Authors { ID };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there was a calculated element

  authorName : author.name;

Would selecting authorName in a query have to be handled separately?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, I added this as a test case → does not need to be handled separately :)

@patricebender patricebender enabled auto-merge (squash) September 25, 2024 16:12
@patricebender patricebender merged commit cd271a8 into main Sep 25, 2024
4 of 5 checks passed
@patricebender patricebender deleted the patrice/keyless branch September 25, 2024 16:19
@cap-bots cap-bots mentioned this pull request Sep 25, 2024
johannes-vogel pushed a commit that referenced this pull request Oct 1, 2024
🤖 I have created a release *beep* *boop*
---


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

##
[1.13.0](db-service-v1.12.1...db-service-v1.13.0)
(2024-10-01)


### Added

* Add quoted mode support
([#681](#681))
([43c7a6c](43c7a6c))


### Fixed

* **deep-queries:** properly return insert result
([#803](#803))
([8d800e2](8d800e2))
* dont use virtual key for `UPDATE … where (&lt;key&gt;) in <subquery>`
([#800](#800))
([d25af70](d25af70))
* reject all path expressions w/o foreign keys
([#806](#806))
([cd271a8](cd271a8))
</details>

<details><summary>hana: 1.3.0</summary>

##
[1.3.0](hana-v1.2.0...hana-v1.3.0)
(2024-10-01)


### Added

* Add quoted mode support
([#681](#681))
([43c7a6c](43c7a6c))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
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