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: Move DB agnostic sections out of sqlite database guide #973

Merged
merged 18 commits into from
Nov 8, 2024

Conversation

patricebender
Copy link
Member

@patricebender patricebender commented Jun 3, 2024

With this change, Common Operators and Common Functions have been moved from the sqlite guide to the generic database guide.

The function list is now including all functions which are supported by all cap-js/dbs.

Todos:

  • align with Java colleagues, I think we can make this section a shared one for both stacks
  • how to document the to-be-expected return types?

Also, it would be nice to remove the disclaimer only supported in runtime queries, but for that we first need compiler support for the functions as well as for the operators.

```
> q = SELECT`from ${Books} { ID, title} where ID != null`
> q.toSQL()
{
  sql: `SELECT json_insert('{}','$."ID"',ID,'$."title"',title) as _json_ FROM (SELECT Books.ID,Books.title FROM sap_capire_bookshop_Books as Books WHERE Books.ID is not NULL)`,
  values: []
}
```

but for non-`null` comparisons, `!=` is translated to `<>`:

```
> q = SELECT`from ${Books} { ID, title} where ID != 207`
> q.toSQL()
{
  sql: `SELECT json_insert('{}','$."ID"',ID,'$."title"',title) as _json_ FROM (SELECT Books.ID,Books.title FROM sap_capire_bookshop_Books as Books WHERE Books.ID <> ?)`,
  values: [ 207 ]
}
```
@patricebender patricebender enabled auto-merge (squash) June 3, 2024 12:28
@patricebender patricebender changed the title fix: != null is translated to is not null but != <xpr> is not fix: Move DB agnostic sections out of sqlite database guide Jun 6, 2024
guides/databases.md Outdated Show resolved Hide resolved
guides/databases.md Outdated Show resolved Hide resolved
guides/databases.md Outdated Show resolved Hide resolved
@patricebender
Copy link
Member Author

ping @johannes-vogel @stewsk

@johannes-vogel
Copy link
Contributor

does that also apply to java?

@renejeglinsky
Copy link
Contributor

does that also apply to java?

@johannes-vogel Do you mean we'd need the equivalent docs for Java when moving those docs in the common guide?

@patricebender Is there anything I can support you with?

@patricebender
Copy link
Member Author

does that also apply to java?

@johannes-vogel Do you mean we'd need the equivalent docs for Java when moving those docs in the common guide?

@patricebender Is there anything I can support you with?

I think the "common operators" section must be moved to the generic part of the db guide, I would like to hear the POs opinions here. From my side, this change is fine (I only need to apply the suggestion from Bob).

@johannes-vogel
Copy link
Contributor

does that also apply to java?

@johannes-vogel Do you mean we'd need the equivalent docs for Java when moving those docs in the common guide?
@patricebender Is there anything I can support you with?

I think the "common operators" section must be moved to the generic part of the db guide, I would like to hear the POs opinions here. From my side, this change is fine (I only need to apply the suggestion from Bob).

I was simply wondering whether this is node specific or also applicable for java. at the moment it's tagged as node specific. in general, fine for me:)

guides/databases.md Outdated Show resolved Hide resolved
@renejeglinsky
Copy link
Contributor

@MattSchur @agoerler Could you please check this PR? It's about moving a topic and also what is probably Node/Java specific (or not!)

Comment on lines 328 to 339
### Standard Operators {.impl .node}

The database services guarantee identical behavior of these operators:

* `==`, `=` — with `=` null being translated to `is null`
* `!=`, `<>` — with `!=` translated to `IS NOT` in SQLite, or to `IS DISTINCT FROM` in standard SQL, or to an equivalent polyfill in SAP HANA
* `<`, `>`, `<=`, `>=`, `IN`, `LIKE` — are supported as is in standard SQL
* `||` — concatenation operator

In particular, the translation of `!=` to `IS NOT` in SQLite — or to `IS DISTINCT FROM` in standard SQL, or to an equivalent polyfill in SAP HANA — greatly improves the portability of your code.

> These operators are available for runtime queries, but not in CDS files.
Copy link
Contributor

Choose a reason for hiding this comment

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

The behaviour of the operators is not standardized in CAP and different in CAP Java. There is an open BLI to get to a common understanding: https://github.tools.sap/cap/dev/issues/909

The corresponding documentation for CAP Java is here: https://pages.github.tools.sap/cap/docs/java/working-with-cql/query-api#predicates

> These operators are available for runtime queries, but not in CDS files.


### Functions Mappings for Runtime Queries {.impl .node}
Copy link
Contributor

Choose a reason for hiding this comment

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

We have a general agreement on the semantic of the functions: https://github.tools.sap/cap/dev/issues/1111

We could create a table here to capture which functions are supported by CAP Java and which by CAP Node.js. SImilar to what @patricebender compiled here: https://github.tools.sap/cap/dev/issues/1111#issuecomment-7092917

Copy link
Member Author

Choose a reason for hiding this comment

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

@renejeglinsky As a first step, this list contains all functions which are supported by the node.js runtime. For months this list has been outdated and it was not at the right place of the documentation (those functions are supported by all db services, not only sqlite).

@agoerler what do you think about adding the functions which are supported by the java runtime ({.impl .java}) in a similar fashion? As we already went over the list, I could also contribute this list (assuming the semantics of the functions are the same).

I would probably refrain from using a table in the documentation. The list of supported functions could be toggled instead, for better readability.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think adding the functions using the toggle would be great. Thanks Patrice!

Copy link
Contributor

Choose a reason for hiding this comment

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

@patricebender As this section is toggled for node, should we include the table for Java in this PR or do this as a follow-up?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd be fine with following up in another change to add a java toggle here 👍

> For the SAP HANA functions, both usages are allowed: all-lowercase as given above, as well as all-uppercase.


### Session Variables {.impl .node}
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a completely different API in CAP Java.

Copy link
Contributor

Choose a reason for hiding this comment

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

As this is toggled for Node we could provide the same section for Java. Would be really nice to have it in this PR but if capacity doesn't allow for that, I don't think that would be a blocker for this change, or?

guides/databases.md Outdated Show resolved Hide resolved

### Functions Mappings for Runtime Queries {.impl .node}

A specified set of standard functions is supported in a **database-agnostic**, hence portable way, and translated to database-specific variants or polyfills.
Copy link
Contributor

Choose a reason for hiding this comment

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

Another thing to consider. These standard functions are not only database-agnostic but even data-source-agnostic. They can also be used with remote OData. Moreover, they there names, semantics and parameters are closer to OData then to SQL. Maybe we should move this section out of database.md and rather to a CDS QL section.

Copy link
Contributor

Choose a reason for hiding this comment

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

@johannes-vogel What do you think?

@patricebender
Copy link
Member Author

@stewsk @johannes-vogel could you kindly review

@agoerler
Copy link
Contributor

agoerler commented Nov 6, 2024

@agoerler what do you think about adding the functions which are supported by the java runtime ({.impl .java}) in a similar fashion?

For CAP Java, the set of supported functions is documented here:

As mentioned earlier, these functions can be used on the DBs but also in remote OData consumption. We could add a link in an ({.impl .java}) variant. Maybe in a follow-up PR).

@patricebender
Copy link
Member Author

@renejeglinsky how can I fix the broken anchor links mentioned in the build:

in: /releases/jun24
Unresolved hash link: /guides/databases-sqlite #standard-operators
Unresolved hash link: /guides/databases-sqlite #standard-functions
Unresolved hash link: /guides/databases-sqlite #sap-hana-functions

I can't find them in the repository :(

@patricebender patricebender merged commit 3d19fbc into main Nov 8, 2024
4 checks passed
@patricebender patricebender deleted the patricebender-patch-1 branch November 8, 2024 05:33
renejeglinsky pushed a commit that referenced this pull request Nov 8, 2024
…973)

* fix: `!= null` is translated to `is not null` but `!= <xpr>` is not

```
> q = SELECT`from ${Books} { ID, title} where ID != null`
> q.toSQL()
{
  sql: `SELECT json_insert('{}','$."ID"',ID,'$."title"',title) as _json_ FROM (SELECT Books.ID,Books.title FROM sap_capire_bookshop_Books as Books WHERE Books.ID is not NULL)`,
  values: []
}
```

but for non-`null` comparisons, `!=` is translated to `<>`:

```
> q = SELECT`from ${Books} { ID, title} where ID != 207`
> q.toSQL()
{
  sql: `SELECT json_insert('{}','$."ID"',ID,'$."title"',title) as _json_ FROM (SELECT Books.ID,Books.title FROM sap_capire_bookshop_Books as Books WHERE Books.ID <> ?)`,
  values: [ 207 ]
}
```

* Update guides/databases-sqlite.md
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.

7 participants