Skip to content

Commit

Permalink
Adding support for IN and NOT IN for non-string array values (#207)
Browse files Browse the repository at this point in the history 
* ENG-36266 Adding support for IN and NOT IN for non-string array values
  • Loading branch information
@varsha-abhinandan
varsha-abhinandan authored Nov 8, 2023
1 parent 2c4635c commit 436adec
Show file tree
Hide file tree
Showing 8 changed files with 296 additions and 64 deletions.
4 changes: 2 additions & 2 deletions owasp-suppressions.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@
<cve>CVE-2019-0201</cve>
<cve>CVE-2023-44981</cve>
</suppress>
<suppress until="2023-10-31Z">
<suppress until="2023-11-30Z">
<notes><![CDATA[
file name: jackson-databind-2.14.2.jar
This is currently disputed.
]]></notes>
<packageUrl regex="true">^pkg:maven/com\.fasterxml\.jackson\.core/jackson\-databind@.*$</packageUrl>
<cve>CVE-2023-35116</cve>
</suppress>
<suppress until="2023-10-31Z">
<suppress until="2023-11-30Z">
<notes><![CDATA[
file name: netty-handler-4.1.94.Final.jar
]]></notes>
Expand Down
67 changes: 32 additions & 35 deletions ...rc/main/java/org/hypertrace/core/query/service/pinot/QueryRequestToPinotSQLConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@
import static org.hypertrace.core.query.service.api.Expression.ValueCase.LITERAL;

import com.google.common.base.Joiner;
import com.google.protobuf.ByteString;
import java.util.AbstractMap.SimpleEntry;
import java.util.ArrayList;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map.Entry;
import java.util.Optional;
import java.util.function.Consumer;
import org.hypertrace.core.query.service.ExecutionContext;
import org.hypertrace.core.query.service.api.Expression;
import org.hypertrace.core.query.service.api.Filter;
Expand Down Expand Up @@ -450,40 +450,28 @@ private String convertLiteralToString(LiteralConstant literal, Params.Builder pa
String ret = null;
switch (value.getValueType()) {
case STRING_ARRAY:
StringBuilder builder = new StringBuilder("(");
String delim = "";
for (String item : value.getStringArrayList()) {
builder.append(delim);
builder.append(QUESTION_MARK);
paramsBuilder.addStringParam(item);
delim = ", ";
}
builder.append(")");
ret = builder.toString();
ret = buildArrayValue(value.getStringArrayList(), paramsBuilder::addStringParam);
break;
case BYTES_ARRAY:
builder = new StringBuilder("(");
delim = "";
for (ByteString item : value.getBytesArrayList()) {
builder.append(delim);
builder.append(QUESTION_MARK);
paramsBuilder.addByteStringParam(item);
delim = ", ";
}
builder.append(")");
ret = builder.toString();
ret = buildArrayValue(value.getBytesArrayList(), paramsBuilder::addByteStringParam);
break;
case BOOLEAN_ARRAY:
builder = new StringBuilder("(");
delim = "";
for (Boolean item : value.getBooleanArrayList()) {
builder.append(delim);
builder.append(QUESTION_MARK);
paramsBuilder.addStringParam(String.valueOf(item));
delim = ", ";
}
builder.append(")");
ret = builder.toString();
ret =
buildArrayValue(
value.getBooleanArrayList(),
item -> paramsBuilder.addStringParam(String.valueOf(item)));
break;
case LONG_ARRAY:
ret = buildArrayValue(value.getLongArrayList(), paramsBuilder::addLongParam);
break;
case INT_ARRAY:
ret = buildArrayValue(value.getIntArrayList(), paramsBuilder::addIntegerParam);
break;
case FLOAT_ARRAY:
ret = buildArrayValue(value.getFloatArrayList(), paramsBuilder::addFloatParam);
break;
case DOUBLE_ARRAY:
ret = buildArrayValue(value.getDoubleArrayList(), paramsBuilder::addDoubleParam);
break;
case STRING:
ret = QUESTION_MARK;
Expand Down Expand Up @@ -525,13 +513,22 @@ private String convertLiteralToString(LiteralConstant literal, Params.Builder pa
ret = QUESTION_MARK;
paramsBuilder.addStringParam("null");
break;
case LONG_ARRAY:
case INT_ARRAY:
case FLOAT_ARRAY:
case DOUBLE_ARRAY:
case UNRECOGNIZED:
break;
}
return ret;
}

private <T> String buildArrayValue(List<T> values, Consumer<T> paramAdder) {
StringBuilder builder = new StringBuilder("(");
String delim = "";
for (T item : values) {
builder.append(delim);
builder.append(QUESTION_MARK);
paramAdder.accept(item);
delim = ", ";
}
builder.append(")");
return builder.toString();
}
}
60 changes: 34 additions & 26 deletions .../org/hypertrace/core/query/service/postgres/converters/DefaultColumnRequestConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@
import static org.hypertrace.core.query.service.postgres.converters.ColumnRequestContext.createColumnRequestContext;

import com.google.common.base.Strings;
import com.google.protobuf.ByteString;
import java.util.ArrayList;
import java.util.List;
import java.util.function.Consumer;
import java.util.stream.Collectors;
import org.apache.commons.codec.DecoderException;
import org.apache.commons.codec.binary.Hex;
Expand Down Expand Up @@ -453,28 +453,28 @@ private String convertLiteralToString(LiteralConstant literal, Builder paramsBui
String ret = null;
switch (value.getValueType()) {
case STRING_ARRAY:
StringBuilder builder = new StringBuilder("(");
String delim = "";
for (String item : value.getStringArrayList()) {
builder.append(delim);
builder.append(QUESTION_MARK);
paramsBuilder.addStringParam(item);
delim = ", ";
}
builder.append(")");
ret = builder.toString();
ret = buildArrayValue(value.getStringArrayList(), paramsBuilder::addStringParam);
break;
case BYTES_ARRAY:
builder = new StringBuilder("(");
delim = "";
for (ByteString item : value.getBytesArrayList()) {
builder.append(delim);
builder.append(QUESTION_MARK);
paramsBuilder.addByteStringParam(item);
delim = ", ";
}
builder.append(")");
ret = builder.toString();
ret = buildArrayValue(value.getBytesArrayList(), paramsBuilder::addByteStringParam);
break;
case LONG_ARRAY:
ret = buildArrayValue(value.getLongArrayList(), paramsBuilder::addLongParam);
break;
case INT_ARRAY:
ret = buildArrayValue(value.getIntArrayList(), paramsBuilder::addIntegerParam);
break;
case FLOAT_ARRAY:
ret = buildArrayValue(value.getFloatArrayList(), paramsBuilder::addFloatParam);
break;
case DOUBLE_ARRAY:
ret = buildArrayValue(value.getDoubleArrayList(), paramsBuilder::addDoubleParam);
break;
case BOOLEAN_ARRAY:
ret =
buildArrayValue(
value.getBooleanArrayList(),
item -> paramsBuilder.addStringParam(String.valueOf(item)));
break;
case STRING:
ret = QUESTION_MARK;
Expand Down Expand Up @@ -516,14 +516,22 @@ private String convertLiteralToString(LiteralConstant literal, Builder paramsBui
ret = QUESTION_MARK;
paramsBuilder.addStringParam("null");
break;
case LONG_ARRAY:
case INT_ARRAY:
case FLOAT_ARRAY:
case DOUBLE_ARRAY:
case BOOLEAN_ARRAY:
case UNRECOGNIZED:
break;
}
return ret;
}

private <T> String buildArrayValue(List<T> values, Consumer<T> paramAdder) {
StringBuilder builder = new StringBuilder("(");
String delim = "";
for (T item : values) {
builder.append(delim);
builder.append(QUESTION_MARK);
paramAdder.accept(item);
delim = ", ";
}
builder.append(")");
return builder.toString();
}
}
56 changes: 56 additions & 0 deletions ...ervice-impl/src/test/java/org/hypertrace/core/query/service/QueryRequestBuilderUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,22 @@ public static Filter createBooleanInFilter(String column, List<Boolean> values)
return createFilter(column, Operator.IN, createBooleanArrayLiteralValueExpression(values));
}

public static Filter createLongInFilter(String column, List<Long> values) {
return createFilter(column, Operator.IN, createLongArrayLiteralValueExpression(values));
}

public static Filter createIntNotInFilter(String column, List<Integer> values) {
return createFilter(column, Operator.NOT_IN, createIntArrayLiteralValueExpression(values));
}

public static Filter createFloatNotInFilter(String column, List<Float> values) {
return createFilter(column, Operator.NOT_IN, createFloatArrayLiteralValueExpression(values));
}

public static Filter createDoubleInFilter(String column, List<Double> values) {
return createFilter(column, Operator.IN, createDoubleArrayLiteralValueExpression(values));
}

public static Filter createNotInFilter(String column, List<String> values) {
return createFilter(column, Operator.NOT_IN, createStringArrayLiteralValueExpression(values));
}
Expand Down Expand Up @@ -228,6 +244,15 @@ public static Expression createLongLiteralValueExpression(long value) {
.build();
}

public static Expression createLongArrayLiteralValueExpression(List<Long> values) {
return Expression.newBuilder()
.setLiteral(
LiteralConstant.newBuilder()
.setValue(
Value.newBuilder().addAllLongArray(values).setValueType(ValueType.LONG_ARRAY)))
.build();
}

public static Expression createTimestampLiteralValueExpression(long value) {
return Expression.newBuilder()
.setLiteral(
Expand All @@ -244,6 +269,17 @@ public static Expression createDoubleLiteralValueExpression(double value) {
.build();
}

public static Expression createDoubleArrayLiteralValueExpression(List<Double> values) {
return Expression.newBuilder()
.setLiteral(
LiteralConstant.newBuilder()
.setValue(
Value.newBuilder()
.addAllDoubleArray(values)
.setValueType(ValueType.DOUBLE_ARRAY)))
.build();
}

public static Expression createFloatLiteralValueExpression(float value) {
return Expression.newBuilder()
.setLiteral(
Expand All @@ -252,6 +288,17 @@ public static Expression createFloatLiteralValueExpression(float value) {
.build();
}

public static Expression createFloatArrayLiteralValueExpression(List<Float> values) {
return Expression.newBuilder()
.setLiteral(
LiteralConstant.newBuilder()
.setValue(
Value.newBuilder()
.addAllFloatArray(values)
.setValueType(ValueType.FLOAT_ARRAY)))
.build();
}

public static Expression createBoolLiteralValueExpression(boolean value) {
return Expression.newBuilder()
.setLiteral(
Expand All @@ -268,6 +315,15 @@ public static Expression createIntLiteralValueExpression(int value) {
.build();
}

public static Expression createIntArrayLiteralValueExpression(List<Integer> values) {
return Expression.newBuilder()
.setLiteral(
LiteralConstant.newBuilder()
.setValue(
Value.newBuilder().addAllIntArray(values).setValueType(ValueType.INT_ARRAY)))
.build();
}

public static Expression createNullStringLiteralValueExpression() {
return Expression.newBuilder()
.setLiteral(
Expand Down
75 changes: 75 additions & 0 deletions ...est/java/org/hypertrace/core/query/service/pinot/QueryRequestToPinotSQLConverterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,13 @@
import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createColumnExpression;
import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createCompositeFilter;
import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createCountByColumnSelection;
import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createDoubleInFilter;
import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createEqualsFilter;
import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createFloatNotInFilter;
import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createFunctionExpression;
import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createInFilter;
import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createIntNotInFilter;
import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createLongInFilter;
import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createLongLiteralValueExpression;
import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createNotEqualsFilter;
import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createNullNumberLiteralValueExpression;
Expand Down Expand Up @@ -1060,6 +1064,77 @@ public void testQueryWithLikeOperatorForResponseBodyHavingTextIndex() {
executionContext);
}

@Test
public void testQueryWithIntArrayFilter() {
ViewDefinition viewDefinition = getDefaultViewDefinition();
defaultMockingForExecutionContext();

assertPQLQuery(
buildSimpleQueryWithFilter(
createIntNotInFilter("Span.metrics.duration_millis", List.of(123))),
"Select span_id FROM SpanEventView WHERE "
+ viewDefinition.getTenantIdColumn()
+ " = '"
+ TENANT_ID
+ "' "
+ "AND duration_millis NOT IN (123)",
viewDefinition,
executionContext);
}

@Test
public void testQueryWithLongArrayFilter() {
ViewDefinition viewDefinition = getDefaultViewDefinition();
defaultMockingForExecutionContext();

assertPQLQuery(
buildSimpleQueryWithFilter(
createLongInFilter("Span.metrics.duration_millis", List.of(10L, 20L, 30L))),
"SELECT span_id FROM SpanEventView WHERE "
+ viewDefinition.getTenantIdColumn()
+ " = '"
+ TENANT_ID
+ "' "
+ "AND duration_millis IN (10, 20, 30)",
viewDefinition,
executionContext);
}

@Test
public void testQueryWithFloatArrayFilter() {
ViewDefinition viewDefinition = getDefaultViewDefinition();
defaultMockingForExecutionContext();

assertPQLQuery(
buildSimpleQueryWithFilter(createFloatNotInFilter("Span.user_latitude", List.of(10.05f))),
"SELECT span_id FROM SpanEventView WHERE "
+ viewDefinition.getTenantIdColumn()
+ " = '"
+ TENANT_ID
+ "' "
+ "AND user_latitude NOT IN (10.05)",
viewDefinition,
executionContext);
}

@Test
public void testQueryWithDoubleArrayFilter() {

ViewDefinition viewDefinition = getDefaultViewDefinition();
defaultMockingForExecutionContext();

assertPQLQuery(
buildSimpleQueryWithFilter(createDoubleInFilter("Span.user_longitude", List.of(10.87654))),
"SELECT span_id FROM SpanEventView WHERE "
+ viewDefinition.getTenantIdColumn()
+ " = '"
+ TENANT_ID
+ "' "
+ "AND user_longitude IN (10.87654)",
viewDefinition,
executionContext);
}

private QueryRequest buildSimpleQueryWithFilter(Filter filter) {
Builder builder = QueryRequest.newBuilder();
builder.addSelection(createColumnExpression("Span.id").build());
Expand Down
Loading

0 comments on commit 436adec

Please sign in to comment.