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

If no data is found return empty set #194

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

123FLO321
Copy link

If we query the db and get an empty set we should return an empty set instead of nil.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@mbarnach
Copy link
Member

I'm re-reading the change, and I'm not so sure about it now.
It is creating a discrepancy between asRows and asResultSet.
If you look at the test TestQueryResult.testQueryResult, you see that we still return nil in that case. This is due to the getRows function, that first check if the asResultSet is nil, before returning the rows. If we want to be consistent, then we should also change asResultSet to return an empty result set, but that doesn't seem very doable.

I agree that empty is more consistent when no records are found, but if you look at the error, it is clear that there is no error.
My opinion is that is should stay like that. @dannys42 do you want to comment/add on this one?

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

3 participants