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

[Feature Request] Make it easier to wrap existing field types #17624

Open
msfroh opened this issue Mar 18, 2025 · 0 comments · May be fixed by #17627
Open

[Feature Request] Make it easier to wrap existing field types #17624

msfroh opened this issue Mar 18, 2025 · 0 comments · May be fixed by #17627
Assignees
Labels
enhancement Enhancement or improvement to existing feature or request Search:Query Capabilities

Comments

@msfroh
Copy link
Collaborator

msfroh commented Mar 18, 2025

Is your feature request related to a problem? Please describe

Talking with @bzhangam about #17575 and opensearch-project/neural-search#1225, we realized that it's kind of tricky/annoying to implement a field type whose role is to "augment" an existing field type.

Describe the solution you'd like

There's a recurring pattern in Lucene (and, as a result, in OpenSearch) where you can wrap something existing but intercept specific methods. If you want to wrap type Foo, you create a public abstract class FilterFoo that takes a Foo and delegates everything to it.

I propose that we add an abstract FilterFieldType class to core that extends MappedFieldType, takes a MappedFieldType, and delegates everything. Subclasses override just the methods that they want to intercept.

Additionally, I would like to add an unwrap method to MappedFieldType that returns this, while FilterFieldType would return the wrapped MappedFieldType (calling unwrap() on it, in the unlikely case that it's also a FilterFieldType). That way, some of our messy instanceof checks on specific field types (which should mostly really be polymorphic "capability" checks exposed through standard MappedFieldType methods) can be changed to check the unwrapped field type (which is a no-op for existing MappedFieldTypes), so the "enhanced" versions can be safely used as stand-ins.

Related component

Search:Query Capabilities

Describe alternatives you've considered

From the discussion/approach in #17575 and opensearch-project/neural-search#1225, we can push a lot of the boilerplate and responsibility out into the plugins.

I worry that risks increasing the API surface area of core a lot, since the plugins end up needing to know a lot more about the types that they're wrapping. I think it would also be extremely brittle, tightly coupling plugins to field types in core.

I believe the proposed approach provides a generic pattern that plugin authors can extend cleanly, while core field type capabilities are managed in core.

Additional context

Examples of Filter* classes in OpenSearch:

There are a lot of examples in Lucene, including FilterDirectoryReader, FilterCodec, FilterLeafCollector, FilterIndexInput, FilterIndexOutput. Actually, we should probably get rid of the OpenSearch FilterIndexOutput and just use the Lucene one.

@msfroh msfroh added enhancement Enhancement or improvement to existing feature or request untriaged labels Mar 18, 2025
@msfroh msfroh self-assigned this Mar 18, 2025
@msfroh msfroh linked a pull request Mar 18, 2025 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement or improvement to existing feature or request Search:Query Capabilities
Projects
Status: 🆕 New
Development

Successfully merging a pull request may close this issue.

2 participants