Skip to content

Commit d720c82

Browse files
kotharironakkotharironak
committed
refactor: clean up and removed duplicate logic
1 parent e4f987a commit d720c82

File tree

4 files changed

+103
-216
lines changed

4 files changed

+103
-216
lines changed

‎document-store/src/main/java/org/hypertrace/core/documentstore/postgres/PostgresCollection.java‎

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -324,19 +324,18 @@ public BulkUpdateResult bulkOperationOnArrayValue(BulkArrayValueUpdateRequest re
324324

325325
@Override
326326
public CloseableIterator<Document> search(Query query) {
327-
String filters = null;
328-
String selection = prepareSelections(query);
327+
String selection = PostgresQueryParser.parseSelections(query.getSelections());
329328
StringBuilder sqlBuilder =
330329
new StringBuilder(String.format("SELECT %s FROM ", selection)).append(collectionName);
330+
331+
String filters = null;
331332
Params.Builder paramsBuilder = Params.newBuilder();
332333

333334
// If there is a filter in the query, parse it fully.
334335
if (query.getFilter() != null) {
335336
filters = PostgresQueryParser.parseFilter(query.getFilter(), paramsBuilder);
336337
}
337338

338-
LOGGER.debug("Sending query to PostgresSQL: {} : {}", collectionName, filters);
339-
340339
if (filters != null) {
341340
sqlBuilder.append(" WHERE ").append(filters);
342341
}
@@ -356,35 +355,25 @@ public CloseableIterator<Document> search(Query query) {
356355
sqlBuilder.append(" OFFSET ").append(offset);
357356
}
358357

358+
String pgSqlQuery = sqlBuilder.toString();
359359
try {
360360
PreparedStatement preparedStatement =
361-
buildPreparedStatement(sqlBuilder.toString(), paramsBuilder.build());
362-
LOGGER.warn("Executing search query:{}", preparedStatement.toString());
361+
buildPreparedStatement(pgSqlQuery, paramsBuilder.build());
362+
LOGGER.warn("Executing search query to PostgresSQL:{}", preparedStatement.toString());
363363
ResultSet resultSet = preparedStatement.executeQuery();
364364
CloseableIterator closeableIterator =
365365
query.getSelections().size() > 0
366366
? new PostgresResultIteratorWithMetaData(resultSet)
367367
: new PostgresResultIterator(resultSet);
368368
return closeableIterator;
369369
} catch (SQLException e) {
370-
LOGGER.error("SQLException querying documents. query: {}", query, e);
370+
LOGGER.error(
371+
"SQLException in querying documents - query: {}, sqlQuery:{}", query, pgSqlQuery, e);
371372
}
372373

373374
return EMPTY_ITERATOR;
374375
}
375376

376-
private String prepareSelections(Query query) {
377-
List<String> selections = query.getSelections();
378-
if (selections.isEmpty()) return "*";
379-
return selections.stream()
380-
.map(
381-
selection ->
382-
String.format(
383-
"%s AS \"%s\"",
384-
PostgresUtils.prepareFieldAccessorExpr(selection, "document"), selection))
385-
.collect(Collectors.joining(","));
386-
}
387-
388377
@Override
389378
public CloseableIterator<Document> find(
390379
final org.hypertrace.core.documentstore.query.Query query) {

‎document-store/src/main/java/org/hypertrace/core/documentstore/postgres/PostgresQueryParser.java‎

Lines changed: 21 additions & 169 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,8 @@
11
package org.hypertrace.core.documentstore.postgres;
22

3-
import static org.hypertrace.core.documentstore.Collection.UNSUPPORTED_QUERY_OPERATION;
4-
import static org.hypertrace.core.documentstore.postgres.PostgresCollection.CREATED_AT;
5-
import static org.hypertrace.core.documentstore.postgres.PostgresCollection.ID;
6-
import static org.hypertrace.core.documentstore.postgres.PostgresCollection.UPDATED_AT;
7-
83
import java.util.Arrays;
9-
import java.util.HashSet;
104
import java.util.List;
11-
import java.util.Set;
5+
import java.util.Optional;
126
import java.util.stream.Collectors;
137
import org.apache.commons.lang3.StringUtils;
148
import org.hypertrace.core.documentstore.Filter;
@@ -18,140 +12,34 @@
1812

1913
class PostgresQueryParser {
2014

21-
private static final String QUESTION_MARK = "?";
22-
// postgres jsonb uses `->` instead of `.` for json field access
23-
private static final String JSON_FIELD_ACCESSOR = "->";
24-
// postgres operator to fetch the value of json object as text.
25-
private static final String JSON_DATA_ACCESSOR = "->>";
26-
private static final Set<String> OUTER_COLUMNS =
27-
new HashSet<>() {
28-
{
29-
add(CREATED_AT);
30-
add(ID);
31-
add(UPDATED_AT);
32-
}
33-
};
15+
static String parseSelections(List<String> selections) {
16+
return Optional.of(
17+
selections.stream()
18+
.map(
19+
selection ->
20+
String.format(
21+
"%s AS \"%s\"",
22+
PostgresUtils.prepareFieldAccessorExpr(
23+
selection, PostgresUtils.DOCUMENT_COLUMN),
24+
selection))
25+
.collect(Collectors.joining(",")))
26+
.filter(str -> StringUtils.isNotBlank(str))
27+
.orElse("*");
28+
}
3429

3530
static String parseFilter(Filter filter, Builder paramsBuilder) {
3631
if (filter.isComposite()) {
3732
return parseCompositeFilter(filter, paramsBuilder);
3833
} else {
39-
return parseNonCompositeFilter(filter, paramsBuilder);
34+
return PostgresUtils.parseNonCompositeFilter(
35+
filter.getFieldName(),
36+
PostgresUtils.DOCUMENT_COLUMN,
37+
filter.getOp().toString(),
38+
filter.getValue(),
39+
paramsBuilder);
4040
}
4141
}
4242

43-
static String parseNonCompositeFilter(Filter filter, Builder paramsBuilder) {
44-
Filter.Op op = filter.getOp();
45-
Object value = filter.getValue();
46-
String fieldName = filter.getFieldName();
47-
String fullFieldName =
48-
prepareCast(
49-
PostgresUtils.prepareFieldDataAccessorExpr(fieldName, PostgresUtils.DOCUMENT_COLUMN),
50-
value);
51-
StringBuilder filterString = new StringBuilder(fullFieldName);
52-
String sqlOperator;
53-
Boolean isMultiValued = false;
54-
switch (op) {
55-
case EQ:
56-
sqlOperator = " = ";
57-
break;
58-
case GT:
59-
sqlOperator = " > ";
60-
break;
61-
case LT:
62-
sqlOperator = " < ";
63-
break;
64-
case GTE:
65-
sqlOperator = " >= ";
66-
break;
67-
case LTE:
68-
sqlOperator = " <= ";
69-
break;
70-
case LIKE:
71-
// Case insensitive regex search, Append % at beginning and end of value to do a regex
72-
// search
73-
sqlOperator = " ILIKE ";
74-
value = "%" + value + "%";
75-
break;
76-
case NOT_IN:
77-
// NOTE: Below two points
78-
// 1. both NOT_IN and IN filter currently limited to non-array field
79-
// - https://github.com/hypertrace/document-store/issues/32#issuecomment-781411676
80-
// 2. To make semantically opposite filter of IN, we need to check for if key is not present
81-
// Ref in context of NEQ -
82-
// https://github.com/hypertrace/document-store/pull/20#discussion_r547101520Other
83-
// so, we need - "document->key IS NULL OR document->key->> NOT IN (v1, v2)"
84-
StringBuilder notInFilterString =
85-
PostgresUtils.prepareFieldAccessorExpr(fieldName, PostgresUtils.DOCUMENT_COLUMN);
86-
if (notInFilterString != null && !OUTER_COLUMNS.contains(fieldName)) {
87-
filterString = notInFilterString.append(" IS NULL OR ").append(fullFieldName);
88-
}
89-
sqlOperator = " NOT IN ";
90-
isMultiValued = true;
91-
value = prepareParameterizedStringForList((List<Object>) value, paramsBuilder);
92-
break;
93-
case IN:
94-
// NOTE: both NOT_IN and IN filter currently limited to non-array field
95-
// - https://github.com/hypertrace/document-store/issues/32#issuecomment-781411676
96-
sqlOperator = " IN ";
97-
isMultiValued = true;
98-
value = prepareParameterizedStringForList((List<Object>) value, paramsBuilder);
99-
break;
100-
case NOT_EXISTS:
101-
sqlOperator = " IS NULL ";
102-
value = null;
103-
// For fields inside jsonb
104-
StringBuilder notExists =
105-
PostgresUtils.prepareFieldAccessorExpr(fieldName, PostgresUtils.DOCUMENT_COLUMN);
106-
if (notExists != null && !OUTER_COLUMNS.contains(fieldName)) {
107-
filterString = notExists;
108-
}
109-
break;
110-
case EXISTS:
111-
sqlOperator = " IS NOT NULL ";
112-
value = null;
113-
// For fields inside jsonb
114-
StringBuilder exists =
115-
PostgresUtils.prepareFieldAccessorExpr(fieldName, PostgresUtils.DOCUMENT_COLUMN);
116-
if (exists != null && !OUTER_COLUMNS.contains(fieldName)) {
117-
filterString = exists;
118-
}
119-
break;
120-
case NEQ:
121-
sqlOperator = " != ";
122-
// https://github.com/hypertrace/document-store/pull/20#discussion_r547101520
123-
// The expected behaviour is to get all documents which either satisfy non equality
124-
// condition
125-
// or the key doesn't exist in them
126-
// Semantics for handling if key not exists and if it exists, its value
127-
// doesn't equal to the filter for Jsonb document will be done as:
128-
// "document->key IS NULL OR document->key->> != value"
129-
StringBuilder notEquals =
130-
PostgresUtils.prepareFieldAccessorExpr(fieldName, PostgresUtils.DOCUMENT_COLUMN);
131-
// For fields inside jsonb
132-
if (notEquals != null && !OUTER_COLUMNS.contains(fieldName)) {
133-
filterString = notEquals.append(" IS NULL OR ").append(fullFieldName);
134-
}
135-
break;
136-
case CONTAINS:
137-
// TODO: Matches condition inside an array of documents
138-
default:
139-
throw new UnsupportedOperationException(UNSUPPORTED_QUERY_OPERATION);
140-
}
141-
142-
filterString.append(sqlOperator);
143-
if (value != null) {
144-
if (isMultiValued) {
145-
filterString.append(value);
146-
} else {
147-
filterString.append(QUESTION_MARK);
148-
paramsBuilder.addObjectParam(value);
149-
}
150-
}
151-
String filters = filterString.toString();
152-
return filters;
153-
}
154-
15543
static String parseCompositeFilter(Filter filter, Builder paramsBuilder) {
15644
Filter.Op op = filter.getOp();
15745
switch (op) {
@@ -192,40 +80,4 @@ static String parseOrderBys(List<OrderBy> orderBys) {
19280
.filter(str -> !StringUtils.isEmpty(str))
19381
.collect(Collectors.joining(" , "));
19482
}
195-
196-
private static String prepareParameterizedStringForList(
197-
List<Object> values, Params.Builder paramsBuilder) {
198-
String collect =
199-
values.stream()
200-
.map(
201-
val -> {
202-
paramsBuilder.addObjectParam(val);
203-
return QUESTION_MARK;
204-
})
205-
.collect(Collectors.joining(", "));
206-
return "(" + collect + ")";
207-
}
208-
209-
private static String prepareCast(String field, Object value) {
210-
String fmt = "CAST (%s AS %s)";
211-
212-
// handle the case if the value type is collection for filter operator - `IN`
213-
// Currently, for `IN` operator, we are considering List collection, and it is fair
214-
// assumption that all its value of the same types. Based on that and for consistency
215-
// we will use CAST ( <field name> as <type> ) for all non string operator.
216-
// Ref : https://github.com/hypertrace/document-store/pull/30#discussion_r571782575
217-
218-
if (value instanceof List<?> && ((List<Object>) value).size() > 0) {
219-
List<Object> listValue = (List<Object>) value;
220-
value = listValue.get(0);
221-
}
222-
223-
if (value instanceof Number) {
224-
return String.format(fmt, field, "NUMERIC");
225-
} else if (value instanceof Boolean) {
226-
return String.format(fmt, field, "BOOLEAN");
227-
} else /* default is string */ {
228-
return field;
229-
}
230-
}
23183
}

‎document-store/src/main/java/org/hypertrace/core/documentstore/postgres/utils/PostgresUtils.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,8 @@
1515
import org.apache.commons.lang3.StringUtils;
1616
import org.hypertrace.core.documentstore.postgres.Params;
1717
import org.hypertrace.core.documentstore.postgres.Params.Builder;
18-
import org.slf4j.Logger;
19-
import org.slf4j.LoggerFactory;
2018

2119
public class PostgresUtils {
22-
private static final Logger LOGGER = LoggerFactory.getLogger(PostgresUtils.class);
2320
private static final String QUESTION_MARK = "?";
2421
private static final String JSON_FIELD_ACCESSOR = "->";
2522
private static final String JSON_DATA_ACCESSOR = "->>";

0 commit comments

Comments
 (0)