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

Add blob literal support #900

Merged
merged 8 commits into from
Jul 25, 2017
Merged

Add blob literal support #900

merged 8 commits into from
Jul 25, 2017

Conversation

senderista
Copy link
Contributor

@senderista senderista commented Jul 12, 2017

Fixes #894. This depends on blob literal support in RACO, which has already been merged (uwescience/raco#562).

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 26.778% when pulling 3cf932e on blob_literal into d4fdb71 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 26.8% when pulling 77ab6d7 on blob_literal into d4fdb71 on master.

@jingjingwang
Copy link
Contributor

I only have a high-level question: why would we need BitSetExpression? If this change is only for testing purpose then I don't see why we need to transfer a byte array to a BitSet and eventually to a boolean[].

@senderista
Copy link
Contributor Author

BitsetExpression was just a "cool idea" and seemed like a nice way to test blob types with a flatmap apply. I don't have a specific use case for it, although I hope there will eventually be one :)

@jingjingwang
Copy link
Contributor

Okay, how about calling it something like BlobToBooleanExpression instead of BitSet? A 'BitSet' is only used internally in the Java expression. (I wish there is a way to transfer a ByteBuffer to a boolean[] without using BitSet)

@senderista
Copy link
Contributor Author

Actually the name was deliberate; the other choices might have been "bitvector" or "bit array". The idea is that the input is interpreted as a bitvector (sequence of booleans) rather than a sequence of bytes. I think a name like BlobToBoolean wouldn't convey that interpretation clearly.

@jingjingwang jingjingwang merged commit 0141614 into master Jul 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants