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

sql_where is not a plural key #25

Closed
dwmintz opened this issue Sep 13, 2019 · 2 comments
Closed

sql_where is not a plural key #25

dwmintz opened this issue Sep 13, 2019 · 2 comments
Milestone

Comments

@dwmintz
Copy link

dwmintz commented Sep 13, 2019

To my knowledge, you cannot define more than one sql_where key as part of a JOIN.

https://docs.looker.com/reference/explore-params/sql_where

This line should be deleted:
https://github.com/joshtemple/lkml/blob/master/lkml/keys.py#L35

@joshtemple
Copy link
Owner

Multiple sql_where in a join are allowed by the IDE validator, but it looks like only the last statement is used the SQL query. I find this to be confusing behavior. If only one will be used in the query, the validator should throw an error.

Generally, I like to stick as close to the validator as possible, but in this case, I'm okay removing sql_where from the list of plural keys and having lkml raise an error when multiple are found.

joshtemple added a commit that referenced this issue Sep 13, 2019
Closes #25. Multiple statements of sql_where are allowed by Looker's 
validator. 
However, Looker only uses the last statement in the actual SQL join. 
Normally I prefer to follow the behavior of the Looker validator, but in 
this case, I disagree with the way the validator works (it should raise 
an error for 2 statements supplied if only 1 will be used) and will 
allow lkml to raise an error when there are multiple sql_where 
statements in a single join.
@dwmintz
Copy link
Author

dwmintz commented Sep 14, 2019

I've filed a bug internally about the validator's incorrect accepting of multiple sql_where definitions within the same join. The team has confirmed this is unintended and that they intend to fix. No estimate for when yet.

@joshtemple joshtemple added this to the 0.2.1 milestone Oct 15, 2019
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

2 participants