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

Query.toAST invalid param reference #222

Open
H-Plus-Time opened this issue Aug 25, 2021 · 1 comment
Open

Query.toAST invalid param reference #222

H-Plus-Time opened this issue Aug 25, 2021 · 1 comment

Comments

@H-Plus-Time
Copy link

So I've been testing out serialising a query to the AST representation, it looks like there's a slight bug when referencing params (serializing them is fine). So this works:
query().params({param1: 3}).filter((d, $) => d.colA}).toAST()
but including the param in any expression fails:
query().params({param1: 3}).filter((d, $) => d.colA / $.param1 }).toAST()

Presumably all verbs that boil down to an ExprObject exhibit the same problem.

Calling aq.internal.parse directly with a fake table in opt works:

const fakeTable = {
  params: () => ({param1: 3}),
  column: () => true
}
const compiledAst = aq.internal.parse({expr: `(d, $) => d.colA < $.param1`, {table: fakeTable, ast: true})

I suspect we just need to pass down the query context in query.js, as a fourth parameter to toAST in verb.js, and tweak getParams to pull in params from opt.table or opt.query.

@jheer
Copy link
Member

jheer commented Aug 30, 2021

Thanks for the issue, and for PR #224! I will take a closer look at this when I have time, hopefully in the coming month. I agree that improved parameter passing would be helpful.

p.s. AST generation should still be considered somewhat "experimental" at this point and is subject to change. Some significant modifications may be made to query serialization in Arquero v5.

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