Skip to content

Commit

Permalink
[FSTORE-1401] Feature view endpoint doesn't return unique feature vie…
Browse files Browse the repository at this point in the history
…ws when paginating (#1791)

* init

* improvements

* tests

* rename test

* fix test

* remove EXPECTATIONS

* fix setFilterQuery

* update fg ApiParam
  • Loading branch information
bubriks committed May 21, 2024
1 parent d34dfd7 commit e477ec6
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 94 deletions.
14 changes: 14 additions & 0 deletions hopsworks-IT/src/test/ruby/spec/featuregroup_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3235,6 +3235,20 @@
expect(json_body.all? { |fg| fg[:name] == "featuregroup1" }).to be true
end

it "should be able to list all featuregroups of the project's featurestore filtered by latest_version" do
project = get_project
featurestore_id = get_featurestore_id(project.id)

# get fgs
get "#{ENV['HOPSWORKS_API']}/project/" + project.id.to_s + "/featurestores/" + featurestore_id.to_s + "/featuregroups?filter_by=latest_version"
expect_status_details(200)
names = json_body.map { |o| "#{o[:name]}" }

expect(json_body.length).to eq(3)
expect(json_body.length).to eq(names.uniq.size)
expect(json_body.any? { |fg| fg[:name] == "featuregroup1" && fg[:version] == 3}).to be true
end

it "should be able to list all featuregroups of the project's featurestore filtered by name and version" do
project = get_project
featurestore_id = get_featurestore_id(project.id)
Expand Down
14 changes: 14 additions & 0 deletions hopsworks-IT/src/test/ruby/spec/featureview_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,20 @@
expect(json_body[:items].all? { |fv| fv[:name] == "featureview2" }).to be true
end

it "should be able to get a list of feature view filtered by latest_version" do
project = get_project
featurestore_id = get_featurestore_id(project.id)

# Get the list
get "#{ENV['HOPSWORKS_API']}/project/" + project.id.to_s + "/featurestores/" + featurestore_id.to_s + "/featureview?filter_by=latest_version"
expect_status_details(200)
names = json_body[:items].map { |o| "#{o[:name]}" }

expect(json_body[:items].length).to eq(2)
expect(json_body[:items].length).to eq(names.uniq.size)
expect(json_body[:items].any? { |fv| fv[:name] == "featureview2" && fv[:version] == 2}).to be true
end

it "should be able to get a list of feature view filtered by name and version" do
project = get_project
featurestore_id = get_featurestore_id(project.id)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,9 @@
public class FeatureGroupBeanParam {

@QueryParam("filter_by")
@ApiParam(value = "ex. filter_by=expectations:exp1&filter_by=expectations:exp2",
allowableValues =
"filter_by=expectations:exp1",
allowMultiple = true)
@ApiParam(value = "filter_by=latest_version",
allowableValues = "filter_by=latest_version,filter_by=name:value,filter_by=version:value",
allowMultiple = true)
private Set<FilterBy> filter;

@QueryParam("sort_by")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public FilterBy(String param) {
this.filter = FeaturegroupFacade.Filters.valueOf(param.substring(0, param.indexOf(':')).toUpperCase());
this.param = param.substring(param.indexOf(':') + 1);
} else {
this.filter = FeaturegroupFacade.Filters.valueOf(param);
this.filter = FeaturegroupFacade.Filters.valueOf(param.toUpperCase());
this.param = this.filter.getDefaultParam();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,6 @@ private void setFilter(Set<? extends AbstractFacade.FilterBy> filters, Query q)
private void setFilterQuery(FilterBy filter, Query q) {
switch (Filters.valueOf(filter.getValue())) {
case NAME:
case EXPECTATIONS:
q.setParameter(filter.getField(), filter.getParam());
break;
case VERSION:
Expand All @@ -267,7 +266,11 @@ private void setFilterQuery(FilterBy filter, Query q) {
public enum Filters {
NAME("NAME", "fg.name = :name", "name", ""),
VERSION("VERSION", "fg.version = :version", "version", ""),
EXPECTATIONS("EXPECTATIONS", "", "expectations", "");
LATEST_VERSION("LATEST_VERSION", String.format("%1$s.version = ( " +
"SELECT MAX(%2$s.version) " +
"FROM Featuregroup %2$s " +
"WHERE %1$s.name = %2$s.name AND %1$s.featurestore = %2$s.featurestore " +
") ", "fg", "fg2"), null, null);

private final String value;
private final String sql;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import javax.persistence.PersistenceContext;
import javax.persistence.Query;
import javax.persistence.TypedQuery;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand All @@ -51,24 +50,14 @@ public List<FeatureView> findAll() {
}

public List<FeatureView> findByFeaturestore(Featurestore featurestore, QueryParam queryParam) {
Boolean latestVersion = false;
if (queryParam != null && queryParam.getFilters().removeIf(filter -> filter.toString().equals("LATEST_VERSION"))) {
latestVersion = true;
}

Map<String, Object> extraParam = new HashMap<>();
extraParam.put("featurestore", featurestore);
String queryStr = buildQuery("SELECT fv FROM FeatureView fv ",
queryParam != null ? queryParam.getFilters(): null,
queryParam != null ? queryParam.getSorts(): null,
"fv.featurestore = :featurestore ");
Query q = makeQuery(queryStr, queryParam, extraParam);
List<FeatureView> results = q.getResultList();

if (latestVersion) {
results = retainLatestVersion(results);
}
return results;
return q.getResultList();
}

public Long countByFeaturestore(Featurestore featurestore) {
Expand All @@ -77,19 +66,6 @@ public Long countByFeaturestore(Featurestore featurestore) {
.getSingleResult();
}

List<FeatureView> retainLatestVersion(List<FeatureView> featureViews) {
Map<String, FeatureView> latestVersion = new HashMap<>();
for (FeatureView featureView : featureViews) {
if (!latestVersion.containsKey(featureView.getName())) {
latestVersion.put(featureView.getName(), featureView);
} else if (latestVersion.containsKey(featureView.getName()) &&
featureView.getVersion() > latestVersion.get(featureView.getName()).getVersion()) {
latestVersion.put(featureView.getName(), featureView);
}
}
return new ArrayList<>(latestVersion.values());
}

public Optional<FeatureView> findByIdAndFeatureStore(Integer id, Featurestore featureStore) {
try {
return Optional.of(
Expand Down Expand Up @@ -191,7 +167,11 @@ protected EntityManager getEntityManager() {
public enum Filters {
NAME("NAME", String.format("%s.name = :%%s ", FeatureView.TABLE_NAME_ALIAS), "name", ""),
VERSION("VERSION", String.format("%s.version = :%%s ", FeatureView.TABLE_NAME_ALIAS), "version", ""),
LATEST_VERSION("LATEST_VERSION", null, null, null);
LATEST_VERSION("LATEST_VERSION", String.format("%1$s.version = ( " +
"SELECT MAX(%2$s.version) " +
"FROM FeatureView %2$s " +
"WHERE %1$s.name = %2$s.name AND %1$s.featurestore = %2$s.featurestore " +
") ", FeatureView.TABLE_NAME_ALIAS, "fv2"), null, null);

private final String name;
private final String sql;
Expand Down

This file was deleted.

0 comments on commit e477ec6

Please sign in to comment.