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

.toString() coerces null to string:null #76

Open
rkaw92 opened this issue Dec 21, 2016 · 2 comments
Open

.toString() coerces null to string:null #76

rkaw92 opened this issue Dec 21, 2016 · 2 comments

Comments

@rkaw92
Copy link

rkaw92 commented Dec 21, 2016

Simple test case:

'use strict';

const Query = require('rql/query').Query;
const testPredicate = new Query().eq('foo', null);
console.log(testPredicate.toString());

Expected output:
eq(foo,null)

Actual output:
eq(foo,string:null)

Going to fix this in greatcare/rql, so that null values no longer get coerced. This is probably due to the special-case treatment of null in the first lines of query's encodeValue.

rkaw92 added a commit to greatcare/rql that referenced this issue Dec 21, 2016
This also includes a refactoring of the query module's `encodeValue` function for clarity and easier modification later, if necessary. Comments included.

For persvr#76
@wshager
Copy link
Contributor

wshager commented Dec 21, 2016

@neonstalwart I see no problem with this fix, could you merge it? Who else has write permission nowadays?

@neonstalwart
Copy link
Member

I'm uncertain about my status to contribute to open source. I believe it's probably ok for me to merge something like this (since I'm not the author) but since I'm not certain I'd rather not. I know I can get permission to contribute code I've authored by having my contributions reviewed and approved by my employer before contributing but that extra step is just enough of a burden to make it more effort than I have the time and energy for right now. I'm not sure who else could merge it.

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