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

Avoid loading belongsTo relations with falsey keys #52

Closed
wants to merge 3 commits into from
Closed

Avoid loading belongsTo relations with falsey keys #52

wants to merge 3 commits into from

Conversation

robertdp
Copy link

See #51

@robertdp
Copy link
Author

With SQL, local keys for optional relations will most likely be nullable, which results in the query:

SELECT * FROM table WHERE id = ''

Also, the falsey check is already being used for the other relation types, just not belongsTo. Which is why only optional belongsTo relations cause unnecessary queries and Not Found! errors to be thrown.

Without this, js-data-sql will try to query belongsTo relations even
without any valid ids.
@robertdp
Copy link
Author

I missed handling the other half of the belongsTo logic, resulting in queries like:

SELECT * FROM table WHERE 1 = 0

Now these should be discarded without querying.

Edit: These queries were being made by the adapter before my changes, not because of them. This is probably knex translating WHERE id IN () to the equivalent and not invalid WHERE 1 = 0

@jmdobry
Copy link
Member

jmdobry commented Jan 19, 2016

Can you squash into one commit?

@robertdp
Copy link
Author

Give me a minute

@robertdp
Copy link
Author

Struggling with GitHub Desktop...

@robertdp
Copy link
Author

#53

@robertdp robertdp closed this Jan 19, 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

Successfully merging this pull request may close these issues.

None yet

2 participants