Fix(rollups): Fix splits in roll-up #7609

[joshua-goldstein]

Cherry-pick-7609

The old PR fixed an issue that arose after posting/list.go was significantly refactored when roaring bitmaps were incorporated. The previous team first implemented roaring and then wrote the sroar library to compress the posting lists. In 21.03 we are not using roaring nor sroar so this code should not be merged. However a new test was written that we can bring in.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ joshua-goldstein
❌ ahsanbarkati
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[coveralls]

Coverage remained the same at 37.099% when pulling 2b1b8f5 on cherry-pick-7609 into 9b67017 on main.

[matthewmcneely]

@joshua-goldstein To recap, this PR only introduces a test, right? And that test exercises existing functionality with a "multi-part" list, right?

[meghalims]

Hi Josh,
I just read through your comment in detail and it make sense. We are just taking in the test

[joshua-goldstein]

@joshua-goldstein To recap, this PR only introduces a test, right? And that test exercises existing functionality with a "multi-part" list, right?

Correct, we are just introducing a test here. And yes, that is correct, there is some commentary here regarding what is happening during rollups. In particular they mention that the posting list will be split up into multiple lists if it gets too big.

[meghalims]

Looks good 👍

[skrdgraph]

I was hoping this test addition would increase the coverage %, but it's kind of odd to see the coverage % to stay the same. I did validate the same in the run logs of the test, and this test did run

=== RUN   TestLargePlistSplit
[8248](https://github.com/dgraph-io/dgraph/actions/runs/3079955654/jobs/4976760244#step:10:8249)
    list_test.go:1018: Num splits: 2
[8249](https://github.com/dgraph-io/dgraph/actions/runs/3079955654/jobs/4976760244#step:10:8250)
--- PASS: TestLargePlistSplit (1.73s)

fyi @kevinmingtarja

[joshua-goldstein]

I was hoping this test addition would increase the coverage %, but it's kind of odd to see the coverage % to stay the same. I did validate the same in the run logs of the test, and this test did run

=== RUN   TestLargePlistSplit
[8248](https://github.com/dgraph-io/dgraph/actions/runs/3079955654/jobs/4976760244#step:10:8249)
    list_test.go:1018: Num splits: 2
[8249](https://github.com/dgraph-io/dgraph/actions/runs/3079955654/jobs/4976760244#step:10:8250)
--- PASS: TestLargePlistSplit (1.73s)

fyi @kevinmingtarja

Hi @skrdgraph , posting lists are integrated deeply into Dgraph and there are already many tests covering them. There are many cases that need to be covered but if the methods are already tested elsewhere then checking additional cases wouldn't add to the test coverage metric, correct? Of course the issue here is that "test coverage" is a rough metric.

[skrdgraph]

I was hoping this test addition would increase the coverage %, but it's kind of odd to see the coverage % to stay the same. I did validate the same in the run logs of the test, and this test did run

=== RUN   TestLargePlistSplit
[8248](https://github.com/dgraph-io/dgraph/actions/runs/3079955654/jobs/4976760244#step:10:8249)
    list_test.go:1018: Num splits: 2
[8249](https://github.com/dgraph-io/dgraph/actions/runs/3079955654/jobs/4976760244#step:10:8250)
--- PASS: TestLargePlistSplit (1.73s)

fyi @kevinmingtarja

Hi @skrdgraph , posting lists are integrated deeply into Dgraph and there are already many tests covering them. There are many cases that need to be covered but if the methods are already tested elsewhere then checking additional cases wouldn't add to the test coverage metric, correct? Of course the issue here is that "test coverage" is a rough metric.

That makes sense, retesting a tested function/method - with another test - doesn't increase the number. I was under the assumption that this was a new test we were bringing in to test a component that was un-tested.

[skrdgraph]

This looks good 👍

We can merge this