Skip to content
This repository has been archived by the owner on Jun 26, 2024. It is now read-only.

Commit

Permalink
fixing the expression
Browse files Browse the repository at this point in the history
  • Loading branch information
aman-bansal committed Mar 11, 2024
1 parent 26d6d67 commit 7b5630d
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 87 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import com.google.common.collect.Streams;
import com.google.protobuf.InvalidProtocolBufferException;
import com.google.protobuf.util.JsonFormat;
import io.grpc.Status;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
Expand Down Expand Up @@ -34,6 +35,7 @@
import org.hypertrace.gateway.service.entity.EntitiesRequestContext;
import org.hypertrace.gateway.service.entity.config.EntityIdColumnsConfig;
import org.hypertrace.gateway.service.explore.entity.EntityServiceEntityFetcher;
import org.hypertrace.gateway.service.v1.common.AttributeExpression;
import org.hypertrace.gateway.service.v1.common.Expression;
import org.hypertrace.gateway.service.v1.common.FunctionExpression;
import org.hypertrace.gateway.service.v1.common.LiteralConstant;
Expand Down Expand Up @@ -65,6 +67,17 @@
*/
public class RequestHandler implements RequestHandlerWithSorting {
private static final Logger LOG = LoggerFactory.getLogger(RequestHandler.class);
private static final Expression NULL_VALUE_EXPRESSION =
Expression.newBuilder()
.setLiteral(
LiteralConstant.newBuilder()
.setValue(
org.hypertrace.gateway.service.v1.common.Value.newBuilder()
.setValueType(ValueType.STRING)
.setString("null")
.build())
.build())
.build();

private final QueryServiceClient queryServiceClient;
private final AttributeMetadataProvider attributeMetadataProvider;
Expand Down Expand Up @@ -543,10 +556,15 @@ private org.hypertrace.gateway.service.v1.common.Filter createEntityIdAttributeF
org.hypertrace.gateway.service.v1.common.Operator operator,
List<String> entityIds) {
if (entityIdAttributes.size() != 1) {
throw new RuntimeException("Cannot have more than one id attribute for an entity");
throw Status.FAILED_PRECONDITION
.withDescription("entity must have one id attribute.")
.asRuntimeException();

Check warning on line 561 in gateway-service-impl/src/main/java/org/hypertrace/gateway/service/explore/RequestHandler.java

View check run for this annotation

Codecov / codecov/patch

gateway-service-impl/src/main/java/org/hypertrace/gateway/service/explore/RequestHandler.java#L559-L561

Added lines #L559 - L561 were not covered by tests
}

if (entityIds.isEmpty()) {
// TODO: have a better approach here. One possible solution could be to
// form a dummy response based on the selections provided

// Having empty entity ids is valid filter because this means that
// EDS source has filtered out all the entities and the result should
// be empty. But QS doesn't recognize empty IN filter as valid filter
Expand All @@ -560,20 +578,21 @@ private org.hypertrace.gateway.service.v1.common.Filter createEntityIdAttributeF
.setOperator(Operator.AND)
.addChildFilter(
org.hypertrace.gateway.service.v1.common.Filter.newBuilder()
.setLhs(buildStringExpression(entityIdAttributes.get(0)))
.setOperator(Operator.NEQ)
.setRhs(buildStringExpression(null))
.setLhs(buildAttributeExpression(entityIdAttributes.get(0)))
.setOperator(Operator.EQ)
.setRhs(NULL_VALUE_EXPRESSION)
.build())
.addChildFilter(
org.hypertrace.gateway.service.v1.common.Filter.newBuilder()
.setLhs(buildStringExpression(entityIdAttributes.get(0)))
.setOperator(Operator.EQ)
.setRhs(buildStringExpression(null))
.setLhs(buildAttributeExpression(entityIdAttributes.get(0)))
.setOperator(Operator.NEQ)
.setRhs(NULL_VALUE_EXPRESSION)
.build())
.build();

Check warning on line 591 in gateway-service-impl/src/main/java/org/hypertrace/gateway/service/explore/RequestHandler.java

View check run for this annotation

Codecov / codecov/patch

gateway-service-impl/src/main/java/org/hypertrace/gateway/service/explore/RequestHandler.java#L577-L591

Added lines #L577 - L591 were not covered by tests
}

return org.hypertrace.gateway.service.v1.common.Filter.newBuilder()
.setLhs(buildStringExpression(entityIdAttributes.get(0)))
.setLhs(buildAttributeExpression(entityIdAttributes.get(0)))
.setOperator(operator)
.setRhs(buildStringArrayExpression(entityIds))
.build();
Expand All @@ -592,16 +611,9 @@ private Expression buildStringArrayExpression(List<String> values) {
.build();
}

private Expression buildStringExpression(String value) {
private Expression buildAttributeExpression(String value) {
return Expression.newBuilder()
.setLiteral(
LiteralConstant.newBuilder()
.setValue(
org.hypertrace.gateway.service.v1.common.Value.newBuilder()
.setValueType(ValueType.STRING)
.setString(value)
.build())
.build())
.setAttributeExpression(AttributeExpression.newBuilder().setAttributeId(value).build())
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -639,101 +639,67 @@ private QueryRequest getQueryRequest() throws InvalidProtocolBufferException {
+ " \"filter\": {\n"
+ " \"childFilter\": [{\n"
+ " \"childFilter\": [{\n"
+ " \"childFilter\": [{\n"
+ " \"childFilter\": [{\n"
+ " \"lhs\": {\n"
+ " \"attributeExpression\": {\n"
+ " \"attributeId\": \"API.attributeId3\"\n"
+ " }\n"
+ " },\n"
+ " \"operator\": \"EQ\",\n"
+ " \"rhs\": {\n"
+ " \"literal\": {\n"
+ " \"value\": {\n"
+ " \"string\": \"value\"\n"
+ " }\n"
+ " }\n"
+ " }\n"
+ " }]\n"
+ " }]\n"
+ " }, {\n"
+ " \"lhs\": {\n"
+ " \"attributeExpression\": {\n"
+ " \"attributeId\": \"API.timestampId\"\n"
+ " \"attributeId\": \"API.attributeId1\"\n"
+ " }\n"
+ " },\n"
+ " \"operator\": \"GE\",\n"
+ " \"operator\": \"EQ\",\n"
+ " \"rhs\": {\n"
+ " \"literal\": {\n"
+ " \"value\": {\n"
+ " \"valueType\": \"LONG\"\n"
+ " \"string\": \"value\"\n"
+ " }\n"
+ " }\n"
+ " }\n"
+ " }, {\n"
+ " \"lhs\": {\n"
+ " \"attributeExpression\": {\n"
+ " \"attributeId\": \"API.timestampId\"\n"
+ " \"attributeId\": \"API.attributeId2\"\n"
+ " }\n"
+ " },\n"
+ " \"operator\": \"LT\",\n"
+ " \"operator\": \"EQ\",\n"
+ " \"rhs\": {\n"
+ " \"literal\": {\n"
+ " \"value\": {\n"
+ " \"valueType\": \"LONG\"\n"
+ " \"string\": \"value\"\n"
+ " }\n"
+ " }\n"
+ " }\n"
+ " }]\n"
+ " }, {\n"
+ " \"childFilter\": [{\n"
+ " \"childFilter\": [{\n"
+ " \"childFilter\": [{\n"
+ " \"childFilter\": [{\n"
+ " \"lhs\": {\n"
+ " \"attributeExpression\": {\n"
+ " \"attributeId\": \"API.attributeId3\"\n"
+ " }\n"
+ " },\n"
+ " \"operator\": \"EQ\",\n"
+ " \"rhs\": {\n"
+ " \"literal\": {\n"
+ " \"value\": {\n"
+ " \"string\": \"value\"\n"
+ " }\n"
+ " }\n"
+ " }\n"
+ " }]\n"
+ " }]\n"
+ " }, {\n"
+ " \"lhs\": {\n"
+ " \"attributeExpression\": {\n"
+ " \"attributeId\": \"API.attributeId1\"\n"
+ " }\n"
+ " },\n"
+ " \"operator\": \"EQ\",\n"
+ " \"rhs\": {\n"
+ " \"literal\": {\n"
+ " \"value\": {\n"
+ " \"string\": \"value\"\n"
+ " }\n"
+ " }\n"
+ " }\n"
+ " }, {\n"
+ " \"lhs\": {\n"
+ " \"attributeExpression\": {\n"
+ " \"attributeId\": \"API.attributeId2\"\n"
+ " }\n"
+ " },\n"
+ " \"operator\": \"EQ\",\n"
+ " \"rhs\": {\n"
+ " \"literal\": {\n"
+ " \"value\": {\n"
+ " \"string\": \"value\"\n"
+ " }\n"
+ " }\n"
+ " }\n"
+ " }]\n"
+ " }, {\n"
+ " \"lhs\": {\n"
+ " \"literal\": {\n"
+ " \"value\": {\n"
+ " \"string\": \"API.timestampId\"\n"
+ " }\n"
+ " }\n"
+ " },\n"
+ " \"operator\": \"IN\",\n"
+ " \"rhs\": {\n"
+ " \"literal\": {\n"
+ " \"value\": {\n"
+ " \"valueType\": \"STRING_ARRAY\",\n"
+ " \"stringArray\": [\"entityId1\", \"entityId2\"]\n"
+ " }\n"
+ " \"lhs\": {\n"
+ " \"attributeExpression\": {\n"
+ " \"attributeId\": \"API.timestampId\"\n"
+ " }\n"
+ " },\n"
+ " \"operator\": \"IN\",\n"
+ " \"rhs\": {\n"
+ " \"literal\": {\n"
+ " \"value\": {\n"
+ " \"valueType\": \"STRING_ARRAY\",\n"
+ " \"stringArray\": [\"entityId1\", \"entityId2\"]\n"
+ " }\n"
+ " }\n"
+ " }]\n"
+ " }\n"
+ " }]\n"
+ " },\n"
+ " \"selection\": [{\n"
Expand Down

0 comments on commit 7b5630d

Please sign in to comment.