Skip to content

Conversation

@ijihang
Copy link
Contributor

@ijihang ijihang commented Jun 25, 2021

jira:https://issues.apache.org/jira/projects/IOTDB/issues/IOTDB-1453?filter=allissues
Test Query statement is:select * from root.ln.wf01.wt01 where time <5 and time > 10
cluster version result set is:
image
server version result set is:
image

I think the cluster result set is true.

@jixuan1989
Copy link
Member

jixuan1989 commented Jun 25, 2021

Hi, IMO, the server version is better... considering you are using a relational database, Oracle or MySQL, and query using the SQL: select * from table where id >5 and id <3, will you get an exception?

But, we indeed can optimize the execution logic, to return empty resultset ASAP.

@coveralls
Copy link

coveralls commented Jun 25, 2021

Coverage Status

Coverage decreased (-0.9%) to 67.162% when pulling 681e4c4 on ijihang:fixSingleError into 7ac39ef on apache:master.

@ijihang
Copy link
Contributor Author

ijihang commented Jun 25, 2021

exactly,if Oracle or MySQL ,the return is empty,now I tried to return the result set to null

Comment on lines +119 to +341
public static Intervals extractTimeInterval(Filter filter) {
if (filter == null) {
return Intervals.ALL_INTERVAL;
}
// and, or, not, value, time, group by
// eq, neq, gt, gteq, lt, lteq, in
if (filter instanceof AndFilter) {
AndFilter andFilter = ((AndFilter) filter);
Intervals leftIntervals = extractTimeInterval(andFilter.getLeft());
Intervals rightIntervals = extractTimeInterval(andFilter.getRight());
return leftIntervals.intersection(rightIntervals);
} else if (filter instanceof OrFilter) {
OrFilter orFilter = ((OrFilter) filter);
Intervals leftIntervals = extractTimeInterval(orFilter.getLeft());
Intervals rightIntervals = extractTimeInterval(orFilter.getRight());
return leftIntervals.union(rightIntervals);
} else if (filter instanceof NotFilter) {
NotFilter notFilter = ((NotFilter) filter);
return extractTimeInterval(notFilter.getFilter()).not();
} else if (filter instanceof TimeFilter.TimeGt) {
TimeFilter.TimeGt timeGt = ((TimeFilter.TimeGt) filter);
return new Intervals(((long) timeGt.getValue()) + 1, Long.MAX_VALUE);
} else if (filter instanceof TimeFilter.TimeGtEq) {
TimeFilter.TimeGtEq timeGtEq = ((TimeFilter.TimeGtEq) filter);
return new Intervals(((long) timeGtEq.getValue()), Long.MAX_VALUE);
} else if (filter instanceof TimeFilter.TimeEq) {
TimeFilter.TimeEq timeEq = ((TimeFilter.TimeEq) filter);
return new Intervals(((long) timeEq.getValue()), ((long) timeEq.getValue()));
} else if (filter instanceof TimeFilter.TimeNotEq) {
TimeFilter.TimeNotEq timeNotEq = ((TimeFilter.TimeNotEq) filter);
Intervals intervals = new Intervals();
intervals.addInterval(Long.MIN_VALUE, (long) timeNotEq.getValue() - 1);
intervals.addInterval((long) timeNotEq.getValue() + 1, Long.MAX_VALUE);
return intervals;
} else if (filter instanceof TimeFilter.TimeLt) {
TimeFilter.TimeLt timeLt = ((TimeFilter.TimeLt) filter);
return new Intervals(Long.MIN_VALUE, (long) timeLt.getValue() - 1);
} else if (filter instanceof TimeFilter.TimeLtEq) {
TimeFilter.TimeLtEq timeLtEq = ((TimeFilter.TimeLtEq) filter);
return new Intervals(Long.MIN_VALUE, (long) timeLtEq.getValue());
} else if (filter instanceof TimeFilter.TimeIn) {
TimeFilter.TimeIn timeIn = ((TimeFilter.TimeIn) filter);
Intervals intervals = new Intervals();
for (Object value : timeIn.getValues()) {
long time = ((long) value);
intervals.addInterval(time, time);
}
return intervals;
} else if (filter instanceof GroupByFilter) {
GroupByFilter groupByFilter = ((GroupByFilter) filter);
return new Intervals(groupByFilter.getStartTime(), groupByFilter.getEndTime() + 1);
}
// value filter
return Intervals.ALL_INTERVAL;
}

/** All intervals are closed. */
public static class Intervals extends ArrayList<Long> {

static final Intervals ALL_INTERVAL = new Intervals(Long.MIN_VALUE, Long.MAX_VALUE);

public Intervals() {
super();
}

Intervals(long lowerBound, long upperBound) {
super();
addInterval(lowerBound, upperBound);
}

public int getIntervalSize() {
return size() / 2;
}

public long getLowerBound(int index) {
return get(index * 2);
}

public long getUpperBound(int index) {
return get(index * 2 + 1);
}

void setLowerBound(int index, long lb) {
set(index * 2, lb);
}

void setUpperBound(int index, long ub) {
set(index * 2 + 1, ub);
}

public void addInterval(long lowerBound, long upperBound) {
add(lowerBound);
add(upperBound);
}

Intervals intersection(Intervals that) {
Intervals result = new Intervals();
int thisSize = this.getIntervalSize();
int thatSize = that.getIntervalSize();
for (int i = 0; i < thisSize; i++) {
for (int j = 0; j < thatSize; j++) {
long thisLB = this.getLowerBound(i);
long thisUB = this.getUpperBound(i);
long thatLB = that.getLowerBound(i);
long thatUB = that.getUpperBound(i);
if (thisUB >= thatLB) {
if (thisUB <= thatUB) {
result.addInterval(Math.max(thisLB, thatLB), thisUB);
} else if (thisLB <= thatUB) {
result.addInterval(Math.max(thisLB, thatLB), thatUB);
}
}
}
}
return result;
}

/**
* The union is implemented by merge, so the two intervals must be ordered.
*
* @param that
* @return
*/
Intervals union(Intervals that) {
if (this.isEmpty()) {
return that;
} else if (that.isEmpty()) {
return this;
}
Intervals result = new Intervals();

int thisSize = this.getIntervalSize();
int thatSize = that.getIntervalSize();
int thisIndex = 0;
int thatIndex = 0;
// merge the heads of the two intervals
while (thisIndex < thisSize && thatIndex < thatSize) {
long thisLB = this.getLowerBound(thisIndex);
long thisUB = this.getUpperBound(thisIndex);
long thatLB = that.getLowerBound(thatIndex);
long thatUB = that.getUpperBound(thatIndex);
if (thisLB <= thatLB) {
result.mergeLast(thisLB, thisUB);
thisIndex++;
} else {
result.mergeLast(thatLB, thatUB);
thatIndex++;
}
}
// merge the remaining intervals
Intervals remainingIntervals = thisIndex < thisSize ? this : that;
int remainingIndex = thisIndex < thisSize ? thisIndex : thatIndex;
mergeRemainingIntervals(remainingIndex, remainingIntervals, result);

return result;
}

private void mergeRemainingIntervals(
int remainingIndex, Intervals remainingIntervals, Intervals result) {
for (int i = remainingIndex; i < remainingIntervals.getIntervalSize(); i++) {
long lb = remainingIntervals.getLowerBound(i);
long ub = remainingIntervals.getUpperBound(i);
result.mergeLast(lb, ub);
}
}

/**
* Merge an interval of [lowerBound, upperBound] with the last interval if they can be merged,
* or just add it as the last interval if its lowerBound is larger than the upperBound of the
* last interval. If the upperBound of the new interval is less than the lowerBound of the last
* interval, nothing will be done.
*
* @param lowerBound
* @param upperBound
*/
private void mergeLast(long lowerBound, long upperBound) {
if (getIntervalSize() == 0) {
addInterval(lowerBound, upperBound);
return;
}
int lastIndex = getIntervalSize() - 1;
long lastLB = getLowerBound(lastIndex);
long lastUB = getUpperBound(lastIndex);
if (lowerBound > lastUB + 1) {
// e.g., last [3, 5], new [7, 10], just add the new interval
addInterval(lowerBound, upperBound);
return;
}
if (upperBound < lastLB - 1) {
// e.g., last [7, 10], new [3, 5], do nothing
return;
}
// merge the new interval into the last one
setLowerBound(lastIndex, Math.min(lastLB, lowerBound));
setUpperBound(lastIndex, Math.max(lastUB, upperBound));
}

public Intervals not() {
if (isEmpty()) {
return ALL_INTERVAL;
}
Intervals result = new Intervals();
long firstLB = getLowerBound(0);
if (firstLB != Long.MIN_VALUE) {
result.addInterval(Long.MIN_VALUE, firstLB - 1);
}

int intervalSize = getIntervalSize();
for (int i = 0; i < intervalSize - 1; i++) {
long currentUB = getUpperBound(i);
long nextLB = getLowerBound(i + 1);
if (currentUB + 1 <= nextLB - 1) {
result.addInterval(currentUB + 1, nextLB - 1);
}
}

long lastUB = getUpperBound(result.getIntervalSize() - 1);
if (lastUB != Long.MAX_VALUE) {
result.addInterval(lastUB + 1, Long.MAX_VALUE);
}
return result;
}
}
Copy link
Member

@neuyilan neuyilan Jun 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is those code duplicated with the org.apache.iotdb.cluster.utils.PartitionUtils.Intervals ? if yes, why repeat coding in two places?
Don't repeat yourself.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll make some improvements

Copy link
Member

@neuyilan neuyilan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@mychaow mychaow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. And please cherry-pick to 0.12

@mychaow mychaow merged commit 6f67a81 into apache:master Jul 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants