shaie commented on PR #841: URL: https://github.com/apache/lucene/pull/841#issuecomment-1148322035
> Ok, if this is the case, then should we provide more out of the box `FacetSetMatcher` classes (like a `double` and a `long` implementation for starters) Yes absolutely! I wrote somewhere in the comments that I only provided `Long` variants to talk about the API itself, but totally we should include a `Double` variant too. > I was also thinking is that if we are able to read all `bytes[]` into `longs[]`, we can do some stuff with R-trees That's a good point! So let me clarify the impl and my thoughts: originally you implemented it by reading the `byte[]` into a `long[]` and @gsmiller commented that you can compare the bytes directly, so I only flowed with this proposal (I assume it's for efficiency). But if we don't think that comparing bytes is necessary more efficient than converting to longs, but it does limit us in terms of the API in the future and where we'd want to take it to, then by all means let's make the matching API long-based. > I could, for example if they wanted to encode String ordinals here where the ordinals are long, they can write that conversion logic in `writeToLong(String... values)` So they can already do that, by passing the ordinals to the `FacetSet` constructor. I even demonstrated that in the test. Do you see an issue w/ that? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
