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.

Sign up for GitHub

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

Jump to bottom

fix: groupby path expression with overlapping identifier #992

Merged
merged 8 commits into from
Jan 27, 2025
Merged

fix: groupby path expression with overlapping identifier #992

merged 8 commits into from
Jan 27, 2025

Conversation

PDT42
Copy link
Contributor

@PDT42 PDT42 commented Jan 23, 2025 •
edited
Loading

Currently, the HANA service is unable to GROUP BY foobar.foobar, raising an error like invalid column name: foobar.foobar, if the query includes an aggregation and sql syntax error: incorrect syntax near "FROM", if it does not. The problem is caused by the overlapping column names and the HANA service not correctly injecting the column groupby requires into the parent select - if the parent includes a column with the same column_name as the 'grouped-by' column.

This fix enables the required injection, by skipping the overlapping names check for columns that contain a select.

@PDT42 PDT42 requested a review from BobdenOs January 23, 2025 14:44
@PDT42 PDT42 changed the title fix: groupby nav property with overlapping identifier results in unknown column name fix: groupby navigation property with overlapping identifier Jan 24, 2025
@PDT42 PDT42 changed the title fix: groupby navigation property with overlapping identifier fix: groupby path expression with overlapping identifier Jan 24, 2025
PDT42 and others added 3 commits January 24, 2025 16:45
@PDT42 @BobdenOs
fix: association modeling
fa3905d 
Co-authored-by: Bob den Os <108393871+BobdenOs@users.noreply.github.com>
@PDT42 @BobdenOs
feat: use modern api for running cql query
6234f76 
Co-authored-by: Bob den Os <108393871+BobdenOs@users.noreply.github.com>
@PDT42
feat: use modern api for running cql query
fdf85c5
@johannes-vogel johannes-vogel enabled auto-merge (squash) January 27, 2025 15:24
@johannes-vogel johannes-vogel merged commit b579da8 into main Jan 27, 2025
3 of 4 checks passed
@johannes-vogel johannes-vogel deleted the fix-groupby-navigation-duplicate-identifier branch January 27, 2025 15:32
@cap-bots cap-bots mentioned this pull request Jan 27, 2025
Merged
johannes-vogel added a commit that referenced this pull request Jan 28, 2025
@cap-bots @johannes-vogel
chore: release main (#961)
0b1b533 
🤖 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 <johannes.vogel@sap.com>
This was referenced Jan 28, 2025
Closed
Merged
johannes-vogel added a commit that referenced this pull request Jan 28, 2025
@cap-bots @johannes-vogel
chore: release main (#999)
c8284d1 
🤖 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 <johannes.vogel@sap.com>
This was referenced Jan 28, 2025
Closed
Merged
johannes-vogel added a commit that referenced this pull request Jan 28, 2025
@cap-bots @johannes-vogel
chore: release main (#1001)
6c4b539 
🤖 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 <johannes.vogel@sap.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BobdenOs BobdenOs BobdenOs left review comments

@johannes-vogel johannes-vogel johannes-vogel approved these changes

@danjoa danjoa Awaiting requested review from danjoa danjoa is a code owner

@stewsk stewsk Awaiting requested review from stewsk stewsk is a code owner

@patricebender patricebender Awaiting requested review from patricebender patricebender is a code owner

@sjvans sjvans Awaiting requested review from sjvans sjvans is a code owner

@David-Kunz David-Kunz Awaiting requested review from David-Kunz David-Kunz is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@PDT42 @johannes-vogel @BobdenOs