+1

Should we be defensive and copy the low/high points before returning them? Also I don't think sortedPackedPointsHashCode should have a getter, and maybe sortedPackedPoints should be returned as a Collection<byte[]> rather exposing PrefixCodedTerms (which should remain internal to Lucene).

Also I don't think sortedPackedPointsHashCode should have a getter,

Oops, totally agree.

I updated the patch. The low/high points are cloned the sortedPackedPoints are exposed as Collection<byte[]>.

Maybe the collection should be a view rather than a copy? For instance this would allow knowing how many points there are in canstant time (by calling size()) and without duplicating all points in memory?

I've updated the patch and the returned collection is now a view.
Also I changed the return type from Collection<byte[]> to Collection<BytesRef> because the sortedPackedPoints iterator already makes a copy and returns that as BytesRef.

Forgot to add an assertion in the test.

Also I changed the return type from Collection<byte[]> to Collection<BytesRef> because the sortedPackedPoints iterator already makes a copy and returns that as BytesRef.

I think the byte[] version was better since it is consitent with PointPangeQuery which exposes the low/high bounds as a byte[]? Also I don't think it is true that the sortedPackedPoints iterator makes a copy: looking at PrefixCodedTerms, it seems to be reusing the same BytesRef object?

Something else we should do is throwing a NoSuchElementException in the Iterator whin upTo==size to comply with the Iterator API.

I think the byte[] version was better since it is consitent with PointPangeQuery which exposes the low/high bounds as a byte[]?

Good point. I'll change that.

Also I don't think it is true that the sortedPackedPoints iterator makes a copy: looking at PrefixCodedTerms, it seems to be reusing the same BytesRef object?

True... we need to make a copy. I guess what I was confused with is the copy that PrefixCodedTerms makes from the input, but if it is reusing the BytesRef it copies into then a copy is required if we return the points in a collection.

Something else we should do is throwing a NoSuchElementException in the Iterator whin upTo==size to comply with the Iterator API.

+1!

I've updated the patch.

Thanks Martijn, +1

Commit 50feae82af5d297b720a98db07ab43e093e788b8 in lucene-solr's branch refs/heads/branch_6x from Martijn van Groningen
[ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=50feae8 ]

LUCENE-7307: Add getters to the PointInSetQuery and PointRangeQuery queries.

Commit b977275185a111fad55da2bd993c1afb73a00b51 in lucene-solr's branch refs/heads/master from Martijn van Groningen
[ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=b977275 ]

LUCENE-7307: Add getters to the PointInSetQuery and PointRangeQuery queries.

Martijn van Groningen can this be closed now?

Michael McCandless yes! Thanks for reminding me.