Skip to content

Adding support for IN and NOT IN for non-string array values #207

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

Merged
varsha-abhinandan merged 5 commits into from
Nov 8, 2023
Merged

Adding support for IN and NOT IN for non-string array values #207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
30df059
ENG-36266 Adding support for IN and NOT IN for non-string array values
varsha-abhinandan Oct 16, 2023
5dcbf47
Merge branch 'main' into ENG-36266
varsha-abhinandan Oct 30, 2023
a97a194
Review comments
varsha-abhinandan Nov 7, 2023
00ad9e4
Merge remote-tracking branch 'origin/ENG-36266' into ENG-36266
varsha-abhinandan Nov 7, 2023
25ec7c6
Vulnerability update
varsha-abhinandan Nov 8, 2023
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 owasp-suppressions.xml
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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:
Copy link
Contributor

@kotharironak kotharironak Oct 17, 2023

Choose a reason for hiding this comment

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

@satish-mittal FYI: we might have to add the support for trino too. But, we also have to check if those queries works fine first.

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
Open in desktop
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
Open in desktop
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