Skip to content

Commit

Permalink
Mcpods 7732 txn read fix (#284)
Browse files Browse the repository at this point in the history 
* MCPODS-7731 add read deleted features

* MCPODS-7732 txn read fix

* MCPODS-7732 since version fix

---------

Co-authored-by: Pawel Mazurek <[email protected]>
  • Loading branch information
@cyberhead-pl
cyberhead-pl and Pawel Mazurek authored Jun 14, 2024
1 parent 79f0332 commit 7c7b917
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 63 deletions.
1 change: 1 addition & 0 deletions here-naksha-lib-core/src/main/java/com/here/naksha/lib/core/NakshaVersion.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ public class NakshaVersion implements Comparable<NakshaVersion> {
public static final String v2_0_14 = "2.0.14";
public static final String v2_0_15 = "2.0.15";
public static final String v2_0_16 = "2.0.16";
public static final String v2_0_17 = "2.0.17";

/**
* The latest version of the naksha-extension stored in the resources.
Expand Down
39 changes: 39 additions & 0 deletions here-naksha-lib-core/src/main/java/com/here/naksha/lib/core/models/storage/POpHelper.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* Copyright (C) 2017-2024 HERE Europe B.V.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
* License-Filename: LICENSE
*/
package com.here.naksha.lib.core.models.storage;

import com.here.naksha.lib.core.NakshaVersion;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;

public class POpHelper {

/**
* Returns features that are best matches for given txn.
* Means that either feature has exact txn, or it's the last version of feature before given txn.
* !Important: set readAllVersions(true) with this search.
*
* @param value
* @return
*/
@ApiStatus.AvailableSince(NakshaVersion.v2_0_17)
public static @NotNull POp closestTxnNotGreaterThan(@NotNull Number value) {
return POp.and(POp.lte(PRef.txn(), value), POp.or(POp.gt(PRef.txn_next(), value), POp.eq(PRef.txn_next(), 0)));
}
}
5 changes: 5 additions & 0 deletions here-naksha-lib-core/src/main/java/com/here/naksha/lib/core/models/storage/ReadFeatures.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ public boolean isReturnAllVersions() {
return collections;
}

@AvailableSince(NakshaVersion.v2_0_17)
public boolean isReturnDeleted() {
return returnDeleted;
}

@AvailableSince(NakshaVersion.v2_0_7)
public void setCollections(@NotNull List<@NotNull String> collections) {
this.collections = collections;
Expand Down
57 changes: 15 additions & 42 deletions here-naksha-lib-psql/src/main/java/com/here/naksha/lib/psql/PostgresSession.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,8 +333,7 @@ private static void addOp( //
}
}

private static void addPropertyQuery(
@NotNull SQL sql, @NotNull POp propertyOp, @NotNull List<Object> parameter, boolean isHstQuery) {
private static void addPropertyQuery(@NotNull SQL sql, @NotNull POp propertyOp, @NotNull List<Object> parameter) {
final OpType op = propertyOp.op();
if (POpType.AND == op || POpType.OR == op || POpType.NOT == op) {
final List<@NotNull POp> children = propertyOp.children();
Expand All @@ -358,7 +357,7 @@ private static void addPropertyQuery(
} else {
sql.add(op_literal);
}
addPropertyQuery(sql, child, parameter, isHstQuery);
addPropertyQuery(sql, child, parameter);
}
sql.add(")");
return;
Expand Down Expand Up @@ -403,22 +402,6 @@ private static void addPropertyQuery(
}
throw new IllegalArgumentException("STARTS_WITH operator requires a string as value");
}
if (op == POpType.EQ && pref == PRef.txn()) {
sql.add("(");
addJsonPath(sql, path, path.size(), false, true);
sql.add("::int8 <= ?");
if (!(value instanceof Number)) {
throw new IllegalArgumentException("Value must be a number");
}
final Long txn = ((Number) value).longValue();
parameter.add(txn);
if (isHstQuery) {
sql.add(" AND ");
addPropertyQuery(sql, POp.gt(PRef.txn_next(), txn), parameter, true);
}
sql.add(")");
return;
}
addOp(sql, parameter, path, op, value);
}

Expand Down Expand Up @@ -474,10 +457,9 @@ private int fillStatementWithParams(
@NotNull PreparedStatement stmt,
@NotNull List<byte[]> wkbs,
@NotNull List<Object> parameters,
int startParamIdx,
int repeatCount)
throws SQLException {
int i = startParamIdx;
int i = 1;
for (int repetition = 1; repetition <= repeatCount; repetition++) {
for (final byte[] wkb : wkbs) {
stmt.setBytes(i++, wkb);
Expand Down Expand Up @@ -520,41 +502,46 @@ Result executeRead(@NotNull ReadRequest<?> readRequest) {
final SQL sql = sql();
final ArrayList<byte[]> wkbs = new ArrayList<>();
final ArrayList<Object> parameters = new ArrayList<>();
final ArrayList<Object> parametersHst = new ArrayList<>();
SOp spatialOp = readFeatures.getSpatialOp();
if (spatialOp != null) {
addSpatialQuery(sql, spatialOp, wkbs);
}
final String spatial_where = sql.toString();
sql.setLength(0);
POp propertyOp = readFeatures.getPropertyOp();
int repeatParameters = 0;
if (propertyOp != null) {
addPropertyQuery(sql, propertyOp, parameters, false);
addPropertyQuery(sql, propertyOp, parameters);
}
final String props_where = sql.toString();
sql.setLength(0);
boolean first = true;
for (final String collection : collections) {
repeatParameters++;
if (first) {
first = false;
} else {
sql.add(" UNION ALL ");
}
SQL headQuery = prepareQuery(collection, spatial_where, props_where, readFeatures.getLimit());
sql.add(headQuery);
if (readFeatures.isReturnDeleted()) {
sql.add(" UNION ALL ");
SQL delSql = prepareQuery(collection + "_del", spatial_where, props_where, readFeatures.getLimit());
sql.add(delSql);
repeatParameters++;
}
if (readFeatures.isReturnAllVersions()) {
sql.add(" UNION ALL ");
SQL hstSql = prepareHstSql(collection, propertyOp, parametersHst, spatial_where, readFeatures);
SQL hstSql = prepareQuery(collection + "_hst", spatial_where, props_where, readFeatures.getLimit());
sql.add(hstSql);
repeatParameters++;
}
}
final String query = sql.toString();
final PreparedStatement stmt = prepareStatement(query);
try {
int lastParamIdx = fillStatementWithParams(stmt, wkbs, parameters, 1, collections.size());
if (readFeatures.isReturnAllVersions()) {
fillStatementWithParams(stmt, wkbs, parametersHst, lastParamIdx, 1);
}
fillStatementWithParams(stmt, wkbs, parameters, repeatParameters);
final ResultSet rs = stmt.executeQuery();
final PsqlCursor<XyzFeature, XyzFeatureCodec> cursor =
new PsqlCursor<>(XyzFeatureCodecFactory.get(), this, stmt, rs);
Expand All @@ -574,20 +561,6 @@ Result executeRead(@NotNull ReadRequest<?> readRequest) {
return new ErrorResult(XyzError.NOT_IMPLEMENTED, "executeRead");
}

private SQL prepareHstSql(
String collection,
POp propertyOp,
ArrayList<Object> parametersHst,
String spatial_where,
ReadFeatures readFeatures) {
String historyCollection = collection + "_hst";
SQL hst_props_where = new SQL();
if (propertyOp != null) {
addPropertyQuery(hst_props_where, propertyOp, parametersHst, true);
}
return prepareQuery(historyCollection, spatial_where, hst_props_where.toString(), readFeatures.getLimit());
}

@NotNull
<FEATURE, CODEC extends FeatureCodec<FEATURE, CODEC>> Result executeWrite(
@NotNull WriteRequest<FEATURE, CODEC, ?> writeRequest) {
Expand Down
75 changes: 54 additions & 21 deletions here-naksha-lib-psql/src/test/java/com/here/naksha/lib/psql/PsqlStorageTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
import com.here.naksha.lib.core.models.storage.MutableCursor;
import com.here.naksha.lib.core.models.storage.NonIndexedPRef;
import com.here.naksha.lib.core.models.storage.POp;
import com.here.naksha.lib.core.models.storage.POpHelper;
import com.here.naksha.lib.core.models.storage.PRef;
import com.here.naksha.lib.core.models.storage.ReadFeatures;
import com.here.naksha.lib.core.models.storage.Result;
Expand All @@ -79,7 +80,9 @@
import com.here.naksha.lib.core.models.storage.XyzFeatureCodec;
import com.here.naksha.lib.core.util.json.Json;
import com.here.naksha.lib.core.util.storage.RequestHelper;

import java.util.ArrayList;

import org.locationtech.jts.geom.Coordinate;
import org.locationtech.jts.geom.Geometry;
import org.locationtech.jts.geom.Point;
Expand All @@ -90,6 +93,7 @@
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.Arrays;
import java.util.List;
import java.util.NoSuchElementException;
import java.util.Objects;
Expand Down Expand Up @@ -514,8 +518,22 @@ void singleFeatureGetSpecificVersionRead() throws NoCursor {
session.execute(requestForPreviousVersion).getXyzMutableCursor()) {
cursor.next();
XyzNamespace xyzNamespace = cursor.getFeature().getProperties().getXyzNamespace();
assertEquals(puuid, xyzNamespace.getUuid());
assertTrue( txnOfMiddleVersion > xyzNamespace.getTxn());
assertEquals(puuid, xyzNamespace.getUuid());
assertTrue(txnOfMiddleVersion > xyzNamespace.getTxn());
assertFalse(cursor.hasNext());
}

// get closest version
long futureTxn = txnOfMiddleVersion + 100;
final ReadFeatures requestForClosestVersion = RequestHelper.readFeaturesByIdRequest(collectionId(), SINGLE_FEATURE_ID);
requestForClosestVersion.withReturnAllVersions(true);
requestForClosestVersion.setPropertyOp(POpHelper.closestTxnNotGreaterThan(futureTxn));

try (final MutableCursor<XyzFeature, XyzFeatureCodec> cursor =
session.execute(requestForClosestVersion).getXyzMutableCursor()) {
cursor.next();
XyzNamespace xyzNamespace = cursor.getFeature().getProperties().getXyzNamespace();
assertTrue(futureTxn > xyzNamespace.getTxn());
assertFalse(cursor.hasNext());
}
}
Expand Down Expand Up @@ -650,6 +668,26 @@ void testInvalidUuid() throws NoCursor {
session.commit(true);
}

@Test
@Order(62)
@EnabledIf("runTest")
void multiCollectionRead() throws NoCursor {
assertNotNull(storage);
assertNotNull(session);
// given
/**
* data inserted in {@link #singleFeatureCreate()} test and updated by {@link #singleFeatureUpdate()}.
*/
final ReadFeatures request = RequestHelper.readFeaturesByIdRequest(collectionId(), SINGLE_FEATURE_ID);
request.setCollections(List.of(collectionId(), collectionId()));

try (final MutableCursor<XyzFeature, XyzFeatureCodec> cursor =
session.execute(request).getXyzMutableCursor()) {
// then
assertEquals(2, cursor.asList().size());
}
}

@Test
@Order(64)
@EnabledIf("runTest")
Expand All @@ -662,7 +700,7 @@ void singleFeatureDeleteById() throws NoCursor {
request.add(EWriteOp.CREATE, feature);

// when
try(final Result result = session.execute(request)) {
try (final Result result = session.execute(request)) {
final WriteXyzFeatures delRequest = new WriteXyzFeatures(collectionId());
delRequest.delete("TO_DEL_BY_ID", null);
try (final ForwardCursor<XyzFeature, XyzFeatureCodec> cursor =
Expand Down Expand Up @@ -742,33 +780,31 @@ void singleFeatureDeleteVerify() throws SQLException, NoCursor {
final ReadFeatures requestWithDeleted =
RequestHelper.readFeaturesByIdRequest(collectionId(), SINGLE_FEATURE_ID);
requestWithDeleted.withReturnDeleted(true);
String featureJsonBeforeDeletion;
String featureJsonFromDel;

/* TODO uncomment it when read with deleted is ready.
try (final ResultCursor<XyzFeature> cursor =
session.execute(requestWithDeleted).cursor()) {
try (final ForwardCursor<XyzFeature, XyzFeatureCodec> cursor =
session.execute(requestWithDeleted).getXyzFeatureCursor()) {
cursor.next();
final XyzFeature feature = cursor.getFeature();
XyzNamespace xyz = feature.xyz();

// then
assertSame(EExecutedOp.DELETED, cursor.getOp());
assertSame(EExecutedOp.READ, cursor.getOp());
final String id = cursor.getId();
assertEquals(SINGLE_FEATURE_ID, id);
final String uuid = cursor.getUuid();
assertNotNull(uuid);
final Geometry geometry = cursor.getGeometry();
assertNotNull(geometry);
assertEquals(new Coordinate(5.1d, 6.0d, 2.1d), geometry.getCoordinate());
assertEquals(new Coordinate(5.0d, 6.0d, 2.0d), geometry.getCoordinate());
assertNotNull(feature);
assertEquals(SINGLE_FEATURE_ID, feature.getId());
assertEquals(uuid, feature.xyz().getUuid());
assertSame(EXyzAction.DELETE, feature.xyz().getAction());
featureJsonBeforeDeletion = cursor.getJson()
assertFalse(cursor.next());
featureJsonFromDel = cursor.getJson();
assertFalse(cursor.hasNext());
}
*/

/**
* Check directly _del table.
*/
Expand All @@ -778,10 +814,7 @@ void singleFeatureDeleteVerify() throws SQLException, NoCursor {

// feature exists in _del table
assertTrue(rs.next());

/* FIXME uncomment this when read with deleted is ready.
assertEquals(featureJsonBeforeDeletion, rs.getString(1));
*/
assertEquals(featureJsonFromDel, rs.getString(1));
}
}

Expand Down Expand Up @@ -858,14 +891,14 @@ void autoPurgeCheck() throws SQLException, NoCursor {

// CREATE feature
final XyzFeature featureToDel = new XyzFeature(SINGLE_FEATURE_ID);
try(final Result result = session.execute(createFeatureRequest(collectionWithAutoPurge, featureToDel))) {
try (final Result result = session.execute(createFeatureRequest(collectionWithAutoPurge, featureToDel))) {
assertTrue(result instanceof SuccessResult);
} finally {
session.commit(true);
}

// DELETE feature
try(final Result result = session.execute(deleteFeatureByIdRequest(collectionWithAutoPurge, featureToDel.getId()))) {
try (final Result result = session.execute(deleteFeatureByIdRequest(collectionWithAutoPurge, featureToDel.getId()))) {
assertTrue(result instanceof SuccessResult);
} finally {
session.commit(true);
Expand Down Expand Up @@ -961,7 +994,7 @@ void multipleFeaturesRead() throws NoCursor {
assertNotNull(tags);
assertTrue(tags.size() > 0);
assertTrue(tags.contains("@:firstName:" + fg.firstNames[0])
|| tags.contains("@:firstName:" + fg.firstNames[1]));
|| tags.contains("@:firstName:" + fg.firstNames[1]));
}
} finally {
session.commit(true);
Expand Down Expand Up @@ -1047,7 +1080,7 @@ void limitedRead() throws NoCursor {
private void limitToN(final long limit) throws NoCursor {
final ReadFeatures request = new ReadFeatures(collectionId()).withLimit(limit);
try (final @NotNull ForwardCursor<XyzFeature, XyzFeatureCodec> cursor =
session.execute(request).getXyzFeatureCursor()) {
session.execute(request).getXyzFeatureCursor()) {

for (long row = 1; row <= limit; row++) {
assertTrue(cursor.hasNext());
Expand Down

0 comments on commit 7c7b917

Please sign in to comment.