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

feat: Add quoted mode support #681

Merged
merged 8 commits into from
Oct 1, 2024
Merged

feat: Add quoted mode support #681

merged 8 commits into from
Oct 1, 2024

Conversation

BobdenOs
Copy link
Contributor

No description provided.

@BobdenOs BobdenOs added the next release pr to be checked for next release label Jun 14, 2024
@BobdenOs BobdenOs marked this pull request as ready for review July 26, 2024 08:46
Copy link
Contributor

@danjoa danjoa left a comment

Choose a reason for hiding this comment

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

What exactly are we doing here?

@@ -34,6 +34,12 @@ class CQN2SQLRenderer {
this.class = new.target // for IntelliSense
this.class._init() // is a noop for subsequent calls
this.model = srv?.model

// Overwrite smart quoting
if (cds.env.cdsc.sqlMapping === 'quoted') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use official setting cds.env.sql.names instead

@ThePlenkov
Copy link

Hi! let me please just share the importance of such a feature. In our app we used quoted on purposes - because we wanted our artifacts to be created using namespace patterns as we used to have in Hana XSC. So now because of this we cannot use @cap-js/hana in our production app without significant refactoring. I'm afraid creating synonyms will not be enough. Therefore - we stay now with CAP native hana driver unless this mode is supported

db-service/lib/cqn2sql.js Outdated Show resolved Hide resolved
@@ -336,7 +342,9 @@ class CQN2SQLRenderer {
const { ref, as } = from
const _aliased = as ? s => s + ` as ${this.quote(as)}` : s => s
if (ref) {
const z = ref[0]
const { target } = SELECT.from(from)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we have to create a new query to get the target?
Can't we just take it from the rendered query?

hana/lib/HANAService.js Show resolved Hide resolved
@ThePlenkov
Copy link

Hi! Is there any progress with this PR? Thanks!

Co-authored-by: Daniel Hutzel <daniel.hutzel@sap.com>
@johannes-vogel johannes-vogel enabled auto-merge (squash) October 1, 2024 12:19
@johannes-vogel johannes-vogel merged commit 43c7a6c into main Oct 1, 2024
4 of 6 checks passed
@johannes-vogel johannes-vogel deleted the feat/quoted-mode branch October 1, 2024 12:27
@cap-bots cap-bots mentioned this pull request Oct 1, 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
next release pr to be checked for next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants