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: improve != and == implementation for @cap-js/hana #426

Merged
merged 11 commits into from
Jan 31, 2024
Merged

fix: improve != and == implementation for @cap-js/hana #426

merged 11 commits into from
Jan 31, 2024

Conversation

BobdenOs
Copy link
Contributor

No description provided.

@BobdenOs BobdenOs marked this pull request as ready for review January 29, 2024 12:12
@BobdenOs BobdenOs requested a review from sjvans January 29, 2024 12:12
sjvans
sjvans reviewed Jan 29, 2024
View reviewed changes
hana/lib/HANAService.js Outdated
* @param {} xpr
* @returns
*/
comparerator({ xpr }, start) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
comparerator({ xpr }, start) {
comparator({ xpr }, start) {

or maybe is_comparator based on the docs comment?

sjvans
sjvans approved these changes Jan 29, 2024
View reviewed changes
Copy link
Contributor

@sjvans sjvans left a comment

Choose a reason for hiding this comment

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

lgtm

danjoa
danjoa reviewed Jan 29, 2024
View reviewed changes
hana/lib/HANAService.js
// Maps the compare operators to what to return when both sides are null
const compareOperators = {
'=': true,
'==': true,
Copy link
Contributor

Choose a reason for hiding this comment

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

This would mean to change what we decided and specified?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this change makes it as followed:

xpr sql
A = NULL A IS NULL
A = B A = B
A == NULL A IS NULL
A == B (A = B || A IS NULL AND B IS NULL)

As part of the reported issue it was determined that A = NULL was not being properly converted to A IS NULL. That is now resolved by calling super.operator. Which will return IS or IS NOT when applicable. Just like it is done for SQLite.

Hope that makes this a bit more clear.

hana/lib/HANAService.js
@@ -636,19 +636,22 @@ class HANAService extends SQLService {
}

where(xpr) {
return this.xpr({ xpr }, ' = TRUE')
return this.xpr({ xpr: [...xpr, 'THEN'] }).slice(0, -4)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hard to understand → What is that doing?

@BobdenOs BobdenOs mentioned this pull request Jan 30, 2024
Closed
@johannes-vogel johannes-vogel added the next release pr to be checked for next release label Jan 30, 2024
johannes-vogel
johannes-vogel approved these changes Jan 31, 2024
View reviewed changes
Copy link
Contributor

@johannes-vogel johannes-vogel left a comment

Choose a reason for hiding this comment

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

approve for release and will add it to agenda of next db sync

@johannes-vogel johannes-vogel enabled auto-merge (squash) January 31, 2024 14:51
@johannes-vogel johannes-vogel merged commit 9b7b5a0 into main Jan 31, 2024
4 checks passed
@johannes-vogel johannes-vogel deleted the hana-lean-draft branch January 31, 2024 14:57
@cap-bots cap-bots mentioned this pull request Jan 31, 2024
Merged
patricebender pushed a commit that referenced this pull request Feb 2, 2024
@cap-bots @sjvans
chore: release main (#393)
cbb584c 
🤖 I have created a release *beep* *boop*
---


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

##
[1.6.0](db-service-v1.5.1...db-service-v1.6.0)
(2024-02-02)


### Added

* Add fallback for @cap-js/hana for unknown entities
([#403](#403))
([e7dd6de](e7dd6de))
* SELECT returns binaries as Buffers
([#416](#416))
([d4240d5](d4240d5))
* SELECT returns LargeBinaries as streams unless feature flag
"stream_compat" is set
([#251](#251))
([8165a4a](8165a4a))
* strict mode to validate input for `INSERT`, `UPDATE` and `UPSERT`
([#384](#384))
([4644483](4644483))
* Support Readable Streams inside INSERT.entries
([#343](#343))
([f6faf89](f6faf89))


### Fixed

* **`cqn4sql`:** only transform list if necessary
([#438](#438))
([8a7ec65](8a7ec65))
* always generate unique subquery aliases
([#435](#435))
([c875b7d](c875b7d))
* consider `list` in `from.where`
([#429](#429))
([3288e94](3288e94))
* **cqn2sql:** $user.locale refs
([#431](#431))
([ec55276](ec55276))
* **cqn4sql:** expand structured keys in on-conditions
([#421](#421))
([b1e0677](b1e0677))
* Do not generate UUIDs for association keys
([#398](#398))
([9970e14](9970e14))
* enumeration issue with session context in @cap-js/hana
([#399](#399))
([8106a20](8106a20))
* make @cap-js/sqlite work with better-sqlite3@9.3.0
([#422](#422))
([44c0a59](44c0a59))
* pass context of navigation for list within infix filter
([#433](#433))
([0ca077f](0ca077f))
* Restore former deep upsert behavior / error
([#406](#406))
([284b1e3](284b1e3))
* Skip virtual fields on UPSERTs
([#405](#405))
([1a05dcb](1a05dcb))
* sqlite date string compatibility parsing only for valid dates
([#410](#410))
([2a8bb2d](2a8bb2d)),
closes [#409](#409)
* UPSERT for @cap-js/hana for entities with multiple keys
([#418](#418))
([9bbac6e](9bbac6e))
</details>

<details><summary>sqlite: 1.5.0</summary>

##
[1.5.0](sqlite-v1.4.0...sqlite-v1.5.0)
(2024-02-02)


### Added

* SELECT returns LargeBinaries as streams unless feature flag
"stream_compat" is set
([#251](#251))
([8165a4a](8165a4a))
* Support Readable Streams inside INSERT.entries
([#343](#343))
([f6faf89](f6faf89))


### Fixed

* config in streaming test with compat flag
([#412](#412))
([335a178](335a178))
* Do not generate UUIDs for association keys
([#398](#398))
([9970e14](9970e14))
* make @cap-js/sqlite work with better-sqlite3@9.3.0
([#422](#422))
([44c0a59](44c0a59))
* sqlite date string compatibility parsing only for valid dates
([#410](#410))
([2a8bb2d](2a8bb2d)),
closes [#409](#409)
* UPSERT for @cap-js/hana for entities with multiple keys
([#418](#418))
([9bbac6e](9bbac6e))
</details>

<details><summary>postgres: 1.5.0</summary>

##
[1.5.0](postgres-v1.4.1...postgres-v1.5.0)
(2024-02-02)


### Added

* SELECT returns LargeBinaries as streams unless feature flag
"stream_compat" is set
([#251](#251))
([8165a4a](8165a4a))
* Support Readable Streams inside INSERT.entries
([#343](#343))
([f6faf89](f6faf89))


### Fixed

* switch Postgres from json to jsonb
([#402](#402))
([c98a964](c98a964))
* UPSERT for @cap-js/hana for entities with multiple keys
([#418](#418))
([9bbac6e](9bbac6e))
</details>

<details><summary>hana: 0.0.4</summary>

##
[0.0.4](hana-v0.0.3...hana-v0.0.4)
(2024-02-02)


### Added

* Add fallback for @cap-js/hana for unknown entities
([#403](#403))
([e7dd6de](e7dd6de))
* SELECT returns binaries as Buffers
([#416](#416))
([d4240d5](d4240d5))
* SELECT returns LargeBinaries as streams unless feature flag
"stream_compat" is set
([#251](#251))
([8165a4a](8165a4a))
* Support Readable Streams inside INSERT.entries
([#343](#343))
([f6faf89](f6faf89))


### Fixed

* Ensure globally unique aliases with large expand queries
([#396](#396))
([c1df747](c1df747))
* enumeration issue with session context in @cap-js/hana
([#399](#399))
([8106a20](8106a20))
* ignore empty order by
([#392](#392))
([a69fed0](a69fed0))
* improve `!=` and `==` implementation for @cap-js/hana
([#426](#426))
([9b7b5a0](9b7b5a0))
* show clear error message when unable to load project package.json
([#419](#419))
([2ebf783](2ebf783))
* UPSERT for @cap-js/hana for entities with multiple keys
([#418](#418))
([9bbac6e](9bbac6e))
</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: sjvans <30337871+sjvans@users.noreply.github.com>
This was referenced Jan 28, 2025
Closed
Closed
@cap-bots cap-bots mentioned this pull request Jan 28, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danjoa danjoa danjoa approved these changes

@sjvans sjvans sjvans approved these changes

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

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

Assignees
No one assigned
Labels
next release pr to be checked for next release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@BobdenOs @danjoa @sjvans @johannes-vogel