FoundationDB / fdb-record-layer

A record-oriented store built on FoundationDB
Apache License 2.0
570 stars 102 forks source link

Add more complete direction control to LogicalSortExpression. #2796

Closed MMcM closed 1 week ago

MMcM commented 2 weeks ago

Extends #2748 and #2771.

foundationdb-ci commented 2 weeks ago

Result of fdb-record-layer-pr on Linux CentOS 7

foundationdb-ci commented 2 weeks ago

Result of fdb-record-layer-pr on Linux CentOS 7

foundationdb-ci commented 2 weeks ago

Result of fdb-record-layer-pr on Linux CentOS 7

foundationdb-ci commented 1 week ago

Result of fdb-record-layer-pr on Linux CentOS 7

foundationdb-ci commented 1 week ago

Result of fdb-record-layer-pr on Linux CentOS 7

normen662 commented 1 week ago

Can you check that the right compatibilities are enforced (as well as the wrong ones don't match) in Ordering.combineBindingsForIntersection() and Ordering.combineBindingsForUnion()?

foundationdb-ci commented 1 week ago

Result of fdb-record-layer-pr on Linux CentOS 7

MMcM commented 1 week ago

Can you check that the right compatibilities are enforced (as well as the wrong ones don't match) in Ordering.combineBindingsForIntersection() and Ordering.combineBindingsForUnion()?

I believe that the new counterflow nulls enum members are still isDirectional, so that

        if (leftSortOrder.isDirectional() && rightSortOrder.isDirectional()) {
            if (leftSortOrder != rightSortOrder) {

takes the conservative approach. Also the directional + FIXED cases, where it takes the non-fixed side, seem still correct.

foundationdb-ci commented 1 week ago

Result of fdb-record-layer-pr on Linux CentOS 7

foundationdb-ci commented 1 week ago

Result of fdb-record-layer-pr on Linux CentOS 7