-
Notifications
You must be signed in to change notification settings - Fork 494
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
feat: index numerical and date fields in Solr with appropriate types + more targeted search result highlighting #10887
base: develop
Are you sure you want to change the base?
Conversation
Additionally, I've set
https://solr.apache.org/guide/solr/latest/query-guide/highlighting.html Two reasons:
With this change, the highlighting is limited to specific fields if the query is: |
@qqmyers Would it make sense to include this feature in the next release? It's a rahter small adaption that improve the serach experience. |
@johannes-darms it's a very cool feature that adds a lot of value, something I've wanted for years. Let's see what @cmbz and @scolapasta think. |
One question - is it possible to enter or have legacy values that don't fit the new types that would break indexing? |
2024/10/15: Added to sprint ready after conversation with @pdurbin |
@qqmyers good question. When I tried to enter to invalid data (non-integer in an integer field, non-float in a float field, or non-date in a date field), I got the following errors via the UI: ...and via the API:
The validator code seems to be quite old, so I don't know if there could be any installations with legacy invalid values entered before it was added. However, looking at the validator code, I found that date fields allow some formats which are not documented: When I add a dataset using one of those formats and try to index it, the indexing fails. The Dataverse log shows an error like So, yes, there may be some installations with date field values using the above formats which would cause invisible datasets due to indexing errors. I am not sure how we should deal with this. Are those date formats intended to be officially fully supported/widely used? If no, it might be OK to offer a workaround in the upgrade instructions like "Please ensure that all date fields containing legacy dates in formats other than If yes, this feature becomes a bit more complicated, because Solr does not support those date formats and we would need to work around that somehow. |
@pdurbin I've just added an API test for the range queries as you suggested. |
I don't know what is required but I wouldn't be surprised if BC dates are something people want and want to have indexed. Perhaps @jggautier would know more about what legacy values exist and what's required. |
It wouldn't be a problem in general. Solr does support BC dates, however in a different format than
https://solr.apache.org/guide/solr/latest/indexing-guide/date-formatting-math.html
Is the code doing checks for API-submitted datasets different from the code doing the UI checks? I assumed it was both the same code I linked above, since the error messages are the same.
Yes, that would be nice. |
Hi all. I haven't been following this issue closely enough to contribute and won't have the time to catch up. But I agree with Jim and encourage folks to look into how others have and are using these fields. My dataset at https://doi.org/10.7910/DVN/2SA6SN might be helpful for seeing who's using the fields in different ways. And the list of contacts in our spreadsheet of Dataverse installations might help for contacting particular installations to learn more. |
Here's the related issue about BC dates: |
…ing invalid ints, floats or dates
I've just pushed a commit that implements the suggestion above (if encountering a bad legacy value in an int/float/date field, just drop that field, but index the rest of the dataset). So, if a dataset contains a bad legacy value in an int/float/date field, this means that queries on that field will not yield that dataset, since the field hasn't been indexed. But for any other query, the dataset will still be found. (I've also added a test showing this) I think this is a small limitation. And we could add BC support relatively easily in a future PR. |
This reflects the following changes: * Integer fields are indexed as `plong` instead of `text_en` * Float fields are indexed as `pdouble` instead of `text_en` * Date fields are indexed as `date_range` (`solr.DateRangeField`) instead of `text_en`
@vera I poked your PR today. I'm very excited about this feature. I started looking into how existing fields will be affected. Please take a look at this PR of suggestions: Also, it's expected that range queries do not produce highlights, right? I'm getting hits but no highlights: |
What this PR does / why we need it:
Currently, all fields regardless of type are indexed in Solr as English text (
text_en
). With this PR, numerical and date fields are indexed in Solr with appropriate types:int
plong
float
pdouble
date
date_range
(solr.DateRangeField
)I chose to index dates as
DateRangeField
because they can be used to represent dates to any precision, e.g. a day YYYY-MM-DD, a month YYYY-MM or a year YYYY. See: Date Formatting and Date Math :: Apache Solr Reference GuideThis matches the allowed formats in a date field as defined by Dataverse.
This means that range queries are now possible on numerical and date fields, e.g.
exampleIntegerField:[25 TO 50]
orexampleDateField:[2000-11-01 TO 2014-12-01]
.Which issue(s) this PR closes:
This PR implements ranged queries as discussed in #370 (issue was already closed)
This issue is related to #8813 and IQSS/dataverse-frontend#278 (the range queries that are now possible lay the groundwork for a nicer search facet UI)
Special notes for your reviewer:
For testing, I've created a sample TSV containing all relevant fields here.
Suggestions on how to test this:
exampleIntegerField:[25 TO 50]
orexampleDateField:[2000-11-01 TO 2014-12-01]
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Facets still look the same as before. There is only a small change in the highlighting of search results, see my comment below
Is there a release notes update needed for this change?:
Yes, there should be an info text describing the new feature + instructions for how to activate the feature:
Additional documentation:
/