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

[QUESTION] Why entity.association('*') is not same as entity?.association?.('*') in cds-typer? #223

Open
ThePlenkov opened this issue Sep 5, 2024 · 14 comments
Assignees
Labels
bug Something isn't working

Comments

@ThePlenkov
Copy link

Question

HI! I have q very weird issue in my project.

Let's say i have a code like this:

const result = await SELECT.from(OBJECTS, (o) => {
    o.OBJECT_TYPE, o.OBJECT_OID, o.SCHEMA_NAME, o.OBJECT_NAME;
    o.view &&
      o.view((v) => {
        v('*');
        v.columns('*');
      });

By default it won't run because of the TS error:

Cannot invoke an object which is possibly 'undefined'.ts(2722)
(property) columns?: cds.ql.QLExtensions_<VIEW_COLUMNS_> | undefined

So here are two possible ways how to solve it:

  • using condition
 o.view((v) => {
        v('*');
        v.columns && v.columns('*');
      });

and it works

  • using optional notation ( also preferred by AI )
  o.view &&
      o.view((v) => {
        v('*');
        v?.columns?.('*');
      });

and this doesn't work!

Here is what I have in the console:

/workspace/node_modules/@cap-js/db-service/lib/infer/index.js:876
        throw new Error(
              ^
Error: "call" not found in the elements of "VIEWS"
    at stepNotFoundInCombinedElements (/workspace/node_modules/@cap-js/db-service/lib/infer/index.js:876:15)
    at /workspace/node_modules/@cap-js/db-service/lib/infer/index.js:576:13
    at Array.forEach (<anonymous>)
    at inferQueryElement (/workspace/node_modules/@cap-js/db-service/lib/infer/index.js:529:18)
    at handleRef (/workspace/node_modules/@cap-js/db-service/lib/infer/index.js:453:7)
    at /workspace/node_modules/@cap-js/db-service/lib/infer/index.js:331:16
    at Array.forEach (<anonymous>)
    at inferQueryElements (/workspace/node_modules/@cap-js/db-service/lib/infer/index.js:300:15)
    at infer (/workspace/node_modules/@cap-js/db-service/lib/infer/index.js:71:22)
    at resolveExpand (/workspace/node_modules/@cap-js/db-service/lib/infer/index.js:842:42)

Do you know why? Thank you!

@ThePlenkov ThePlenkov added the question Further information is requested label Sep 5, 2024
@ThePlenkov
Copy link
Author

@chgeo do you know why? Can it be a bug? Thanks!

@daogrady daogrady transferred this issue from cap-js/cds-typer Sep 5, 2024
@daogrady
Copy link
Contributor

daogrady commented Sep 5, 2024

Hi Petr,

thanks for filing this issue. I have transferred it to https://github.com/cap-js/cds-types as this is likely related to the types for the cds runtime, and probably has nothing to do with the generation of types for your model (cds-typer).

Can you share the type definition of OBJECTS?

Best,
Daniel

@ThePlenkov
Copy link
Author

@daogrady THank you for looking into the issue. Types are generated - you can see them in a published package: https://www.npmjs.com/package/hana2cds?activeTab=code

Here is the source:
https://github.com/sapops/hana-cli/tree/main/packages/hana2cds/src/cds

Here is the place causing the error:
https://github.com/sapops/hana-cli/blob/main/packages/hana2cds/src/lib/hana2csn.ts#L22

@daogrady
Copy link
Contributor

daogrady commented Sep 5, 2024

@sjvans / @johannes-vogel
user code looks fine on first glance. Nested projections should be allowed:

o.view((v /* type: { columns: Association to many { ... } } */) => {
  v('*');
  v?.columns?.('*');
});

the error seems to be thrown at runtime from the db-service module (see stacktrace in original post). Could you please have a look? Thanks!

@sjvans
Copy link
Contributor

sjvans commented Sep 9, 2024

@patricebender should be able to assist

@patricebender
Copy link
Member

Hi @ThePlenkov,

please provide steps to reproduce the error:

/workspace/node_modules/@cap-js/db-service/lib/infer/index.js:876
        throw new Error(
              ^
Error: "call" not found in the elements of "VIEWS"
    at stepNotFoundInCombinedElements (/workspace/node_modules/@cap-js/db-service/lib/infer/index.js:876:15)
    at /workspace/node_modules/@cap-js/db-service/lib/infer/index.js:576:13
    at Array.forEach (<anonymous>)
    at inferQueryElement (/workspace/node_modules/@cap-js/db-service/lib/infer/index.js:529:18)
    at handleRef (/workspace/node_modules/@cap-js/db-service/lib/infer/index.js:453:7)
    at /workspace/node_modules/@cap-js/db-service/lib/infer/index.js:331:16
    at Array.forEach (<anonymous>)
    at inferQueryElements (/workspace/node_modules/@cap-js/db-service/lib/infer/index.js:300:15)
    at infer (/workspace/node_modules/@cap-js/db-service/lib/infer/index.js:71:22)
    at resolveExpand (/workspace/node_modules/@cap-js/db-service/lib/infer/index.js:842:42)

Thank you
Patrice

@ThePlenkov
Copy link
Author

@patricebender Hi Patrcie, of course. Here it is:

You will see that there will be two tests, one is working ( with conditional guards ) and one is not working with optional notation.

@patricebender
Copy link
Member

@ThePlenkov thanks for providing the sample, I was able to reproduce the issue.

@sjvans i looks like an issue with the query builder API:

  const query = SELECT.from(OBJECTS, (o) => {
    o.OBJECT_TYPE, o.OBJECT_OID, o.SCHEMA_NAME, o.OBJECT_NAME;
    o?.view?.((t) => {
      t("*");
      t?.columns?.("*");
    });
  });
  console.log(JSON.stringify(query, null, 2));

yields:

{
      "SELECT": {
        "from": {
          "ref": [
            "OBJECTS"
          ]
        },
        "columns": [
          {
            "ref": [
              "OBJECT_TYPE"
            ]
          },
          {
            "ref": [
              "OBJECT_OID"
            ]
          },
          {
            "ref": [
              "SCHEMA_NAME"
            ]
          },
          {
            "ref": [
              "OBJECT_NAME"
            ]
          },
          { // from here on, the query looks very wrong 💥
            "ref": [
              "call" 
            ],
            "as": "undefined"
          },
          {
            "ref": [
              "raw"
            ]
          },
          {
            "ref": [
              "0"
            ]
          },
          {
            "ref": [
              null,
              "next",
              "done"
            ],
            "expand": [
              "*"
            ]
          }
        ]
      }
    }

cds.infer rightfully issues an error for the ref: [ "call" ] as there is no element call. Also the following refs look also as something went wrong.

@patricebender patricebender removed their assignment Sep 10, 2024
@ThePlenkov
Copy link
Author

@patricebender may be we can label then it as a bug, not question, thanks!

@ThePlenkov
Copy link
Author

I also thought of something - why is it even optional? I mean if it's non-optional on the type level, then it's not even the error for TS. What could be the cases when some column references are undefined in the projection function?
Thanks!

@patricebender patricebender added bug Something isn't working and removed question Further information is requested labels Sep 10, 2024
@daogrady
Copy link
Contributor

@patricebender can you please assign this issue to someone who is familiar with the query builder api? Thanks!

@sjvans
Copy link
Contributor

sjvans commented Sep 11, 2024

i added it to the team backlog

@David-Kunz David-Kunz self-assigned this Sep 13, 2024
@David-Kunz
Copy link

Hi,

The projection function only allows specific operations:

  • projections are single-argument arrow functions: a => { ... }
  • with the argument as entity alias in column expressions: a.name
  • with functions for nested projections: a.books (b => {...})
  • with * as special case of that: b.* , and b.suppliers('*')
  • with template strings for aliases: b.createdAtas since
  • as well as for infix filters: b.suppliers[city='Paris']

It allows you to statically define which columns of your entity you want to select. Technically, a JavaScript Proxy is used to execute some logic once a certain field is accessed, e.g. in

SELECT.from(MyEntity, e => { e.ID /* <-- field access */ })

There must not be any conditional operator and there shouldn't be a need for it. The corresponding TypeScript type should not complain that some properties might be undefined. It must be possible to follow every navigation of that entity, without checking for its existence. For example, it must be possible to write

const result = await SELECT.from(OBJECTS, (o) => {
    o.OBJECT_TYPE, o.OBJECT_OID, o.SCHEMA_NAME, o.OBJECT_NAME,
    o.view((v) => {
        v('*'),
        v.columns('*');
      });
})

@daogrady , do you think we can make sure that the above code works w.r.t. TypeScript?

@daogrady
Copy link
Contributor

Hi David,

thanks for weighing in on this! I guess we should be able to express that properties, in the context of projections, are not optional. I will look into it.

Best,
Daniel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants