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

SortField.INT / SortField.FLOAT inconsistency with DocValues [LUCENE-3192] #4265

Open
asfimport opened this issue Jun 11, 2011 · 3 comments
Open

Comments

@asfimport
Copy link

asfimport commented Jun 11, 2011

When reviewing sorting by DocValues I found the following naming inconsistency, which shoulc be fixed.

  1. DocValues are always Longs or Doubles, but it uses SortField.INT and SortField.FLOAT
  2. If you enable docValues in SortField but not use not SortField.INT / SortField.FLOAT, it will use FieldCache without informing the user e.g. by Exception

I would wish to fix this in any of the following ways:

  • as a comment TODO notes, create new types for docvalues and remove UseDocValues setter: SortField.INT_DOCVALUES, Sort.FLOAT_DOCVALUES (using the naming INT/FLOAT from the general DOCVALUES API). Ideally there should be also Sort.BYTESREF_DOCVALUES ? This would be more consistent, as getCompataor would be only a big switch as it was before.
  • use more "correct" SortField.LONG and SortField.DOUBLE and throw Exception if doc values is enabled, but a totally different SortField type is used. The Exception can be thrown in SortField.getComparator(). A second problem with SortField.INT instead of LONG is that when you request sort values to be filled into FieldDocs, the type there is suddenly Long, that may be totally confusing.
  • make SortField.LONG==SortField.INT(maybe also ==BYTE==SHORT) use also docvalues if enabled by using the same comparator. If DocValues incompatible type is used, throw Ex in getComparator()

I would prefer solution #1, especially as I dont like SortField to be modifiable (useDocValues setter...). Solution #2 is also fine.


Migrated from LUCENE-3192 by Uwe Schindler (@uschindler), updated May 05 2012
Linked issues:

@asfimport
Copy link
Author

Uwe Schindler (@uschindler) (migrated from JIRA)

This comment is not really related to this issue, but could be done together:

We should make SortField types an ENUM in trunk. We can change API and those integer constants are type unsafe.

For most code this change would not lead to compile failures, only code that assigns the types to a variable/field. In all other cases code like new SortField("field", SortField.INT) could work like it was before (if we add replicas of the enum constants to SortField, else it would change to new SortField("field", SortField.Type.INT).

The getComparator method should be moved to the enum and return a comparator without switch statement.

@asfimport
Copy link
Author

Uwe Schindler (@uschindler) (migrated from JIRA)

One more bug: useIndexValues is not used by SortField.equals()/hashCode(). If we go for #1, we can ignore that one. For solutions #2 and #3 it needs to be fixed, too.

@asfimport
Copy link
Author

Uwe Schindler (@uschindler) (migrated from JIRA)

There is SortField.BYTES already there, but no comparator is returned... If we go #1, i would prefer BYTES_DOVAVLUES as name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants