Skip to content

Commit

Permalink
Merge pull request eXist-db#4621 from line-o/fix/map-merge-2
Browse files Browse the repository at this point in the history
allow all options in map:merge#2
  • Loading branch information
adamretter authored Nov 30, 2022
2 parents 60bb4c1 + a6b620b commit a78f84a
Show file tree
Hide file tree
Showing 5 changed files with 191 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

import javax.annotation.Nullable;
import java.util.List;
import java.util.function.BinaryOperator;

/**
* Abstract base class for map types. A map item is also a function item. This class thus extends
Expand Down Expand Up @@ -70,11 +71,11 @@ public abstract class AbstractMapType extends FunctionReference

protected XQueryContext context;

protected AbstractMapType(XQueryContext context) {
protected AbstractMapType(final XQueryContext context) {
this(null, context);
}

protected AbstractMapType(final Expression expression, XQueryContext context) {
protected AbstractMapType(final Expression expression, final XQueryContext context) {
super(expression, null);
this.context = context;
}
Expand All @@ -85,12 +86,15 @@ protected AbstractMapType(final Expression expression, XQueryContext context) {

public abstract AbstractMapType merge(Iterable<AbstractMapType> others);

public abstract AbstractMapType merge(Iterable<AbstractMapType> others, BinaryOperator<Sequence> mergeFn);

public abstract boolean contains(AtomicValue key);

public abstract Sequence keys();

public abstract AbstractMapType remove(AtomicValue[] keysAtomicValues) throws XPathException;

@SuppressWarnings("unused")
public abstract int getKeyType();

public abstract int size();
Expand All @@ -106,12 +110,12 @@ public int getType() {
}

@Override
public void analyze(AnalyzeContextInfo contextInfo) throws XPathException {
public void analyze(final AnalyzeContextInfo contextInfo) throws XPathException {
getAccessorFunc().analyze(contextInfo);
}

@Override
public Sequence eval(Sequence contextSequence) throws XPathException {
public Sequence eval(final Sequence contextSequence) throws XPathException {
return getAccessorFunc().eval(contextSequence);
}

Expand All @@ -127,12 +131,12 @@ public Sequence evalFunction(final Sequence contextSequence, final Item contextI
}

@Override
public void setArguments(List<Expression> arguments) throws XPathException {
public void setArguments(final List<Expression> arguments) throws XPathException {
getAccessorFunc().setArguments(arguments);
}

@Override
public void resetState(boolean postOptimization) {
public void resetState(final boolean postOptimization) {
getAccessorFunc().resetState(postOptimization);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,10 @@ private Sequence merge(final Sequence[] args) throws XPathException {

final MergeDuplicates mergeDuplicates;
if (args.length == 2) {
final Sequence mapValue = ((MapType) args[1]).get(new StringValue(this, "duplicates"));
if (mapValue != null) {
final MapType map = (MapType) args[1];
final StringValue key = new StringValue(this, "duplicates");
if (map.contains(key)) {
final Sequence mapValue = map.get(key);
mergeDuplicates = MergeDuplicates.fromDuplicatesValue(mapValue.getStringValue());
if (mergeDuplicates == null) {
throw new XPathException(this, ErrorCodes.FOJS0005, "value for duplicates key was not recognised: " + mapValue.getStringValue());
Expand All @@ -265,19 +267,15 @@ private Sequence merge(final Sequence[] args) throws XPathException {
mergeDuplicates = MergeDuplicates.USE_LAST;
}

if (mergeDuplicates == MergeDuplicates.REJECT) {
throw new XPathException(this, ErrorCodes.FOJS0005, "map { \"duplicates\": \"reject\" } is not yet implemented by eXist-db");
} else if (mergeDuplicates == MergeDuplicates.COMBINE) {
throw new XPathException(this, ErrorCodes.FOJS0005, "map { \"combine\": \"reject\" } is not yet implemented by eXist-db");
}

final Sequence maps = args[0];
final int totalMaps = maps.getItemCount();
final AbstractMapType head;
final List<AbstractMapType> tail = new ArrayList<>(totalMaps - 1);

if (mergeDuplicates == MergeDuplicates.USE_LAST) {
if (mergeDuplicates == MergeDuplicates.USE_LAST || mergeDuplicates == MergeDuplicates.COMBINE) {
// head is the first map
// USE_LAST will pick the item from the last map containing a duplicate item
// COMBINE will combine duplicate items in head-first order
head = (AbstractMapType) maps.itemAt(0);
for (int i = 1; i < totalMaps; i++) {
final AbstractMapType other = (AbstractMapType) maps.itemAt(i);
Expand All @@ -286,14 +284,51 @@ private Sequence merge(final Sequence[] args) throws XPathException {

} else {
// head is the last map
// USE_FIRST will pick the item from the first map containing a duplicate item
head = (AbstractMapType) maps.itemAt(totalMaps - 1);
for (int i = totalMaps - 2; i >= 0; i--) {
final AbstractMapType other = (AbstractMapType) maps.itemAt(i);
tail.add(other);
}
}

return head.merge(tail);
if (mergeDuplicates == MergeDuplicates.COMBINE) {
// Provide a callback function for merging items which share a key
// Call merge variant
final List<XPathException> mergeExceptions = new ArrayList<>();
final AbstractMapType merged
= head.merge(tail, (first, second) -> {
try {
final ValueSequence sequence = new ValueSequence(first);
sequence.addAll(second);
return sequence;
} catch (final XPathException e) {
//We cannot throw out of the MapType - pass exceptions here.
mergeExceptions.add(e);
}
return Sequence.EMPTY_SEQUENCE;
});
if (!mergeExceptions.isEmpty()) {
throw mergeExceptions.get(0);
}
return merged;
}

final AbstractMapType result = head.merge(tail);

if (mergeDuplicates == MergeDuplicates.REJECT) {

int inputItemsSize = head.size();
for (final AbstractMapType other : tail) {
inputItemsSize += other.size();
}
if (inputItemsSize > result.size()) {
// no duplicates, so we don't need to consider the duplicates
throw new XPathException(this, ErrorCodes.FOJS0003, "map { \"duplicates\": \"reject\" } maps had duplicate entry");
}
}

return result;
}

private Sequence forEach(final Sequence[] args) throws XPathException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,15 @@

import javax.annotation.Nullable;
import java.util.Iterator;
import java.util.Optional;
import java.util.function.BinaryOperator;
import java.util.function.ToLongFunction;

/**
* Full implementation of the XDM map() type based on an
* immutable hash-map.
*
* @author <a href="mailto:[email protected]">Adam Rettter</a>
* @author <a href="mailto:[email protected]">Adam Retter</a>
*/
public class MapType extends AbstractMapType {

Expand Down Expand Up @@ -208,6 +210,43 @@ public AbstractMapType merge(final Iterable<AbstractMapType> others) {
return new MapType(getExpression(), context, newMap.forked(), prevType);
}

@Override
public AbstractMapType merge(final Iterable<AbstractMapType> others, final BinaryOperator<Sequence> mergeFn) {

// create a transient map
IMap<AtomicValue, Sequence> newMap = map.linear();

int prevType = keyType;
for (final AbstractMapType other: others) {
if (other instanceof MapType) {
// MapType - optimise merge
final MapType otherMap = (MapType) other;
newMap = newMap.merge(otherMap.map, mergeFn);

if (prevType != otherMap.keyType) {
prevType = MIXED_KEY_TYPES;
}
} else {
// non MapType
for (final IEntry<AtomicValue, Sequence> entry : other) {
final AtomicValue key = entry.key();
final Optional<Sequence> headEntry = newMap.get(key);
if (headEntry.isPresent()) {
newMap = newMap.put(key, mergeFn.apply(headEntry.get(), entry.value()));
} else {
newMap = newMap.put(key, entry.value());
}
if (prevType != key.getType()) {
prevType = MIXED_KEY_TYPES;
}
}
}
}

// return an immutable map
return new MapType(context, newMap.forked(), prevType);
}

public void add(final AtomicValue key, final Sequence value) {
setKeyType(key.getType());
map = map.put(key, value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

import javax.annotation.Nullable;
import java.util.Iterator;
import java.util.function.BinaryOperator;

import static org.exist.xquery.functions.map.MapType.newLinearMap;

Expand All @@ -44,9 +45,9 @@
*/
public class SingleKeyMapType extends AbstractMapType {

private AtomicValue key;
private Sequence value;
private @Nullable Collator collator;
private final AtomicValue key;
private final Sequence value;
private final @Nullable Collator collator;

public SingleKeyMapType(final XQueryContext context, final @Nullable Collator collator, final AtomicValue key, final Sequence value) {
this(null, context, collator, key, value);
Expand Down Expand Up @@ -79,6 +80,13 @@ public AbstractMapType merge(final Iterable<AbstractMapType> others) {
}
}

@Override
public AbstractMapType merge(final Iterable<AbstractMapType> others, final BinaryOperator<Sequence> mergeFn) {
try (final MapType map = new MapType(context, collator, key, value)) {
return map.merge(others, mergeFn);
}
}

@Override
public AbstractMapType put(final AtomicValue key, final Sequence value) {
final IMap<AtomicValue, Sequence> map = newLinearMap(collator);
Expand Down
85 changes: 85 additions & 0 deletions exist-core/src/test/xquery/maps/maps.xqm
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,78 @@ function mt:merge-duplicate-keys-use-last-explicit-2() {
($mt:integerKeys(7), $specialWeek(7))
};

declare
%test:assertError("err:FOJS0003")
function mt:merge-duplicate-keys-reject-has-duplicates() {
let $specialWeek := map:merge((map { 7 : "Caturday" }, $mt:integerKeys), map { "duplicates": "reject" })
return
($specialWeek(7))
};

declare
%test:assertEquals("Saturday", "Saturday", "Caturday")
function mt:merge-duplicate-keys-reject-no-duplicates() {
let $specialWeek := map:merge((map { 8 : "Caturday" }, $mt:integerKeys), map { "duplicates": "reject" })
return
($mt:integerKeys(7), $specialWeek(7), $specialWeek(8))
};

declare
%test:assertEquals("Caturday","Maturday","Saturday")
function mt:merge-duplicate-keys-combine-has-duplicates-sequence() {
let $specialWeek := map:merge((map { 7 : ("Caturday","Maturday") }, $mt:integerKeys), map { "duplicates": "combine" })
return
($specialWeek(7))
};

declare
%test:assertEquals("Saturday","Caturday","Maturday")
function mt:merge-duplicate-keys-combine-has-duplicates-sequence-order() {
let $specialWeek := map:merge(($mt:integerKeys, map { 7 : ("Caturday","Maturday") }), map { "duplicates": "combine" })
return
($specialWeek(7))
};

declare
%test:assertEquals("Saturday","Caturday","Maturday", "Caturday", "Zaturday")
function mt:merge-duplicate-keys-combine-3-has-duplicates-sequence-order() {
let $specialWeek := map:merge(($mt:integerKeys, map { 7 : ("Caturday","Maturday") }, map { 7 : ("Caturday","Zaturday") }), map { "duplicates": "combine" })
return
($specialWeek(7))
};

declare
%test:assertEquals("Caturday","Saturday")
function mt:merge-duplicate-keys-combine-has-duplicates-atomic() {
let $specialWeek := map:merge((map { 7 : ("Caturday") }, $mt:integerKeys), map { "duplicates": "combine" })
return
($specialWeek(7))
};

declare
%test:assertEquals("Caturday","Saturday", "Maturday")
function mt:merge-duplicate-keys-combine-has-duplicates-three() {
let $specialWeek := map:merge((map { 7 : ("Caturday") }, $mt:integerKeys, map { 7: ("Maturday")}), map { "duplicates": "combine" })
return
($specialWeek(7))
};

declare
%test:assertEquals("Saturday", "Caturday", "Maturday", "Waturday")
function mt:merge-duplicate-keys-combine-has-duplicates-four() {
let $specialWeek := map:merge((map { 7 : () }, $mt:integerKeys, map { 7: ("Caturday", "Maturday", "Waturday")}), map { "duplicates": "combine" })
return
($specialWeek(7))
};

declare
%test:assertEquals("Caturday","Saturday")
function mt:merge-duplicate-keys-combine-has-duplicates-empty-third() {
let $specialWeek := map:merge((map { 7 : ("Caturday") }, $mt:integerKeys, map { 7: ()}), map { "duplicates": "combine" })
return
($specialWeek(7))
};

declare
%test:assertEmpty
function mt:mapEmptyValue() {
Expand Down Expand Up @@ -904,3 +976,16 @@ function mt:immutable-merge-duplicates-then-merge() {
$expected ne $result($mt:test-key-one)
)
};

(:~
: ensure that empty options map is allowed and behaves like
: map:merge#1
:)
declare
%test:assertTrue
function mt:map-merge-2-empty-options-map() {
let $maps := (mt:getMapFixture(), map { "Su": "Sunnuntai" })
let $expected := map:merge($maps)
let $actual := map:merge($maps, map {})
return $expected?Su eq $actual?Su
};

0 comments on commit a78f84a

Please sign in to comment.