Skip to content

Commit a78f84a

Browse files
adamretteradamretter
authored
Merge pull request #4621 from line-o/fix/map-merge-2
allow all options in map:merge#2
2 parents 60bb4c1 + a6b620b commit a78f84a

File tree

5 files changed

+191
-20
lines changed

5 files changed

+191
-20
lines changed

‎exist-core/src/main/java/org/exist/xquery/functions/map/AbstractMapType.java‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131

3232
import javax.annotation.Nullable;
3333
import java.util.List;
34+
import java.util.function.BinaryOperator;
3435

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

7172
protected XQueryContext context;
7273

73-
protected AbstractMapType(XQueryContext context) {
74+
protected AbstractMapType(final XQueryContext context) {
7475
this(null, context);
7576
}
7677

77-
protected AbstractMapType(final Expression expression, XQueryContext context) {
78+
protected AbstractMapType(final Expression expression, final XQueryContext context) {
7879
super(expression, null);
7980
this.context = context;
8081
}
@@ -85,12 +86,15 @@ protected AbstractMapType(final Expression expression, XQueryContext context) {
8586

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

89+
public abstract AbstractMapType merge(Iterable<AbstractMapType> others, BinaryOperator<Sequence> mergeFn);
90+
8891
public abstract boolean contains(AtomicValue key);
8992

9093
public abstract Sequence keys();
9194

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

97+
@SuppressWarnings("unused")
9498
public abstract int getKeyType();
9599

96100
public abstract int size();
@@ -106,12 +110,12 @@ public int getType() {
106110
}
107111

108112
@Override
109-
public void analyze(AnalyzeContextInfo contextInfo) throws XPathException {
113+
public void analyze(final AnalyzeContextInfo contextInfo) throws XPathException {
110114
getAccessorFunc().analyze(contextInfo);
111115
}
112116

113117
@Override
114-
public Sequence eval(Sequence contextSequence) throws XPathException {
118+
public Sequence eval(final Sequence contextSequence) throws XPathException {
115119
return getAccessorFunc().eval(contextSequence);
116120
}
117121

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

129133
@Override
130-
public void setArguments(List<Expression> arguments) throws XPathException {
134+
public void setArguments(final List<Expression> arguments) throws XPathException {
131135
getAccessorFunc().setArguments(arguments);
132136
}
133137

134138
@Override
135-
public void resetState(boolean postOptimization) {
139+
public void resetState(final boolean postOptimization) {
136140
getAccessorFunc().resetState(postOptimization);
137141
}
138142

‎exist-core/src/main/java/org/exist/xquery/functions/map/MapFunction.java‎

Lines changed: 45 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -250,8 +250,10 @@ private Sequence merge(final Sequence[] args) throws XPathException {
250250

251251
final MergeDuplicates mergeDuplicates;
252252
if (args.length == 2) {
253-
final Sequence mapValue = ((MapType) args[1]).get(new StringValue(this, "duplicates"));
254-
if (mapValue != null) {
253+
final MapType map = (MapType) args[1];
254+
final StringValue key = new StringValue(this, "duplicates");
255+
if (map.contains(key)) {
256+
final Sequence mapValue = map.get(key);
255257
mergeDuplicates = MergeDuplicates.fromDuplicatesValue(mapValue.getStringValue());
256258
if (mergeDuplicates == null) {
257259
throw new XPathException(this, ErrorCodes.FOJS0005, "value for duplicates key was not recognised: " + mapValue.getStringValue());
@@ -265,19 +267,15 @@ private Sequence merge(final Sequence[] args) throws XPathException {
265267
mergeDuplicates = MergeDuplicates.USE_LAST;
266268
}
267269

268-
if (mergeDuplicates == MergeDuplicates.REJECT) {
269-
throw new XPathException(this, ErrorCodes.FOJS0005, "map { \"duplicates\": \"reject\" } is not yet implemented by eXist-db");
270-
} else if (mergeDuplicates == MergeDuplicates.COMBINE) {
271-
throw new XPathException(this, ErrorCodes.FOJS0005, "map { \"combine\": \"reject\" } is not yet implemented by eXist-db");
272-
}
273-
274270
final Sequence maps = args[0];
275271
final int totalMaps = maps.getItemCount();
276272
final AbstractMapType head;
277273
final List<AbstractMapType> tail = new ArrayList<>(totalMaps - 1);
278274

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

287285
} else {
288286
// head is the last map
287+
// USE_FIRST will pick the item from the first map containing a duplicate item
289288
head = (AbstractMapType) maps.itemAt(totalMaps - 1);
290289
for (int i = totalMaps - 2; i >= 0; i--) {
291290
final AbstractMapType other = (AbstractMapType) maps.itemAt(i);
292291
tail.add(other);
293292
}
294293
}
295294

296-
return head.merge(tail);
295+
if (mergeDuplicates == MergeDuplicates.COMBINE) {
296+
// Provide a callback function for merging items which share a key
297+
// Call merge variant
298+
final List<XPathException> mergeExceptions = new ArrayList<>();
299+
final AbstractMapType merged
300+
= head.merge(tail, (first, second) -> {
301+
try {
302+
final ValueSequence sequence = new ValueSequence(first);
303+
sequence.addAll(second);
304+
return sequence;
305+
} catch (final XPathException e) {
306+
//We cannot throw out of the MapType - pass exceptions here.
307+
mergeExceptions.add(e);
308+
}
309+
return Sequence.EMPTY_SEQUENCE;
310+
});
311+
if (!mergeExceptions.isEmpty()) {
312+
throw mergeExceptions.get(0);
313+
}
314+
return merged;
315+
}
316+
317+
final AbstractMapType result = head.merge(tail);
318+
319+
if (mergeDuplicates == MergeDuplicates.REJECT) {
320+
321+
int inputItemsSize = head.size();
322+
for (final AbstractMapType other : tail) {
323+
inputItemsSize += other.size();
324+
}
325+
if (inputItemsSize > result.size()) {
326+
// no duplicates, so we don't need to consider the duplicates
327+
throw new XPathException(this, ErrorCodes.FOJS0003, "map { \"duplicates\": \"reject\" } maps had duplicate entry");
328+
}
329+
}
330+
331+
return result;
297332
}
298333

299334
private Sequence forEach(final Sequence[] args) throws XPathException {

‎exist-core/src/main/java/org/exist/xquery/functions/map/MapType.java‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,15 @@
4343

4444
import javax.annotation.Nullable;
4545
import java.util.Iterator;
46+
import java.util.Optional;
47+
import java.util.function.BinaryOperator;
4648
import java.util.function.ToLongFunction;
4749

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

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

213+
@Override
214+
public AbstractMapType merge(final Iterable<AbstractMapType> others, final BinaryOperator<Sequence> mergeFn) {
215+
216+
// create a transient map
217+
IMap<AtomicValue, Sequence> newMap = map.linear();
218+
219+
int prevType = keyType;
220+
for (final AbstractMapType other: others) {
221+
if (other instanceof MapType) {
222+
// MapType - optimise merge
223+
final MapType otherMap = (MapType) other;
224+
newMap = newMap.merge(otherMap.map, mergeFn);
225+
226+
if (prevType != otherMap.keyType) {
227+
prevType = MIXED_KEY_TYPES;
228+
}
229+
} else {
230+
// non MapType
231+
for (final IEntry<AtomicValue, Sequence> entry : other) {
232+
final AtomicValue key = entry.key();
233+
final Optional<Sequence> headEntry = newMap.get(key);
234+
if (headEntry.isPresent()) {
235+
newMap = newMap.put(key, mergeFn.apply(headEntry.get(), entry.value()));
236+
} else {
237+
newMap = newMap.put(key, entry.value());
238+
}
239+
if (prevType != key.getType()) {
240+
prevType = MIXED_KEY_TYPES;
241+
}
242+
}
243+
}
244+
}
245+
246+
// return an immutable map
247+
return new MapType(context, newMap.forked(), prevType);
248+
}
249+
211250
public void add(final AtomicValue key, final Sequence value) {
212251
setKeyType(key.getType());
213252
map = map.put(key, value);

‎exist-core/src/main/java/org/exist/xquery/functions/map/SingleKeyMapType.java‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333

3434
import javax.annotation.Nullable;
3535
import java.util.Iterator;
36+
import java.util.function.BinaryOperator;
3637

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

@@ -44,9 +45,9 @@
4445
*/
4546
public class SingleKeyMapType extends AbstractMapType {
4647

47-
private AtomicValue key;
48-
private Sequence value;
49-
private @Nullable Collator collator;
48+
private final AtomicValue key;
49+
private final Sequence value;
50+
private final @Nullable Collator collator;
5051

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

83+
@Override
84+
public AbstractMapType merge(final Iterable<AbstractMapType> others, final BinaryOperator<Sequence> mergeFn) {
85+
try (final MapType map = new MapType(context, collator, key, value)) {
86+
return map.merge(others, mergeFn);
87+
}
88+
}
89+
8290
@Override
8391
public AbstractMapType put(final AtomicValue key, final Sequence value) {
8492
final IMap<AtomicValue, Sequence> map = newLinearMap(collator);

‎exist-core/src/test/xquery/maps/maps.xqm‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,78 @@ function mt:merge-duplicate-keys-use-last-explicit-2() {
250250
($mt:integerKeys(7), $specialWeek(7))
251251
};
252252

253+
declare
254+
%test:assertError("err:FOJS0003")
255+
function mt:merge-duplicate-keys-reject-has-duplicates() {
256+
let $specialWeek := map:merge((map { 7 : "Caturday" }, $mt:integerKeys), map { "duplicates": "reject" })
257+
return
258+
($specialWeek(7))
259+
};
260+
261+
declare
262+
%test:assertEquals("Saturday", "Saturday", "Caturday")
263+
function mt:merge-duplicate-keys-reject-no-duplicates() {
264+
let $specialWeek := map:merge((map { 8 : "Caturday" }, $mt:integerKeys), map { "duplicates": "reject" })
265+
return
266+
($mt:integerKeys(7), $specialWeek(7), $specialWeek(8))
267+
};
268+
269+
declare
270+
%test:assertEquals("Caturday","Maturday","Saturday")
271+
function mt:merge-duplicate-keys-combine-has-duplicates-sequence() {
272+
let $specialWeek := map:merge((map { 7 : ("Caturday","Maturday") }, $mt:integerKeys), map { "duplicates": "combine" })
273+
return
274+
($specialWeek(7))
275+
};
276+
277+
declare
278+
%test:assertEquals("Saturday","Caturday","Maturday")
279+
function mt:merge-duplicate-keys-combine-has-duplicates-sequence-order() {
280+
let $specialWeek := map:merge(($mt:integerKeys, map { 7 : ("Caturday","Maturday") }), map { "duplicates": "combine" })
281+
return
282+
($specialWeek(7))
283+
};
284+
285+
declare
286+
%test:assertEquals("Saturday","Caturday","Maturday", "Caturday", "Zaturday")
287+
function mt:merge-duplicate-keys-combine-3-has-duplicates-sequence-order() {
288+
let $specialWeek := map:merge(($mt:integerKeys, map { 7 : ("Caturday","Maturday") }, map { 7 : ("Caturday","Zaturday") }), map { "duplicates": "combine" })
289+
return
290+
($specialWeek(7))
291+
};
292+
293+
declare
294+
%test:assertEquals("Caturday","Saturday")
295+
function mt:merge-duplicate-keys-combine-has-duplicates-atomic() {
296+
let $specialWeek := map:merge((map { 7 : ("Caturday") }, $mt:integerKeys), map { "duplicates": "combine" })
297+
return
298+
($specialWeek(7))
299+
};
300+
301+
declare
302+
%test:assertEquals("Caturday","Saturday", "Maturday")
303+
function mt:merge-duplicate-keys-combine-has-duplicates-three() {
304+
let $specialWeek := map:merge((map { 7 : ("Caturday") }, $mt:integerKeys, map { 7: ("Maturday")}), map { "duplicates": "combine" })
305+
return
306+
($specialWeek(7))
307+
};
308+
309+
declare
310+
%test:assertEquals("Saturday", "Caturday", "Maturday", "Waturday")
311+
function mt:merge-duplicate-keys-combine-has-duplicates-four() {
312+
let $specialWeek := map:merge((map { 7 : () }, $mt:integerKeys, map { 7: ("Caturday", "Maturday", "Waturday")}), map { "duplicates": "combine" })
313+
return
314+
($specialWeek(7))
315+
};
316+
317+
declare
318+
%test:assertEquals("Caturday","Saturday")
319+
function mt:merge-duplicate-keys-combine-has-duplicates-empty-third() {
320+
let $specialWeek := map:merge((map { 7 : ("Caturday") }, $mt:integerKeys, map { 7: ()}), map { "duplicates": "combine" })
321+
return
322+
($specialWeek(7))
323+
};
324+
253325
declare
254326
%test:assertEmpty
255327
function mt:mapEmptyValue() {
@@ -904,3 +976,16 @@ function mt:immutable-merge-duplicates-then-merge() {
904976
$expected ne $result($mt:test-key-one)
905977
)
906978
};
979+
980+
(:~
981+
: ensure that empty options map is allowed and behaves like
982+
: map:merge#1
983+
:)
984+
declare
985+
%test:assertTrue
986+
function mt:map-merge-2-empty-options-map() {
987+
let $maps := (mt:getMapFixture(), map { "Su": "Sunnuntai" })
988+
let $expected := map:merge($maps)
989+
let $actual := map:merge($maps, map {})
990+
return $expected?Su eq $actual?Su
991+
};

0 commit comments

Comments
 (0)