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

Multiple field values #9

Closed

Conversation

techphoria414
Copy link

This is a rather substantial change to better support pulling values from index fields that have multiple values stored. (though should be non-breaking to existing implementations). Also updates the crawler to better support crawling of multi-valued fields.

Not 100% sure it's the correct approach to doing this, would definitely be open to feedback.

…h a new property. existing Fields collection would only include the last seen value for a field.
@techphoria414
Copy link
Author

I am going to close this for now, I think I just realized that I didn't need to refactor as aggressively as I did, since NameValueCollection supports multiple values.

… so that we can take advantage of multiple values. added feature to demo site to output/test multiple values.
@techphoria414 techphoria414 reopened this Oct 5, 2012
@techphoria414
Copy link
Author

re-opened this, went back to using NameValueCollection, but had to make some tweaks to how it is used to ensure all field values are available. also, updated Demo pages to show and allow testing of returned values.

@@ -79,9 +82,10 @@
<fieldType name="memo" storageType="NO" indexType="TOKENIZED" vectorType="NO" boost="1f" />
<fieldType name="text" storageType="NO" indexType="TOKENIZED" vectorType="NO" boost="1f" />
<!-- Multilist based fields need to be tokenized to support search of multiple values -->
<fieldType name="multilist" storageType="NO" indexType="TOKENIZED" vectorType="NO" boost="1f" />
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these 3 lines are just for testing, right? Should these be excluded from merge?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well storageType="YES" allows testing of multilist values on Demo page, so up to you as to whether it's included. Safe to exclude.

@alexshyba
Copy link
Owner

I was always wondering if this is the right way of crawling the pipe separated GUID fields. So the way I understand it, this will result in multiple fields with the same name in the document if there is more than one GUID in raw value.
If this works fine, then there is no need in having the multilist, treelist and treelistEx fields be marked as "TOKENIZED". The reason why I had to do it this way is to have field values with more than one GUID be searchable.

@techphoria414
Copy link
Author

I just did a quick test of this, and confirmed that with this addition, the Multilist is searchable as both TOKENIZED and UN_TOKENIZED. The other nice aspect of this is that all values are accessible individually in the SkinnyItem.

@techphoria414
Copy link
Author

Any thoughts on pulling this one in Alex? We utilize it in Active Commerce, but can maintain our own fork if need be.

@alexshyba
Copy link
Owner

Nick, thanks again for your contribution. I think this is gonna work great for multilist fields.
Some notes:

  1. added Checklist field association with the multilist crawler.
  2. made the fields non-stored.
  3. "Show stored values (skinny items only)" became a separate commit.

See details: a081b2c

@alexshyba alexshyba closed this May 5, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants