Skip to content

Remove @ExperimentalApi annotations from star-tree related code #18521

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Remove @ExperimentalApi annotations from star-tree related code #18521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions server/src/main/java/org/opensearch/search/startree/StarTreeNodeCollector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@

package org.opensearch.search.startree;

import org.opensearch.common.annotation.ExperimentalApi;
import org.opensearch.common.annotation.PublicApi;
import org.opensearch.index.compositeindex.datacube.startree.node.StarTreeNode;

/**
* Collects one or more @{@link StarTreeNode}'s
*/
@ExperimentalApi
@PublicApi(since = "2.18.0")
Copy link
Member

Choose a reason for hiding this comment

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

Can we get rid of PublicApi annotation as well? Classes although public, still shouldn't be annotated PublicApi

I think you will get a trail of classes that might fail to build because they are still marked experimental which is okay to remove experimental as well.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for your review !
However, when I remove the @PublicApi annotation and run the build, I get the following error

error: The element org.opensearch.search.startree.StarTreeNodeCollector is part of the public APIs but is not marked as @PublicApi, @ExperimentalApi or @DeprecatedApi (referenced by org.opensearch.search.startree.filter.DimensionFilter)

How can I resolve this?

Copy link
Contributor

Choose a reason for hiding this comment

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

org.opensearch.search.startree.filter.DimensionFilter too is marked as public api. Can we remove publicApi from the chain and see what's the actual class that could be causing this error ?

Copy link
Author

@happysubin happysubin Jun 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@bharath-techie
I haven’t pushed yet,
I continue removing the PublicApi annotation starting from DimensionFilter,
I end up removing it from the following related classes.
DimensionFilter, DimensionFilterMapper, StarTreeNode, StarTreeFilter, and StarTreeQueryContext

Eventually, this leads to the following error

error: The element org.opensearch.search.startree.StarTreeQueryContext is part of the public APIs but is not marked as @PublicApi, @ExperimentalApi or @DeprecatedApi (referenced by org.opensearch.index.query.QueryShardContext)

Copy link
Contributor

Choose a reason for hiding this comment

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

@sandeshkr419 @expani

I don't see good options other than marking the chain with public API since we have a reference in QueryShardContext.
Any thoughts ?

Copy link
Member

@sandeshkr419 sandeshkr419 Jun 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I strongly don't like the idea of chaining up all the classes as @PublicApi as a workaround.

Thinking out loud some alternatives:

How about introducing a CompositeQueryContext with @PublicApi annotation. Its implementation StarTreeQueryContext can be an internal api or with no annotations. QueryShardContext will then hold CompositeQueryContext instead of StarTreeQueryContext.

I think this can provide a good abstraction since star-tree is also an implementation of composite index. Introducing CompositeQueryContext can hide the star-tree related details and also pave way for future implementations similar to star-tree without having to bundle up query context with additional information.

Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't solve the problem by the way , still the chain of StarTreeQueryContext needs to be PublicApi since CompositeQueryContext is public API.

Copy link
Contributor

Choose a reason for hiding this comment

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

For example if we do the above change , we still have the following chain that needs to be solved

CompositeDataCubeFieldType --> StarTreeQueryContext

Dimension -> CompositeDataCubeFieldType
DimensionDataType -> Dimension
Metric -> CompositeDataCubeFieldType
MetricStat -> Metric

And there is an additional chain :
CompositeIndexSettings --> IndicesService

But it does remove the chain in same package chain - DimensionFilter, StarTreeFilter, DimensionFilterMapper.

Copy link
Member

Choose a reason for hiding this comment

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

Basically exposing a sub-set of interfaces as @PublicAPI is still better (may not be ideal - and we can increment on this) than exposing all the implementations as @PublicAPI.

public interface StarTreeNodeCollector {
/**
* Called to collect a @{@link StarTreeNode}
Expand Down
4 changes: 2 additions & 2 deletions server/src/main/java/org/opensearch/search/startree/StarTreeQueryContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
package org.opensearch.search.startree;

import org.apache.lucene.util.FixedBitSet;
import org.opensearch.common.annotation.ExperimentalApi;
import org.opensearch.common.annotation.PublicApi;
import org.opensearch.index.codec.composite.CompositeIndexFieldInfo;
import org.opensearch.index.compositeindex.datacube.DateDimension;
import org.opensearch.index.compositeindex.datacube.Dimension;
Expand Down Expand Up @@ -38,7 +38,7 @@
/**
* Stores the star tree related context of a search request.
*/
@ExperimentalApi
@PublicApi(since = "2.18.0")
public class StarTreeQueryContext {

private final CompositeDataCubeFieldType compositeMappedFieldType;
Expand Down
1 change: 0 additions & 1 deletion server/src/main/java/org/opensearch/search/startree/StarTreeQueryHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
* Helper class for building star-tree query
*
* @opensearch.internal
* @opensearch.experimental
*/
public class StarTreeQueryHelper {

Expand Down
1 change: 0 additions & 1 deletion server/src/main/java/org/opensearch/search/startree/StarTreeTraversalUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
/**
* Filter operator for star tree data structure.
*
* @opensearch.experimental
* @opensearch.internal
*/
public class StarTreeTraversalUtil {
Expand Down
6 changes: 3 additions & 3 deletions server/src/main/java/org/opensearch/search/startree/filter/DimensionFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

package org.opensearch.search.startree.filter;

import org.opensearch.common.annotation.ExperimentalApi;
import org.opensearch.common.annotation.PublicApi;
import org.opensearch.index.compositeindex.datacube.startree.index.StarTreeValues;
import org.opensearch.index.compositeindex.datacube.startree.node.StarTreeNode;
import org.opensearch.search.internal.SearchContext;
Expand All @@ -19,7 +19,7 @@
/**
* Contains the logic to filter over a dimension either in StarTree Index or it's Dimension DocValues
*/
@ExperimentalApi
@PublicApi(since = "2.18.0")
public interface DimensionFilter {
/**
* Converts parsed user values to ordinals based on segment and other init actions can be performed.
Expand Down Expand Up @@ -59,7 +59,7 @@ default String getMatchingDimension() {
/**
* Represents how to match a value when comparing during StarTreeTraversal
*/
@ExperimentalApi
@PublicApi(since = "2.18.0")
enum MatchType {
GT,
LT,
Expand Down
2 changes: 0 additions & 2 deletions server/src/main/java/org/opensearch/search/startree/filter/ExactMatchDimFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

package org.opensearch.search.startree.filter;

import org.opensearch.common.annotation.ExperimentalApi;
import org.opensearch.index.compositeindex.datacube.Dimension;
import org.opensearch.index.compositeindex.datacube.startree.index.StarTreeValues;
import org.opensearch.index.compositeindex.datacube.startree.node.StarTreeNode;
Expand All @@ -25,7 +24,6 @@
/**
* Handles Term and Terms query like search in StarTree Dimension filtering.
*/
@ExperimentalApi
public class ExactMatchDimFilter implements DimensionFilter {

private final String dimensionName;
Expand Down
2 changes: 0 additions & 2 deletions server/src/main/java/org/opensearch/search/startree/filter/MatchAllFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

package org.opensearch.search.startree.filter;

import org.opensearch.common.annotation.ExperimentalApi;
import org.opensearch.index.compositeindex.datacube.startree.index.StarTreeValues;
import org.opensearch.index.compositeindex.datacube.startree.node.StarTreeNode;
import org.opensearch.index.compositeindex.datacube.startree.node.StarTreeNodeType;
Expand All @@ -21,7 +20,6 @@
/**
* Matches all StarTreeNodes
*/
@ExperimentalApi
public class MatchAllFilter implements DimensionFilter {

public final String dimensionName;
Expand Down
2 changes: 0 additions & 2 deletions server/src/main/java/org/opensearch/search/startree/filter/MatchNoneFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

package org.opensearch.search.startree.filter;

import org.opensearch.common.annotation.ExperimentalApi;
import org.opensearch.index.compositeindex.datacube.startree.index.StarTreeValues;
import org.opensearch.index.compositeindex.datacube.startree.node.StarTreeNode;
import org.opensearch.search.internal.SearchContext;
Expand All @@ -17,7 +16,6 @@
/**
* Filter which matches no StarTreeNodes.
*/
@ExperimentalApi
public class MatchNoneFilter implements DimensionFilter {

@Override
Expand Down
2 changes: 0 additions & 2 deletions server/src/main/java/org/opensearch/search/startree/filter/RangeMatchDimFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

package org.opensearch.search.startree.filter;

import org.opensearch.common.annotation.ExperimentalApi;
import org.opensearch.index.compositeindex.datacube.startree.index.StarTreeValues;
import org.opensearch.index.compositeindex.datacube.startree.node.StarTreeNode;
import org.opensearch.search.internal.SearchContext;
Expand All @@ -22,7 +21,6 @@
* Performs range match based on the params of @{@link org.opensearch.index.query.RangeQueryBuilder}
* Also, contains logic to skip performing range search if it's sure that it won't be found in Star Tree.
*/
@ExperimentalApi
public class RangeMatchDimFilter implements DimensionFilter {

private final String dimensionName;
Expand Down
4 changes: 2 additions & 2 deletions server/src/main/java/org/opensearch/search/startree/filter/StarTreeFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

package org.opensearch.search.startree.filter;

import org.opensearch.common.annotation.ExperimentalApi;
import org.opensearch.common.annotation.PublicApi;

import java.util.HashSet;
import java.util.List;
Expand All @@ -18,7 +18,7 @@
/**
* Container for intermediate/consolidated dimension filters that will be applied for a query in star tree traversal.
*/
@ExperimentalApi
@PublicApi(since = "2.18.0")
public class StarTreeFilter {

private final Map<String, List<DimensionFilter>> dimensionFilterMap;
Expand Down
4 changes: 2 additions & 2 deletions ...r/src/main/java/org/opensearch/search/startree/filter/provider/DimensionFilterMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import org.apache.lucene.util.BytesRef;
import org.apache.lucene.util.NumericUtils;
import org.opensearch.common.Numbers;
import org.opensearch.common.annotation.ExperimentalApi;
import org.opensearch.common.annotation.PublicApi;
import org.opensearch.common.lucene.BytesRefs;
import org.opensearch.common.lucene.Lucene;
import org.opensearch.index.compositeindex.datacube.DimensionDataType;
Expand Down Expand Up @@ -52,7 +52,7 @@
/**
* Generates the @{@link DimensionFilter} raw values and the @{@link MappedFieldType} of the dimension.
*/
@ExperimentalApi
@PublicApi(since = "2.18.0")
public interface DimensionFilterMapper {
/**
* Generates @{@link ExactMatchDimFilter} from Term/Terms query input.
Expand Down
2 changes: 0 additions & 2 deletions .../src/main/java/org/opensearch/search/startree/filter/provider/StarTreeFilterProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

package org.opensearch.search.startree.filter.provider;

import org.opensearch.common.annotation.ExperimentalApi;
import org.opensearch.index.compositeindex.datacube.Dimension;
import org.opensearch.index.mapper.CompositeDataCubeFieldType;
import org.opensearch.index.mapper.MappedFieldType;
Expand All @@ -32,7 +31,6 @@
* Converts a {@link QueryBuilder} into a {@link StarTreeFilter} by generating the appropriate @{@link DimensionFilter}
* for the fields provided in the user query.
*/
@ExperimentalApi
public interface StarTreeFilterProvider {

/**
Expand Down
Loading