Skip to content

Commit

Permalink
Fix ORDER BY + LIMIT 0 in slotted runtime
Browse files Browse the repository at this point in the history
Handle limit <= 0 in the Top slotted pipe.

Fail explicitly on limit values larger than the maximum signed 32-bit
integer in the Top pipe.
  • Loading branch information
henriknyman committed Oct 11, 2018
1 parent d82c880 commit aa4ec13
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import java.util.Comparator
import org.neo4j.cypher.internal.DefaultComparatorTopTable
import org.neo4j.cypher.internal.runtime.interpreted.ExecutionContext
import org.neo4j.cypher.internal.runtime.interpreted.commands.expressions.Expression
import org.neo4j.cypher.internal.util.v3_4.CypherExecutionException
import org.neo4j.cypher.internal.util.v3_4.attribution.Id
import org.neo4j.values.storable.NumberValue

Expand All @@ -44,18 +45,26 @@ case class TopNPipe(source: Pipe, countExpression: Expression, comparator: Compa
if (input.isEmpty) Iterator.empty
else {
val first = input.next()
val count = countExpression(first, state).asInstanceOf[NumberValue].longValue().toInt
val topTable = new DefaultComparatorTopTable(comparator, count)
topTable.add(first)
val longCount: Long = countExpression(first, state).asInstanceOf[NumberValue].longValue()

input.foreach {
ctx =>
topTable.add(ctx)
}
if (longCount <= 0) {
Iterator.empty
} else if(longCount > Int.MaxValue) {
throw new CypherExecutionException(s"Top operator does not support limit $longCount > ${Int.MaxValue}", null)
} else {
val count = longCount.toInt
val topTable = new DefaultComparatorTopTable(comparator, count)
topTable.add(first)

topTable.sort()
input.foreach {
ctx =>
topTable.add(ctx)
}

topTable.iterator.asScala
topTable.sort()

topTable.iterator.asScala
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,3 +175,18 @@ Feature: SkipLimitAcceptance
| id |
| 1 |
And no side effects

Scenario: Top with limit 0
And having executed:
"""
CREATE (:A {id:0})
"""
When executing query:
"""
MATCH (n:A)
RETURN n.id AS id
ORDER BY id LIMIT 0
"""
Then the result should be, in order:
| id |
And no side effects
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,38 @@ class TopSlottedPipeTest extends CypherFunSuite {
result should equal(list(10, null))
}

test("should handle limit 0") {
val input = randomlyShuffledIntDataFromZeroUntil(5)
val result = singleColumnTopWithInput(
input, orderBy = AscendingOrder, limit = 0
)
result should equal(List.empty)
}

test("should handle negative limit") {
val input = randomlyShuffledIntDataFromZeroUntil(5)
val result = singleColumnTopWithInput(
input, orderBy = AscendingOrder, limit = -1
)
result should equal(List.empty)
}

test("should handle limit of Int.MaxValue") {
val input = randomlyShuffledIntDataFromZeroUntil(5)
val result = singleColumnTopWithInput(
input, orderBy = AscendingOrder, limit = Int.MaxValue
)
result should equal(list(0, 1, 2, 3, 4))
}

ignore("should handle limit larger than Int.MaxValue") {
val input = randomlyShuffledIntDataFromZeroUntil(5)
val result = singleColumnTopWithInput(
input, orderBy = AscendingOrder, limit = Int.MaxValue.asInstanceOf[Long] * 2
)
result should equal(list(0, 1, 2, 3, 4))
}

test("returning top 1 from 5 possible should return lowest") {
val result = singleColumnTopWithInput(
randomlyShuffledIntDataFromZeroUntil(5), orderBy = AscendingOrder, limit = 1
Expand Down Expand Up @@ -210,7 +242,7 @@ object TopSlottedPipeTestSupport {
data
}

private def createTopPipe(source: Pipe, orderBy: List[ColumnOrder], limit: Int, withTies: Boolean) = {
private def createTopPipe(source: Pipe, orderBy: List[ColumnOrder], limit: Long, withTies: Boolean) = {
val comparator = ExecutionContextOrdering.asComparator(orderBy)
if (withTies) {
assert(limit == 1)
Expand All @@ -224,7 +256,7 @@ object TopSlottedPipeTestSupport {
}
}

def singleColumnTopWithInput(data: Traversable[Any], orderBy: TestColumnOrder, limit: Int, withTies: Boolean = false): List[Any] = {
def singleColumnTopWithInput(data: Traversable[Any], orderBy: TestColumnOrder, limit: Long, withTies: Boolean = false): List[Any] = {
val slots = SlotConfiguration.empty
.newReference("a", nullable = true, CTAny)

Expand Down

0 comments on commit aa4ec13

Please sign in to comment.