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

Wrong identifier #71

Open
ozum opened this issue May 12, 2016 · 7 comments
Open

Wrong identifier #71

ozum opened this issue May 12, 2016 · 7 comments

Comments

@ozum
Copy link

ozum commented May 12, 2016

Hi,

sum(price)=gt=3 query generates following object:

{
    name: 'and',
    args: [
        { name: 'sumgt', args: [ [ 'price' ], 3 ] }
    ],
    cache: {}
}

Shouldn't it be like gt(sum(price),3)

{
    name: 'and',
    args: [{
        name: 'gt',
        args: [ { name: 'sum', args: [ 'price' ] }, 3 ]
    } ],
    cache: {}
}

Or at least it may throw exception.

Best Regards,

@wshager
Copy link
Contributor

wshager commented May 12, 2016

This is not possible in RQL, please see issue #51. I'm working on a solution for this, and will post it soon.

@ozum
Copy link
Author

ozum commented May 12, 2016

Thank you for your response.

Is sum(price)=gt=3 not valid? If it is not valid then, throwing something similar to Invalid RQL exception would be great.

Otherwise users type sum(price)=gt=3 in URL and RQL parser parses this string as in my previous message. Then my library processes parsed tree and sees 'sumgt' and my library throws exception that 'sumgt' is not a function. Users get confused, because they didn't type 'sumgt'.

This seems simple in this example, but in more complex queries it becomes very hard to see what's wrong.

Thanks again,

@wshager
Copy link
Contributor

wshager commented May 12, 2016

@ozum more complex queries are possible in a Turing-complete query language, for instance xquery. AFAIK this was never the goal of RQL. Even if you could write the kind of queries you propose, at some point you'd want to create temporaries and the whole shebang (to not have to type in sum(price) multiple times)...

I don't think you should always expose the entire query to the user though, as that could indeed lead to unexpected results, and might even be considered unsafe in some cases. I usually create a mapping between URI fragments and complex queries. In other words: always provide users with a bunch of understandable links.

@ozum
Copy link
Author

ozum commented May 12, 2016

Thank you.

@ozum ozum closed this as completed May 12, 2016
@rkaw92
Copy link

rkaw92 commented May 12, 2016

I feel that the core issue here is that there are actually two kinds of "operators" in RQL: those that evaluate as a logical sentence for filtering entries in a collection (i.e. true/false predicates) and those that mutate the behaviour / output format of the retrieval in other ways. In my opinion, those should be always kept separate: it is probably pointless to perform a logical "or" on sorting or aggregation directives. Thus, I consider these "non-logical" operators as more of commands, really. To be fair, RQL bugs me a bit because it mixes the two in a single query string (and, in fact, under a common logical AND predicate).

Indeed, I find that there is an implicit parity between what MongoDB (lacking an expression language) can do, and what RQL may represent. Example: in MongoDB, you can do eq(field, constantValue), but not eq(field, field), nor eq(value, value), or even eq(field + 1, value). I suppose this is a desired feature of the language, though I have not seen it stated explicitly.

This may need clearing up in the README. A separation between things that count as predicates and those useful for sorting/limiting/aggregation etc. would also be welcome.

@wshager
Copy link
Contributor

wshager commented May 12, 2016

@rkaw92 I agree. Syntactically it would be correct to add a filter() function, which evaluates the filter. This is indeed how functions are evaluated, as anything but the filter operators is considered top-level.

I'd add this to the parser, but it would break compatibility, so instead I suggest adding a normalization that wraps operators in a filter(), so we can move towards a Turing-complete RQL.

@ozum
Copy link
Author

ozum commented May 13, 2016

Oh, I think I misrepresent my question, I try to rephrase it:

As my understanding of RQL documentation, those two queries are equal RQL queries:

sum(price)=gt=3 is identical to
gt(sum(price),3)

I expect RQL parser generates identical AST for those equal queries, but RQL parser generates two different AST for those equal queries.

There are two possibilities:
A. I'm wrong, those are not equal/identical queries.
B. RQL parser generates wrong AST.

Best Regards,

@ozum ozum reopened this May 13, 2016
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

No branches or pull requests

3 participants