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

Incorrect syntax generated for property off of select that actually queries correctly #1036

Open
jackfischer opened this issue Jun 6, 2024 · 1 comment

Comments

@jackfischer
Copy link

jackfischer commented Jun 6, 2024

Code

const selectQuery = e.select(e.ActivitySnapshot, () => ({
  id: true,
})).id;
console.log(selectQuery.toEdgeQL()); // see below
console.log(await selectQuery.run(Client)); // [], no exception thrown

Schema

# as applicable to this example
module default {
  type ActivitySnapshot {}
}

Generated EdgeQL

(WITH
  __scope_0_defaultActivitySnapshot := DETACHED default::ActivitySnapshot
SELECT __scope_0_defaultActivitySnapshot {
  id
}).id

Error or desired behavior

The generated EdgeQL throws EdgeQLSyntaxError when executed directly Unexpected '('. It does not throw an error on selectQuery.run and in fact returns "correct" results.

Versions (please complete the following information):

  • OS: macos 14.5
  • EdgeDB version (e.g. 2.0): 4.7
  • EdgeDB CLI version (e.g. 2.0): 5.1.0
  • edgedb-js version (e.g. 0.20.10;): 1.5.6
  • @edgedb/generate version (e.g. 0.0.7;): 0.5.3
  • TypeScript version: 5.4.5
  • Node/Deno version: 20.11.1
@jackfischer jackfischer changed the title Incorrect syntax generated for property off of select that executes against Incorrect syntax generated for property off of select that actually queries correctly Jun 6, 2024
@scotttrinh
Copy link
Collaborator

I think the real issue here is that run knows how to wrap these fragments in a way that sends a valid query. (See

const expr = runnableExpressionKinds.has(this.__kind__)
? this
: wrappedExprCache.get(this) ??
wrappedExprCache.set(this, select(this)).get(this);
)

Maybe we should expose another method like .toRunnableEdgeQL that will do the same thing 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants