Skip to content

Commit

Permalink
Merge pull request geoserver#462 from groldan/feature/pgconfig_multiv…
Browse files Browse the repository at this point in the history
…alued_id_query_properties

pgconfig: query optimizations
  • Loading branch information
groldan authored May 3, 2024
2 parents addd2e9 + bfd4ca8 commit 9f1cdad
Show file tree
Hide file tree
Showing 14 changed files with 512 additions and 83 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
/*
* (c) 2024 Open Source Geospatial Foundation - all rights reserved This code is licensed under the
* GPL 2.0 license, available at the root application directory.
*/
package org.geoserver.cloud.backend.pgconfig.catalog.filter;

import lombok.NonNull;
import lombok.extern.slf4j.Slf4j;

import org.geoserver.catalog.CatalogInfo;
import org.geoserver.catalog.WorkspaceInfo;
import org.geotools.api.filter.BinaryComparisonOperator;
import org.geotools.api.filter.Filter;
import org.geotools.api.filter.FilterFactory;
import org.geotools.api.filter.MultiValuedFilter.MatchAction;
import org.geotools.api.filter.PropertyIsEqualTo;
import org.geotools.api.filter.PropertyIsGreaterThan;
import org.geotools.api.filter.PropertyIsGreaterThanOrEqualTo;
import org.geotools.api.filter.PropertyIsLessThan;
import org.geotools.api.filter.PropertyIsLessThanOrEqualTo;
import org.geotools.api.filter.PropertyIsNotEqualTo;
import org.geotools.api.filter.expression.Expression;
import org.geotools.api.filter.expression.Literal;
import org.geotools.api.filter.expression.PropertyName;
import org.geotools.factory.CommonFactoryFinder;
import org.geotools.filter.visitor.DuplicatingFilterVisitor;

import java.util.Optional;
import java.util.Set;

/**
* Converts binary comparison operators using a property name and a {@link CatalogInfo} instance
* literal as a comparison by id.
*
* <p>For example, a filter like {@code workspace = WorkspaceInfo[test]} is translated to {@code
* workspace.id = WorkspaceInfo[test].id}, where {@code WorkspaceInfo[test]} is an instance of a
* {@link WorkspaceInfo}.
*
* <p>
*
* @since 1.8.1
*/
@Slf4j(topic = "org.geoserver.cloud.backend.pgconfig.catalog.filter")
class CatalogInfoLiteralAdaptor extends DuplicatingFilterVisitor {

private static final FilterFactory FACTORY = CommonFactoryFinder.getFilterFactory(null);

private Set<String> supportedPropertyNames;

public CatalogInfoLiteralAdaptor(@NonNull Set<String> supportedPropertyNames) {
super(FACTORY);
this.supportedPropertyNames = supportedPropertyNames;
}

public static Filter replaceCatalogInfoLiterals(
Filter filter, Set<String> supportedPropertyNames) {
var adaptor = new CatalogInfoLiteralAdaptor(supportedPropertyNames);
return (Filter) filter.accept(adaptor, null);
}

@Override
public Object visit(PropertyIsEqualTo filter, Object notUsed) {
return super.visit(adapt(filter, ff::equal), notUsed);
}

@Override
public Object visit(PropertyIsNotEqualTo filter, Object notUsed) {
return super.visit(adapt(filter, ff::notEqual), notUsed);
}

@Override
public Object visit(PropertyIsGreaterThan filter, Object notUsed) {
return super.visit(adapt(filter, ff::greater), notUsed);
}

@Override
public Object visit(PropertyIsGreaterThanOrEqualTo filter, Object notUsed) {
return super.visit(adapt(filter, ff::greaterOrEqual), notUsed);
}

@Override
public Object visit(PropertyIsLessThan filter, Object notUsed) {
return super.visit(adapt(filter, ff::less), notUsed);
}

@Override
public Object visit(PropertyIsLessThanOrEqualTo filter, Object notUsed) {
return super.visit(adapt(filter, ff::lessOrEqual), notUsed);
}

@FunctionalInterface
private static interface BinaryComparisonBuilder<F extends BinaryComparisonOperator> {
F build(Expression left, Expression right, boolean matchCase, MatchAction matchAction);
}

/**
* Converts a binary comparision operator using a property name and a {@link CatalogInfo}
* instance literal as a comparison by id.
*
* <p>For example, a filter like {@code workspace = WorkspaceInfo} is translated to {@code
* workspace.id = WorkspaceInfo.id}, where {@code WorkspaceInfo} is an instanceo of a {@link
* WorkspaceInfo}.
*/
private <F extends BinaryComparisonOperator> F adapt(
F filter, BinaryComparisonBuilder<F> builder) {

PropertyName prop = propertyName(filter);
Literal literal = literal(filter);
if (prop != null && literal != null) {
final Filter orig = filter;
String propertyName = prop.getPropertyName();
Object value = literal.getValue();
if (value instanceof CatalogInfo info) {
String idProp = "%s.id".formatted(propertyName);
if (supportedPropertyNames.contains(idProp)) {
prop = ff.property(idProp);
literal = ff.literal(info.getId());
boolean matchingCase = filter.isMatchingCase();
MatchAction matchAction = filter.getMatchAction();
filter = builder.build(prop, literal, matchingCase, matchAction);
log.debug(
"Fitler with CatalogInfo literal '{}' translated to '{}'",
orig,
filter);
}
}
}

return filter;
}

private PropertyName propertyName(BinaryComparisonOperator filter) {
return propertyName(filter.getExpression1())
.or(() -> propertyName(filter.getExpression2()))
.orElse(null);
}

private Literal literal(BinaryComparisonOperator filter) {
return literal(filter.getExpression1())
.or(() -> literal(filter.getExpression2()))
.orElse(null);
}

private Optional<PropertyName> propertyName(Expression e) {
return Optional.of(e).filter(PropertyName.class::isInstance).map(PropertyName.class::cast);
}

private Optional<Literal> literal(Expression e) {
return Optional.of(e).filter(Literal.class::isInstance).map(Literal.class::cast);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
*/
package org.geoserver.cloud.backend.pgconfig.catalog.filter;

import lombok.NonNull;

import org.geotools.api.filter.Filter;
import org.geotools.api.filter.expression.PropertyName;
import org.geotools.filter.visitor.PostPreProcessFilterSplittingVisitor;
Expand All @@ -20,15 +22,14 @@ class PgsqlCatalogFilterSplitter extends PostPreProcessFilterSplittingVisitor {

private Set<String> supportedPropertyNames;

public PgsqlCatalogFilterSplitter(Set<String> supportedPropertyNames) {
public PgsqlCatalogFilterSplitter(@NonNull Set<String> supportedPropertyNames) {
super(PgsqlFilterCapabilities.capabilities(), null, null);
this.supportedPropertyNames = supportedPropertyNames;
}

public static PgsqlCatalogFilterSplitter split(
Filter filter, Set<String> supportedPropertyNames) {
PgsqlCatalogFilterSplitter splitter =
new PgsqlCatalogFilterSplitter(supportedPropertyNames);
var splitter = new PgsqlCatalogFilterSplitter(supportedPropertyNames);
filter.accept(splitter, null);
return splitter;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import lombok.experimental.UtilityClass;

import org.geoserver.function.IsInstanceOf;
import org.geotools.api.filter.ExcludeFilter;
import org.geotools.api.filter.Id;
import org.geotools.api.filter.IncludeFilter;
Expand Down Expand Up @@ -113,6 +114,9 @@ static FilterCapabilities createFilterCapabilities() {
// compare functions
caps.addType(FilterFunction_equalTo.class);

// well-known VolatileFunction implementations
caps.addType(IsInstanceOf.class);

return caps;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,22 @@
*/
package org.geoserver.cloud.backend.pgconfig.catalog.filter;

import lombok.NonNull;
import lombok.Value;

import org.geoserver.catalog.CatalogInfo;
import org.geoserver.catalog.impl.ClassMappings;
import org.geoserver.function.IsInstanceOf;
import org.geotools.api.filter.Filter;
import org.geotools.api.filter.PropertyIsEqualTo;
import org.geotools.api.filter.PropertyIsLike;
import org.geotools.api.filter.expression.Expression;
import org.geotools.api.filter.expression.Function;
import org.geotools.api.filter.expression.Literal;
import org.geotools.api.filter.expression.PropertyName;
import org.geotools.filter.LengthFunction;
import org.geotools.filter.LikeFilterImpl;
import org.geotools.filter.LiteralExpressionImpl;
import org.geotools.filter.function.FilterFunction_strConcat;
import org.geotools.filter.function.FilterFunction_strEndsWith;
import org.geotools.filter.function.FilterFunction_strEqualsIgnoreCase;
Expand All @@ -28,13 +36,18 @@
import org.geotools.filter.function.math.FilterFunction_abs_3;
import org.geotools.filter.function.math.FilterFunction_abs_4;
import org.geotools.jdbc.PreparedFilterToSQL;
import org.geotools.util.Converters;

import java.io.IOException;
import java.io.StringWriter;
import java.io.UncheckedIOException;
import java.io.Writer;
import java.util.Collection;
import java.util.Date;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;
import java.util.stream.Stream;

/**
* @since 1.4
Expand All @@ -60,7 +73,7 @@ public PgsqlFilterToSQL(Writer out) {

public static Result evaluate(Filter filter) {
StringWriter out = new StringWriter();
PgsqlFilterToSQL filterToPreparedStatement = new PgsqlFilterToSQL(out);
var filterToPreparedStatement = new PgsqlFilterToSQL(out);
filterToPreparedStatement.setSqlNameEscape("\"");
filter.accept(filterToPreparedStatement, null);
out.flush();
Expand All @@ -72,6 +85,56 @@ public static Result evaluate(Filter filter) {
return new Result(whereClause, literalValues, literalTypes);
}

@Override
public Object visit(PropertyIsEqualTo filter, Object extraData) {
Expression left = filter.getExpression1();
Expression right = filter.getExpression2();

PropertyName prop =
Optional.of(left)
.filter(PropertyName.class::isInstance)
.or(() -> Optional.of(right).filter(PropertyName.class::isInstance))
.map(PropertyName.class::cast)
.orElse(null);
if (isArray(prop)) {
Expression value = right;
if (right instanceof Literal literal) {
String values =
asList(literal.getValue()).stream()
.map(o -> Converters.convert(o, String.class))
.map("'%s'"::formatted)
.collect(Collectors.joining(","));
value = new LiteralExpressionImpl("ARRAY[%s]".formatted(values));
}
try {
prop.accept(this, extraData);
out.write(" && ");
out.write((String) ((Literal) value).getValue());
} catch (IOException e) {
throw new UncheckedIOException(e);
}

return extraData;
}

return super.visit(filter, extraData);
}

@SuppressWarnings({"unchecked", "java:S6204"})
private List<Object> asList(Object value) {
if (value instanceof Collection) {
// beware Stream.toList() does not support null values but Collectors.toList() does
return ((Collection<Object>) value).stream().collect(Collectors.toList());
}
return List.of(value);
}

private boolean isArray(PropertyName prop) {
if (null == prop) return false;
String propertyName = prop.getPropertyName();
return "styles.id".equals(propertyName) || "layers.id".equals(propertyName);
}

/**
* Writes the SQL for the Like Filter. Assumes the current java implemented wildcards for the
* Like Filter: . for multi and .? for single. And replaces them with the SQL % and _,
Expand Down Expand Up @@ -262,7 +325,35 @@ protected boolean visitFunction(Function function) throws IOException {
out.write(")");
return true;
}
if (function instanceof IsInstanceOf instanceOf) {
Expression typeExpr = getParameter(instanceOf, 0, true);
@SuppressWarnings("unchecked")
Class<? extends CatalogInfo> type = typeExpr.evaluate(null, Class.class);

String types = infoTypes(type);
String f =
"""
"@type" = ANY('{%s}')
"""
.formatted(types);
out.write(f);
return true;
}

// function not supported
return false;
}

protected @NonNull String infoTypes(Class<? extends CatalogInfo> clazz) {
ClassMappings cm;
if (clazz.isInterface()) cm = ClassMappings.fromInterface(clazz);
else cm = ClassMappings.fromImpl(clazz);
if (null == cm)
throw new IllegalArgumentException(
"Unknown type for IsInstanceOf: " + clazz.getCanonicalName());

return Stream.of(cm.concreteInterfaces())
.map(Class::getSimpleName)
.collect(Collectors.joining(","));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
*/
package org.geoserver.cloud.backend.pgconfig.catalog.filter;

import static org.geoserver.cloud.backend.pgconfig.catalog.filter.CatalogInfoLiteralAdaptor.replaceCatalogInfoLiterals;
import static org.geoserver.cloud.backend.pgconfig.catalog.filter.PgsqlCatalogFilterSplitter.split;

import lombok.Getter;
import lombok.RequiredArgsConstructor;

Expand All @@ -20,7 +23,7 @@
@RequiredArgsConstructor
public class PgsqlQueryBuilder {

private final Filter filter;
private final Filter origFilter;
private final Set<String> supportedPropertyNames;

private @Getter Filter supportedFilter = Filter.INCLUDE;
Expand All @@ -33,12 +36,17 @@ public class PgsqlQueryBuilder {
private @Getter List<Class> literalTypes;

public PgsqlQueryBuilder build() {
if (Filter.INCLUDE.equals(filter)) {
if (Filter.INCLUDE.equals(origFilter)) {
return this;
}
var splitter = PgsqlCatalogFilterSplitter.split(filter, supportedPropertyNames);

Filter filter = replaceCatalogInfoLiterals(origFilter, supportedPropertyNames);
filter = simplify(filter);

var splitter = split(filter, supportedPropertyNames);

supportedFilter = adaptToSql(splitter.getFilterPre());

unsupportedFilter = simplify(splitter.getFilterPost());

Result encodeResult = PgsqlFilterToSQL.evaluate(supportedFilter);
Expand All @@ -49,7 +57,8 @@ public PgsqlQueryBuilder build() {
}

private Filter adaptToSql(Filter filterPre) {
Filter supported = ToPgsqlCompatibleFilterDuplicator.adapt(filterPre);
Filter supported =
ToPgsqlCompatibleFilterDuplicator.adapt(filterPre, supportedPropertyNames);
return simplify(supported);
}

Expand Down
Loading

0 comments on commit 9f1cdad

Please sign in to comment.