Skip to content

Commit

Permalink
MONDRIAN-PACINO: Fixes a bug in ArraySortedSet.merge where the merge …
Browse files Browse the repository at this point in the history
…method wouldn't trim the array it returns to the actual number of values it holds. Also removes the hashCode() and equals() overrides. They were not needed.

Fixes a bug in SegmentCacheIndexImpl.matches. A faulty condition was causing false negatives.

Adds an optimization in SegmentHeader.canConstrain to return false if the excluded region is equal to the axis contents.

[git-p4: depot-paths = "//open/mondrian-release/pacino/": change = 14802]
  • Loading branch information
lucboudreau committed Nov 25, 2011
1 parent 32f9f93 commit d996c2e
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 31 deletions.
4 changes: 3 additions & 1 deletion src/main/mondrian/rolap/agg/CellRequest.java
Expand Up @@ -355,7 +355,9 @@ public Map<String, Comparable<?>> getMappedCellValues() {
final Object o = values[i];
map.put(
column.getExpression().getGenericExpression(),
(Comparable<?>) o);
o == RolapUtil.sqlNullValue
? null
: (Comparable<?>) o);
}
return map;
}
Expand Down
4 changes: 3 additions & 1 deletion src/main/mondrian/rolap/cache/SegmentCacheIndexImpl.java
Expand Up @@ -143,7 +143,9 @@ private boolean matches(
}
final SortedSet<Comparable<?>> values =
constrainedColumn.getValues();
if (values != null && values.contains(entry.getValue())) {
if (values != null
&& !values.contains(entry.getValue()))
{
return false;
}
}
Expand Down
11 changes: 8 additions & 3 deletions src/main/mondrian/spi/SegmentHeader.java
Expand Up @@ -215,7 +215,11 @@ public SegmentHeader clone(ConstrainedColumn[] overrideValues) {

/**
* Checks if this header can be constrained by a given region.
* It will return null if the region sits outside of the bounds
*
* <p>It will return false if the region covers one of the axis in
* its entirety.
*
* <p>It will return false if the region sits outside of the bounds
* of this header. This means that when performing a flush operation,
* the header must be scrapped altogether.
*/
Expand All @@ -225,10 +229,11 @@ public boolean canConstrain(ConstrainedColumn[] region) {
ConstrainedColumn ccActual =
getConstrainedColumn(ccToFlush.columnExpression);
if (ccActual != null) {
ConstrainedColumn ccActualExcl =
final ConstrainedColumn ccActualExcl =
getExcludedRegion(ccToFlush.columnExpression);
if (ccActualExcl != null
if ((ccActualExcl != null
&& ccActualExcl.merge(ccToFlush).values == null)
|| ccActual.values.equals(ccToFlush.values))
{
// This means that the whole axis is excluded.
// Better destroy that segment.
Expand Down
33 changes: 7 additions & 26 deletions src/main/mondrian/util/ArraySortedSet.java
Expand Up @@ -31,7 +31,6 @@ public class ArraySortedSet<E extends Comparable<E>>
private final E[] values;
private final int start;
private final int end;
private final int hashCode;

/**
* Creates a set backed by an array. The array must be sorted, and is
Expand All @@ -56,11 +55,6 @@ public ArraySortedSet(E[] values, int start, int end) {
this.values = values;
this.start = start;
this.end = end;
int hash = super.hashCode();
for (Object comp : this.toArray()) {
hash = Util.hash(hash, comp);
}
this.hashCode = hash;
}

public Iterator<E> iterator() {
Expand Down Expand Up @@ -180,15 +174,16 @@ public ArraySortedSet<E> merge(
return this;
}

int p1 = 0, p2 = 0, m = 0, k = this.size() + arrayToMerge.size();

final E[] data1 = this.values;
final E[] data2 = arrayToMerge.values;
final E[] merged =
E[] merged =
(E[])
Util.genericArray(
this.values[0].getClass(),
this.size() + arrayToMerge.size());
k);

int p1 = 0, p2 = 0, m = 0;

while (p1 < data1.length && p2 < data2.length) {
final int compare =
Expand All @@ -211,6 +206,9 @@ public ArraySortedSet<E> merge(
merged[m++] = data2[p2++];
}

if (m < k) {
merged = Arrays.copyOf(merged, m);
}
return new ArraySortedSet<E>(merged);
}

Expand All @@ -220,23 +218,6 @@ public boolean contains(Object o) {
return o != null
&& Util.binarySearch(values, start, end, (E) o) >= 0;
}

@Override
public boolean equals(Object o) {
if (!(o instanceof ArraySortedSet<?>)) {
return false;
}
// REVIEW: Should we not consider start and end boundaries in
// the equality check?
return Arrays.equals(
this.toArray(),
((ArraySortedSet<?>)o).toArray());
}

@Override
public int hashCode() {
return this.hashCode;
}
}

// End ArraySortedSet.java

0 comments on commit d996c2e

Please sign in to comment.