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(UPDATE): only perform subselect matching if necessary #989

Merged
merged 1 commit into from
Jan 23, 2025

Conversation

patricebender
Copy link
Member

the issue was that while calculating the UPDATE.elements, we accidentially assumed that the calculated element would lead to a join, hence we did our subselect matching. This logic is not necessary for non SELECT queries --> there we need joins if we do a SELECT * from <entity with calculated element /w path expression>


fix(UPDATE): no assocs in list which matches subquery results

the issue was that while calculating the `UPDATE.elements`,
we accidentially assumed that the calculated element would lead to a join,
hence we did our subselect matching. This logic is not necessary for non `SELECT`
queries --> there we need joins if we do a `SELECT * from <entity with calculated element /w path expression>`

---

fix(`UPDATE`): no assocs in list which matches subquery results
@patricebender patricebender merged commit 4bcb88a into main Jan 23, 2025
4 checks passed
@patricebender patricebender deleted the patrice/fix-update branch January 23, 2025 10:48
@cap-bots cap-bots mentioned this pull request Jan 20, 2025
@danjoa
Copy link
Contributor

danjoa commented Jan 23, 2025

I tested it and it works in general 👍, but found one glitch, though:

await UPDATE `Orders.Items` .set `quantity = 0`
await UPDATE `Orders.Items` .set `quantity = 11` .where ({ up__ID: 'Order1' })
await UPDATE `Orders.Items` .set `quantity = 12` .where ({ up_: 'Order1' }) // auto-coercing to up__ID?
await UPDATE `Orders.Items` .set `quantity = 13` .where ({ up_: {ID:'Order1'} }) // FAILS

TODO: → the last one should work as well, shouldn't it?

await UPDATE `Orders['Order1']:Items` .set `quantity = 21`
await UPDATE `Orders[ID='Order1']:Items` .set `quantity = 22`
await UPDATE `Orders[discount=0.2]:Items` .set `quantity = 23`
await UPDATE `Orders.Items` .set `quantity = 31` .where `1=1`
await UPDATE `Orders:Items` .set `quantity = 32`

→ all work as expected 👍

@danjoa
Copy link
Contributor

danjoa commented Jan 23, 2025

P.S. We might want to cove all the cases above also in our tests

johannes-vogel added a commit that referenced this pull request Jan 28, 2025
🤖 I have created a release *beep* *boop*
---


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

##
[1.17.0](db-service-v1.16.2...db-service-v1.17.0)
(2025-01-27)


### Added

* support for cds.Map
([#889](#889))
([cde7514](cde7514))


### Fixed

* **`UPDATE`:** no assocs in list which matches subquery results
([4bcb88a](4bcb88a))
* **`UPDATE`:** only perform subselect matching if necessary
([#989](#989))
([4bcb88a](4bcb88a))
* contains not evaluting to bool
([#980](#980))
([760484b](760484b))
* nested ternary in calculated element
([#981](#981))
([5f4a1fe](5f4a1fe))
* reject virtual elements in all expressions
([#972](#972))
([d0c949d](d0c949d))
* starts endswith for null values
([#975](#975))
([f0330bc](f0330bc))
* use "$$value$$" as internal column name for UPSERT
([#976](#976))
([8c86b86](8c86b86))
</details>

<details><summary>sqlite: 1.8.0</summary>

##
[1.8.0](sqlite-v1.7.8...sqlite-v1.8.0)
(2025-01-27)


### Added

* support for cds.Map
([#889](#889))
([cde7514](cde7514))
</details>

<details><summary>postgres: 1.11.0</summary>

##
[1.11.0](postgres-v1.10.5...postgres-v1.11.0)
(2025-01-27)


### Added

* support for cds.Map
([#889](#889))
([cde7514](cde7514))


### Fixed

* starts endswith for null values
([#975](#975))
([f0330bc](f0330bc))
</details>

<details><summary>hana: 1.6.0</summary>

##
[1.6.0](hana-v1.5.2...hana-v1.6.0)
(2025-01-27)


### Added

* support driver self-wrapping (for Dynatrace)
([#974](#974))
([5346bc4](5346bc4))
* support for cds.Map
([#889](#889))
([cde7514](cde7514))


### Fixed

* Fix expand aliasing
([#959](#959))
([f39097c](f39097c))
* groupby path expression with overlapping identifier
([#992](#992))
([b579da8](b579da8))
* multi raw SQL queries
([#973](#973))
([b953480](b953480))
* Remove `orderBy` ref check for `sql_simple_queries`
([#964](#964))
([1b77725](1b77725))
</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: Johannes Vogel <[email protected]>
This was referenced Jan 28, 2025
johannes-vogel added a commit that referenced this pull request Jan 28, 2025
🤖 I have created a release *beep* *boop*
---


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

##
[1.17.0](db-service-v1.16.2...db-service-v1.17.0)
(2025-01-28)


### Added

* support for cds.Map
([#889](#889))
([cde7514](cde7514))


### Fixed

* **`UPDATE`:** no assocs in list which matches subquery results
([4bcb88a](4bcb88a))
* **`UPDATE`:** only perform subselect matching if necessary
([#989](#989))
([4bcb88a](4bcb88a))
* contains not evaluting to bool
([#980](#980))
([760484b](760484b))
* nested ternary in calculated element
([#981](#981))
([5f4a1fe](5f4a1fe))
* reject virtual elements in all expressions
([#972](#972))
([d0c949d](d0c949d))
* starts endswith for null values
([#975](#975))
([f0330bc](f0330bc))
* use "$$value$$" as internal column name for UPSERT
([#976](#976))
([8c86b86](8c86b86))
</details>

<details><summary>sqlite: 1.8.0</summary>

##
[1.8.0](sqlite-v1.7.8...sqlite-v1.8.0)
(2025-01-28)


### Added

* support for cds.Map
([#889](#889))
([cde7514](cde7514))
</details>

<details><summary>postgres: 1.11.0</summary>

##
[1.11.0](postgres-v1.10.5...postgres-v1.11.0)
(2025-01-28)


### Added

* support for cds.Map
([#889](#889))
([cde7514](cde7514))


### Fixed

* starts endswith for null values
([#975](#975))
([f0330bc](f0330bc))
</details>

<details><summary>hana: 1.6.0</summary>

##
[1.6.0](hana-v1.5.2...hana-v1.6.0)
(2025-01-28)


### Added

* support driver self-wrapping (for Dynatrace)
([#974](#974))
([5346bc4](5346bc4))
* support for cds.Map
([#889](#889))
([cde7514](cde7514))


### Fixed

* Fix expand aliasing
([#959](#959))
([f39097c](f39097c))
* groupby path expression with overlapping identifier
([#992](#992))
([b579da8](b579da8))
* multi raw SQL queries
([#973](#973))
([b953480](b953480))
* Remove `orderBy` ref check for `sql_simple_queries`
([#964](#964))
([1b77725](1b77725))
</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: Johannes Vogel <[email protected]>
This was referenced Jan 28, 2025
johannes-vogel added a commit that referenced this pull request Jan 28, 2025
🤖 I have created a release *beep* *boop*
---


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

##
[1.17.0](db-service-v1.16.2...db-service-v1.17.0)
(2025-01-28)


### Added

* support for cds.Map
([#889](#889))
([cde7514](cde7514))


### Fixed

* **`UPDATE`:** no assocs in list which matches subquery results
([4bcb88a](4bcb88a))
* **`UPDATE`:** only perform subselect matching if necessary
([#989](#989))
([4bcb88a](4bcb88a))
* contains not evaluting to bool
([#980](#980))
([760484b](760484b))
* nested ternary in calculated element
([#981](#981))
([5f4a1fe](5f4a1fe))
* reject virtual elements in all expressions
([#972](#972))
([d0c949d](d0c949d))
* starts endswith for null values
([#975](#975))
([f0330bc](f0330bc))
* use "$$value$$" as internal column name for UPSERT
([#976](#976))
([8c86b86](8c86b86))
</details>

<details><summary>sqlite: 1.8.0</summary>

##
[1.8.0](sqlite-v1.7.8...sqlite-v1.8.0)
(2025-01-28)


### Added

* support for cds.Map
([#889](#889))
([cde7514](cde7514))
</details>

<details><summary>postgres: 1.11.0</summary>

##
[1.11.0](postgres-v1.10.5...postgres-v1.11.0)
(2025-01-28)


### Added

* support for cds.Map
([#889](#889))
([cde7514](cde7514))


### Fixed

* starts endswith for null values
([#975](#975))
([f0330bc](f0330bc))
</details>

<details><summary>hana: 1.6.0</summary>

##
[1.6.0](hana-v1.5.2...hana-v1.6.0)
(2025-01-28)


### Added

* support driver self-wrapping (for Dynatrace)
([#974](#974))
([5346bc4](5346bc4))
* support for cds.Map
([#889](#889))
([cde7514](cde7514))


### Fixed

* Fix expand aliasing
([#959](#959))
([f39097c](f39097c))
* groupby path expression with overlapping identifier
([#992](#992))
([b579da8](b579da8))
* multi raw SQL queries
([#973](#973))
([b953480](b953480))
* Remove `orderBy` ref check for `sql_simple_queries`
([#964](#964))
([1b77725](1b77725))
</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: Johannes Vogel <[email protected]>
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.

3 participants