Skip to content

Commit

Permalink
MID-7074: null value placeholder gone, conditional unique indexes in
Browse files Browse the repository at this point in the history
M_connector now uses two conditional unique indexes based on whether
connectorHostRefTargetOid is (not) null.
Many ref table indexes were added.
Unique constraints were rewritten to unique indexes - while not what
PG recommends, it's the same and I prefer single way of writing these.
We can't use WHERE with constraints syntax.
  • Loading branch information
virgo47 committed Oct 5, 2021
1 parent 43a173b commit 3f4ec78
Show file tree
Hide file tree
Showing 12 changed files with 97 additions and 149 deletions.
112 changes: 59 additions & 53 deletions config/sql/native-new/postgres-new.sql

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,13 @@ public class SqaleRepositoryConfiguration implements JdbcRepositoryConfiguration

private static final String DEFAULT_DRIVER = "org.postgresql.Driver";
private static final SupportedDatabase DEFAULT_DATABASE = SupportedDatabase.POSTGRESQL;
private static final String DEFAULT_JDBC_URL = "jdbc:postgresql://localhost:5432/midpoint";
private static final String DEFAULT_JDBC_USERNAME = "midpoint";
private static final String DEFAULT_JDBC_PASSWORD = "password";
private static final String DEFAULT_FULL_OBJECT_FORMAT = PrismContext.LANG_JSON;

private static final int DEFAULT_MIN_POOL_SIZE = 8;
private static final int DEFAULT_MAX_POOL_SIZE = 20;
private static final int DEFAULT_MAX_POOL_SIZE = 50;

private static final int DEFAULT_ITERATIVE_SEARCH_PAGE_SIZE = 100;

Expand All @@ -53,8 +57,6 @@ public class SqaleRepositoryConfiguration implements JdbcRepositoryConfiguration
private int minPoolSize;
private int maxPoolSize;

private boolean useZip;
private boolean useZipAudit;
private String fullObjectFormat;

private String performanceStatisticsFile;
Expand All @@ -72,8 +74,8 @@ public SqaleRepositoryConfiguration(@NotNull Configuration configuration) {
public void init() throws RepositoryServiceFactoryException {
dataSource = configuration.getString(PROPERTY_DATASOURCE);

jdbcUrl = configuration.getString(PROPERTY_JDBC_URL);
jdbcUsername = configuration.getString(PROPERTY_JDBC_USERNAME);
jdbcUrl = configuration.getString(PROPERTY_JDBC_URL, DEFAULT_JDBC_URL);
jdbcUsername = configuration.getString(PROPERTY_JDBC_USERNAME, DEFAULT_JDBC_USERNAME);

databaseType = DEFAULT_DATABASE;
driverClassName = DEFAULT_DRIVER;
Expand All @@ -87,15 +89,13 @@ public void init() throws RepositoryServiceFactoryException {
+ jdbcPasswordFile + "': " + e.getMessage(), e);
}
} else {
jdbcPassword = configuration.getString(PROPERTY_JDBC_PASSWORD);
jdbcPassword = configuration.getString(PROPERTY_JDBC_PASSWORD, DEFAULT_JDBC_PASSWORD);
}

minPoolSize = configuration.getInt(PROPERTY_MIN_POOL_SIZE, DEFAULT_MIN_POOL_SIZE);
maxPoolSize = configuration.getInt(PROPERTY_MAX_POOL_SIZE, DEFAULT_MAX_POOL_SIZE);

useZip = configuration.getBoolean(PROPERTY_USE_ZIP, false);
useZipAudit = configuration.getBoolean(PROPERTY_USE_ZIP_AUDIT, true);
fullObjectFormat = configuration.getString(PROPERTY_FULL_OBJECT_FORMAT, PrismContext.LANG_JSON)
fullObjectFormat = configuration.getString(PROPERTY_FULL_OBJECT_FORMAT, DEFAULT_FULL_OBJECT_FORMAT)
.toLowerCase(); // all language string constants are lower-cases

performanceStatisticsFile = configuration.getString(PROPERTY_PERFORMANCE_STATISTICS_FILE);
Expand All @@ -112,7 +112,7 @@ public void init() throws RepositoryServiceFactoryException {

private void validateConfiguration() throws RepositoryServiceFactoryException {
if (dataSource == null) {
notEmpty(jdbcUrl, "JDBC Url is empty or not defined.");
notEmpty(jdbcUrl, "JDBC URL is empty or not defined.");
// We don't check username and password, they can be null (MID-5342)
// In case of configuration mismatch we let the JDBC driver to fail.
notEmpty(driverClassName, "Driver class name is empty or not defined.");
Expand Down Expand Up @@ -149,12 +149,14 @@ public String getJdbcPassword() {
return jdbcPassword;
}

/** We leave potential compression to PG, for us the values are plain. */
public boolean isUseZip() {
return useZip;
throw new UnsupportedOperationException();
}

/** We leave potential compression to PG, for us the values are plain. */
public boolean isUseZipAudit() {
return useZipAudit;
throw new UnsupportedOperationException();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ public class RefItemDeltaProcessor extends ItemDeltaSingleValueProcessor<Referen
private final UuidPath oidPath;
private final EnumPath<MObjectType> typePath;
private final NumberPath<Integer> relationIdPath;
private final UUID nullOidPlaceholder;

/**
* @param <Q> entity query type from which the attribute is resolved
Expand All @@ -34,13 +33,11 @@ public <Q extends FlexibleRelationalPathBase<R>, R> RefItemDeltaProcessor(
SqaleUpdateContext<?, Q, R> context,
Function<Q, UuidPath> rootToOidPath,
Function<Q, EnumPath<MObjectType>> rootToTypePath,
Function<Q, NumberPath<Integer>> rootToRelationIdPath,
UUID nullOidPlaceholder) {
Function<Q, NumberPath<Integer>> rootToRelationIdPath) {
super(context);
this.oidPath = rootToOidPath.apply(context.entityPath());
this.typePath = rootToTypePath != null ? rootToTypePath.apply(context.entityPath()) : null;
this.relationIdPath = rootToRelationIdPath != null ? rootToRelationIdPath.apply(context.entityPath()) : null;
this.nullOidPlaceholder = nullOidPlaceholder;
}

@Override
Expand All @@ -54,11 +51,7 @@ public void setValue(Referencable value) {

@Override
public void delete() {
if (nullOidPlaceholder != null) {
context.set(oidPath, nullOidPlaceholder);
} else {
context.setNull(oidPath);
}
context.setNull(oidPath);
context.setNull(typePath);
context.setNull(relationIdPath);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

import com.querydsl.core.types.ExpressionUtils;
import com.querydsl.core.types.Predicate;
import com.querydsl.core.types.dsl.BooleanExpression;
import com.querydsl.core.types.dsl.EnumPath;
import com.querydsl.core.types.dsl.NumberPath;
import com.querydsl.core.types.dsl.StringPath;
Expand All @@ -33,8 +32,6 @@
* Filter processor for reference item paths embedded in table as three columns.
* OID is represented by UUID column, type by ID (see {@link MObjectType} and relation
* by Integer (foreign key) to {@link QUri}.
*
* Optionally, this processor supports null OID placeholder value.
*/
public class RefItemFilterProcessor extends ItemValueFilterProcessor<RefFilter> {

Expand All @@ -43,21 +40,18 @@ public class RefItemFilterProcessor extends ItemValueFilterProcessor<RefFilter>
@Nullable private final EnumPath<MObjectType> typePath;
@Nullable private final NumberPath<Integer> relationIdPath;
@Nullable private final StringPath targetNamePath;
@Nullable private final UUID nullOidPlaceholder;

public <Q extends FlexibleRelationalPathBase<R>, R> RefItemFilterProcessor(
SqlQueryContext<?, Q, R> context,
Function<Q, UuidPath> rootToOidPath,
@Nullable Function<Q, EnumPath<MObjectType>> rootToTypePath,
@Nullable Function<Q, NumberPath<Integer>> rootToRelationIdPath,
@Nullable Function<Q, StringPath> rootToTargetNamePath,
@Nullable UUID nullOidPlaceholder) {
@Nullable Function<Q, StringPath> rootToTargetNamePath) {
this(context,
rootToOidPath.apply(context.path()),
rootToTypePath != null ? rootToTypePath.apply(context.path()) : null,
rootToRelationIdPath != null ? rootToRelationIdPath.apply(context.path()) : null,
rootToTargetNamePath != null ? rootToTargetNamePath.apply(context.path()) : null,
nullOidPlaceholder);
rootToTargetNamePath != null ? rootToTargetNamePath.apply(context.path()) : null);
}

// exposed mainly for RefTableItemFilterProcessor
Expand All @@ -66,21 +60,19 @@ <Q extends FlexibleRelationalPathBase<R>, R> RefItemFilterProcessor(
UuidPath oidPath,
@Nullable EnumPath<MObjectType> typePath,
@Nullable NumberPath<Integer> relationIdPath,
@Nullable StringPath targetNamePath,
@Nullable UUID nullOidPlaceholder) {
@Nullable StringPath targetNamePath) {
super(context);
this.oidPath = oidPath;
this.typePath = typePath;
this.relationIdPath = relationIdPath;
this.targetNamePath = targetNamePath;
this.nullOidPlaceholder = nullOidPlaceholder;
}

@Override
public Predicate process(RefFilter filter) {
List<PrismReferenceValue> values = filter.getValues();
if (values == null || values.isEmpty()) {
return filter.isOidNullAsAny() ? null : oidIsNullPredicate();
return filter.isOidNullAsAny() ? null : oidPath.isNull();
}
if (values.size() == 1) {
return processSingleValue(filter, values.get(0));
Expand All @@ -99,7 +91,7 @@ private Predicate processSingleValue(RefFilter filter, PrismReferenceValue ref)
predicate = predicateWithNotTreated(oidPath,
oidPath.eq(UUID.fromString(ref.getOid())));
} else if (!filter.isOidNullAsAny()) {
predicate = oidIsNullPredicate();
predicate = oidPath.isNull();
}

// Audit sometimes does not use target type path
Expand Down Expand Up @@ -131,11 +123,4 @@ private Predicate processSingleValue(RefFilter filter, PrismReferenceValue ref)

return predicate;
}

/** Target OID is null predicate, using placeholder value if configured. */
private BooleanExpression oidIsNullPredicate() {
return nullOidPlaceholder != null
? oidPath.eq(nullOidPlaceholder)
: oidPath.isNull();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public Predicate process(RefFilter filter) {
subquery = subquery
.where(referenceMapping.correlationPredicate().apply(context.path(), ref))
.where(new RefItemFilterProcessor(
context, ref.targetOid, ref.targetType, ref.relationId, null, null)
context, ref.targetOid, ref.targetType, ref.relationId, null)
.process(filter));
if (filter.getValues() == null) {
// If values == null, we search for all items without reference
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.evolveum.midpoint.repo.sqale.mapping;

import java.util.Objects;
import java.util.UUID;
import java.util.function.BiFunction;
import java.util.function.Function;
import java.util.function.Supplier;
Expand Down Expand Up @@ -97,26 +96,11 @@ default <TS, TQ extends QObject<TR>, TR extends MObject> SqaleMappingMixin<S, Q,
@NotNull Function<Q, EnumPath<MObjectType>> rootToTypePath,
@NotNull Function<Q, NumberPath<Integer>> rootToRelationIdPath,
@NotNull Supplier<QueryTableMapping<TS, TQ, TR>> targetMappingSupplier) {
return addRefMapping(itemName,
rootToOidPath, rootToTypePath, rootToRelationIdPath, targetMappingSupplier, null);
}

/**
* Defines single-value reference mapping for both query and modifications, columns embedded in the table.
* This allows defining placeholder value to be stored in DB when target OID is null.
*/
default <TS, TQ extends QObject<TR>, TR extends MObject> SqaleMappingMixin<S, Q, R> addRefMapping(
@NotNull QName itemName,
@NotNull Function<Q, UuidPath> rootToOidPath,
@NotNull Function<Q, EnumPath<MObjectType>> rootToTypePath,
@NotNull Function<Q, NumberPath<Integer>> rootToRelationIdPath,
@NotNull Supplier<QueryTableMapping<TS, TQ, TR>> targetMappingSupplier,
@Nullable UUID nullOidPlaceholder) {
ItemSqlMapper<Q, R> referenceMapping = new SqaleItemSqlMapper<>(
ctx -> new RefItemFilterProcessor(ctx,
rootToOidPath, rootToTypePath, rootToRelationIdPath, null, nullOidPlaceholder),
rootToOidPath, rootToTypePath, rootToRelationIdPath, null),
ctx -> new RefItemDeltaProcessor(ctx,
rootToOidPath, rootToTypePath, rootToRelationIdPath, nullOidPlaceholder));
rootToOidPath, rootToTypePath, rootToRelationIdPath));
addItemMapping(itemName, referenceMapping);

// Needed for queries with ref/@/... paths, this resolves the "ref/" part before @
Expand All @@ -135,7 +119,7 @@ default <TS, TQ extends QObject<TR>, TR extends MObject> SqaleMappingMixin<S, Q,
@NotNull Supplier<QueryTableMapping<TS, TQ, TR>> targetMappingSupplier) {
ItemSqlMapper<Q, R> referenceMapping = new DefaultItemSqlMapper<>(
ctx -> new RefItemFilterProcessor(ctx,
rootToOidPath, rootToTypePath, null, rootToTargetNamePath, null));
rootToOidPath, rootToTypePath, null, rootToTargetNamePath));
addItemMapping(itemName, referenceMapping);

// Needed for queries with ref/@/... paths, this resolves the "ref/" part before @
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -411,23 +411,13 @@ protected void setReference(Referencable ref,
Consumer<UUID> targetOidConsumer,
Consumer<MObjectType> targetTypeConsumer,
Consumer<Integer> relationIdConsumer) {
setReference(ref, targetOidConsumer, targetTypeConsumer, relationIdConsumer, null);
}

protected void setReference(Referencable ref,
Consumer<UUID> targetOidConsumer,
Consumer<MObjectType> targetTypeConsumer,
Consumer<Integer> relationIdConsumer,
UUID nullPlaceholder) {
if (ref != null) {
if (ref.getType() == null) {
ref = SqaleUtils.referenceWithTypeFixed(ref);
}
targetOidConsumer.accept(SqaleUtils.oidToUUid(ref.getOid()));
targetTypeConsumer.accept(schemaTypeToObjectType(ref.getType()));
relationIdConsumer.accept(processCacheableRelation(ref.getRelation()));
} else if (nullPlaceholder != null) {
targetOidConsumer.accept(nullPlaceholder);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ public class MConnector extends MObject {
public String connectorType;
public String connectorVersion;
public Integer frameworkId;

// null ref OID is replaced with QConnectorMapping.NULL_CONNECTOR_HOST_OID
public UUID connectorHostRefTargetOid;
public MObjectType connectorHostRefTargetType;
public Integer connectorHostRefRelationId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ public class QConnector extends QAssignmentHolder<MConnector> {
public static final ColumnMetadata CONNECTOR_BUNDLE =
ColumnMetadata.named("connectorBundle").ofType(Types.VARCHAR);
public static final ColumnMetadata CONNECTOR_TYPE =
ColumnMetadata.named("connectorType").ofType(Types.VARCHAR);
ColumnMetadata.named("connectorType").ofType(Types.VARCHAR).notNull();
public static final ColumnMetadata CONNECTOR_VERSION =
ColumnMetadata.named("connectorVersion").ofType(Types.VARCHAR);
ColumnMetadata.named("connectorVersion").ofType(Types.VARCHAR).notNull();
public static final ColumnMetadata FRAMEWORK_ID =
ColumnMetadata.named("frameworkId").ofType(Types.INTEGER);
public static final ColumnMetadata CONNECTOR_HOST_REF_TARGET_OID =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import static com.evolveum.midpoint.xml.ns._public.common.common_3.ConnectorType.*;

import java.util.Objects;
import java.util.UUID;

import org.jetbrains.annotations.NotNull;

Expand All @@ -26,13 +25,6 @@ public class QConnectorMapping

public static final String DEFAULT_ALIAS_NAME = "con";

/**
* Placeholder non-null value for null reference to connector host.
* This allows for reliable unique index encompassing this column as well.
*/
public static final UUID NULL_CONNECTOR_HOST_OID =
UUID.fromString("00000000-0000-0000-0000-000000000000");

private static QConnectorMapping instance;

// Explanation in class Javadoc for SqaleTableMapping
Expand All @@ -58,8 +50,7 @@ private QConnectorMapping(@NotNull SqaleRepoContext repositoryContext) {
q -> q.connectorHostRefTargetOid,
q -> q.connectorHostRefTargetType,
q -> q.connectorHostRefRelationId,
QConnectorHostMapping::get,
NULL_CONNECTOR_HOST_OID);
QConnectorHostMapping::get);

addItemMapping(F_TARGET_SYSTEM_TYPE, multiUriMapper(q -> q.targetSystemTypes));
}
Expand Down Expand Up @@ -87,8 +78,7 @@ public MConnector newRowObject() {
setReference(schemaObject.getConnectorHostRef(),
o -> row.connectorHostRefTargetOid = o,
t -> row.connectorHostRefTargetType = t,
r -> row.connectorHostRefRelationId = r,
NULL_CONNECTOR_HOST_OID);
r -> row.connectorHostRefRelationId = r);

row.targetSystemTypes = processCacheableUris(schemaObject.getTargetSystemType());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@
import com.evolveum.midpoint.repo.sqale.qmodel.common.MContainer;
import com.evolveum.midpoint.repo.sqale.qmodel.common.MContainerType;
import com.evolveum.midpoint.repo.sqale.qmodel.common.QContainer;
import com.evolveum.midpoint.repo.sqale.qmodel.connector.*;
import com.evolveum.midpoint.repo.sqale.qmodel.connector.MConnector;
import com.evolveum.midpoint.repo.sqale.qmodel.connector.MConnectorHost;
import com.evolveum.midpoint.repo.sqale.qmodel.connector.QConnector;
import com.evolveum.midpoint.repo.sqale.qmodel.connector.QConnectorHost;
import com.evolveum.midpoint.repo.sqale.qmodel.focus.MFocus;
import com.evolveum.midpoint.repo.sqale.qmodel.focus.MUser;
import com.evolveum.midpoint.repo.sqale.qmodel.focus.QGenericObject;
Expand Down Expand Up @@ -1350,11 +1353,11 @@ public void test812ConnectorWithNullConnectorHost() throws Exception {
when("adding it to the repository");
repositoryService.addObject(connector.asPrismObject(), null, result);

then("it is stored and null reference is replaced with UUID placeholder");
then("it is stored and with null connection host reference");
assertThatOperationResult(result).isSuccess();

MConnector row = selectObjectByOid(QConnector.class, connector.getOid());
assertThat(row.connectorHostRefTargetOid).isEqualTo(QConnectorMapping.NULL_CONNECTOR_HOST_OID);
assertThat(row.connectorHostRefTargetOid).isNull();
}

@Test
Expand All @@ -1376,7 +1379,7 @@ public void test813NonUniqueConnector() {
.isInstanceOf(ObjectAlreadyExistsException.class);

assertThatOperationResult(result).isFatalError()
.hasMessageContaining("m_connector_typeversionhost_key");
.hasMessageContaining("m_connector_typeversion_key");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
import com.evolveum.midpoint.repo.sqale.qmodel.assignment.*;
import com.evolveum.midpoint.repo.sqale.qmodel.common.MContainerType;
import com.evolveum.midpoint.repo.sqale.qmodel.connector.QConnector;
import com.evolveum.midpoint.repo.sqale.qmodel.connector.QConnectorMapping;
import com.evolveum.midpoint.repo.sqale.qmodel.focus.MUser;
import com.evolveum.midpoint.repo.sqale.qmodel.focus.QUser;
import com.evolveum.midpoint.repo.sqale.qmodel.focus.QUserMapping;
Expand Down Expand Up @@ -1740,10 +1739,8 @@ public void test195ConnectorUpdateToNullConnectorHost() throws Exception {

then("operation is successful");
assertThatOperationResult(result).isSuccess();

and("reference OID in DB is set to null placeholder value");
assertThat(selectObjectByOid(QConnector.class, oid).connectorHostRefTargetOid)
.isEqualTo(QConnectorMapping.NULL_CONNECTOR_HOST_OID);
// For a day, placeholder value was used instead of NULL, now this test is nothing special, but stays.
assertThat(selectObjectByOid(QConnector.class, oid).connectorHostRefTargetOid).isNull();
}
// endregion

Expand Down

0 comments on commit 3f4ec78

Please sign in to comment.