TUR-134: MLIR: unwind a typed list of any value type into a typed column

docs/maps.md

@@ -116,8 +116,9 @@ out of the tag. An index takes a cell column the same way, and the result is the
 `nullable<any>` every index gives.
 
 A map held in a list reads through those same two paths, since `xs[0]` hands back a
-`!storage.any` whose `MapView` tag is the map. An `UNWIND` of such a list binds its
-variable to the same cell, so `UNWIND [{a: 1}] AS m RETURN m.a` reads through one too. The
+`!storage.any` whose `MapView` tag is the map. An `UNWIND` of a list of maps binds its
+variable to a map column instead, as for every list whose element type is known, so
+`UNWIND [{a: 1}] AS m RETURN m.a` reads a map; `UNWIND [{a: 1}, 2] AS m` binds cells. The
 dotted form `xs[0].a` needs a parser rule of its own (`propertyLookupExpr`), because a `.`
 after a `]` is otherwise no expression.
 

query/ir/common/IRLiteralList.h

@@ -11,10 +11,10 @@ namespace db {
 
 inline mlir::Type sharedLiteralElementType(mlir::ArrayAttr elements);
 
-// The type one literal list element carries: its own for a typed attribute, and the list
-// type its own verdict names for a nested list. A null names none, and an embedding (a
-// dense f32 array attr) or a map carries no type a list element is read under. A string
-// written in IR text parses with no type of its own, and is a string all the same.
+// The type one literal list element carries: its own for a typed attribute, the list type
+// its own verdict names for a nested list, a map for a dictionary and an embedding for a
+// dense f32 array. A null names none. A string written in IR text parses with no type of
+// its own, and is a string all the same.
 inline mlir::Type literalElementType(mlir::Attribute element) {
     mlir::MLIRContext* const context = element.getContext();
 
@@ -24,6 +24,10 @@ inline mlir::Type literalElementType(mlir::Attribute element) {
         return mlir::NoneType::get(context);
     } else if (mlir::isa<mlir::StringAttr>(element)) {
         return mlir::storage::StringType::get(context);
+    } else if (mlir::isa<mlir::DictionaryAttr>(element)) {
+        return mlir::storage::MapType::get(context);
+    } else if (mlir::isa<mlir::DenseF32ArrayAttr>(element)) {
+        return mlir::storage::EmbeddingType::get(context);
     }
 
     const auto typed = mlir::dyn_cast<mlir::TypedAttr>(element);

query/ir/common/IRValueTypes.cpp

@@ -3,34 +3,14 @@
 #include "mlir/IR/BuiltinAttributes.h"
 #include "mlir/IR/Builders.h"
 
+#include "NullLiteralType.h"
 #include "StorageEnums.h"
 #include "StorageTypes.h"
 
 #include "IRException.h"
 
 using namespace db;
 
-namespace {
-
-// The list element types an unwind can drain into a column of that very type: the entity
-// IDs, the value types a nullable value chunk is laid out for, the schema IDs, and a nested
-// list, which drains into a list column one level shallower. An unresolved element, an
-// embedding, or the !storage.any a heterogeneous list holds drains as tagged scalars instead -
-// none of them names a column shape the drain could fill.
-bool drainsToItsOwnElementType(mlir::Type listElement) {
-    if (mlir::isa<mlir::storage::NodeIDType, mlir::storage::EdgeIDType, mlir::storage::StringType, mlir::storage::ListType>(listElement)) {
-        return true;
-    } else if (mlir::isa<mlir::Float64Type>(listElement) || mlir::storage::namesASchemaID(listElement)) {
-        return true;
-    }
-
-    const auto intType = mlir::dyn_cast<mlir::IntegerType>(listElement);
-
-    return intType && (intType.getWidth() == 1 || intType.getWidth() == 64);
-}
-
-}
-
 mlir::Type db::valueTypeToElementType(mlir::OpBuilder& builder, ValueType valueType) {
     switch (valueType) {
         case ValueType::Int64:
@@ -180,21 +160,17 @@ mlir::Type db::unwoundElementType(mlir::MLIRContext* context, mlir::Type sourceE
         return sourceElement;
     }
 
-    // The elements of a list whose type is known are that type, so the unwind hands the
-    // rest of the query a column it can read as one - a node stays a node, an integer an
-    // integer. Only a list whose elements share no such type drains into the type-erased
-    // column of tagged scalars.
+    // A column that spells a null as a value of its own rides a plain chunk, and one of a
+    // property value type or a schema ID the nullable chunk every value-chunk consumer
+    // reads. Any other list, one of mixed elements or of nulls alone, drains into tagged
+    // scalars.
     const mlir::Type listElement = listType.getElementType();
-    if (!drainsToItsOwnElementType(listElement)) {
-        return mlir::storage::AnyType::get(context);
-    }
-
-    // An entity ID column spells a null entity as an invalid ID, so an entity rides a
-    // plain chunk; a value or a nested list, either of which may be a tagged null, rides
-    // the nullable one every value-chunk consumer dispatches on, as lowerUnwindConst's
-    // homogeneous list does.
-    if (mlir::isa<mlir::storage::NodeIDType, mlir::storage::EdgeIDType>(listElement)) {
+    const bool drainsToANullableChunk = elementTypeToValueType(listElement) != ValueType::Invalid
+                                     || mlir::storage::namesASchemaID(listElement);
+    if (mlir::holdsItsOwnNull(listElement)) {
         return listElement;
+    } else if (!drainsToANullableChunk) {
+        return mlir::storage::AnyType::get(context);
     }
 
     return mlir::storage::NullableType::get(context, listElement);

query/ir/dialect/db/DBOps.td

@@ -364,11 +364,13 @@ def Unwind : TuringOp<"unwind", [Pure, DeclareOpInterfaceMethods<ColumnCarrierOp
     row-aligned with the elements. A constant source stands for every carried row, so it
     expands each of them.
 
-    The element column's type is the source's to decide and is resolved during
-    lowering, as a property fetch's value column is: unwinding a list yields a
-    type-erased !storage.any column of tagged scalars, matching Cypher's LIST
-    OF ANY, while a source that is already a scalar column keeps its own type - the
-    singleton is the value it already holds.
+    The element column's type is the source's to decide (`unwoundElementType`):
+    unwinding a list of IDs or paths yields a column of them, a list of values a
+    nullable column of their type, and a list whose elements share no type a
+    type-erased !storage.any column of tagged scalars, matching Cypher's LIST OF ANY,
+    as does a list of nulls alone.
+    A source that is already a scalar column keeps its own type - the singleton is the
+    value it already holds.
   }];
 
   let arguments = (ins Column:$source, Variadic<Column>:$columns_to_filter);

query/ir/dialect/nl/NLOps.td

@@ -209,18 +209,20 @@ def Unwind : NLOp<"unwind", [Pure]> {
 
       %rows = nl.unwind(%names, {%nodes})
                 : (!nl.chunk<!storage.list<!storage.string>>, !nl.chunk<!storage.node_id>)
-                -> !nl.iter<!nl.chunk<!storage.any>, !nl.chunk<!storage.node_id>>
-      nl.for %x, %n in %rows : !nl.iter<!nl.chunk<!storage.any>, !nl.chunk<!storage.node_id>> {
-        nl.output(%x, %n) : !nl.chunk<!storage.any>, !nl.chunk<!storage.node_id>
+                -> !nl.iter<!nl.chunk<!storage.nullable<!storage.string>>, !nl.chunk<!storage.node_id>>
+      nl.for %x, %n in %rows : !nl.iter<!nl.chunk<!storage.nullable<!storage.string>>, !nl.chunk<!storage.node_id>> {
+        nl.output(%x, %n) : !nl.chunk<!storage.nullable<!storage.string>>, !nl.chunk<!storage.node_id>
       }
 
     A list cell contributes one row per element, a null cell none, and any other cell
     the single row it is - a tagged scalar included, so a !storage.any cell that is
     itself a list unwinds one level deeper. Unlike nl.unwind_const it reads an input
     chunk, so it nests in the loop that binds it, the way nl.get_out_edges nests in the
-    loop binding its nodes. The element chunk type follows the source's - a list unwinds
-    into the type-erased !storage.any chunk, a scalar column into its own - and, like
-    nl.collect's, is spelled out rather than inferred.
+    loop binding its nodes. The element chunk type follows the source's
+    (`unwoundElementType`) - a list of IDs or paths unwinds into a chunk of them, a list
+    of values into a nullable chunk of their type, any other list into the type-erased
+    !storage.any chunk, a scalar column into its own - and, like nl.collect's, is
+    spelled out rather than inferred.
   }];
 
   // `columns_to_filter`: the variadic carry set, after the fixed source, mirroring

query/ir/exec/NLExecutor.cpp

@@ -2697,6 +2697,34 @@ int compareContainerColumn(const Column* column, size_t a, size_t b) {
     return 0;
 }
 
+// 3-way compare two rows of a path column. The empty path a null is spelled as sorts after
+// every path, as compareOptColumn places a null after every value.
+int comparePathColumn(const Column* column, size_t a, size_t b) {
+    const std::vector<GraphPath>& raw = static_cast<const ColumnVector<GraphPath>*>(column)->getRaw();
+    const GraphPath& pathA = raw[a];
+    const GraphPath& pathB = raw[b];
+
+    const bool aNull = pathA.isNull();
+    const bool bNull = pathB.isNull();
+    if (aNull || bNull) {
+        if (aNull && bNull) {
+            return 0;
+        }
+
+        return aNull ? 1 : -1;
+    }
+
+    const std::strong_ordering order = PathView {pathA.getIDs()} <=> PathView {pathB.getIDs()};
+
+    if (order == std::strong_ordering::less) {
+        return -1;
+    } else if (order == std::strong_ordering::greater) {
+        return 1;
+    }
+
+    return 0;
+}
+
 // The nullable sibling, for a list or a map read out of a property: a row holding none
 // sorts after every value, as compareOptColumn places a null after every value.
 template <typename View>
@@ -14236,7 +14264,7 @@ NLCompareFunction NLExecutor::selectCompareFunction(NLChunkKind kind) {
         break;
 
         case NLChunkKind::Path:
-            throw IRException("A path column cannot be a sort key: a path has no order here");
+            return &comparePathColumn;
         break;
 
         case NLChunkKind::DateTime:

storage/CellOrder.cpp

@@ -228,12 +228,8 @@ std::strong_ordering compareValues(const Lhs lhs, const Rhs rhs) {
             return valueOf<ListView>(lhs) <=> valueOf<ListView>(rhs);
         break;
 
-        case CellOrderClass::Path: {
-            const std::span<const uint64_t> lhsIDs = valueOf<PathView>(lhs).getIDs();
-            const std::span<const uint64_t> rhsIDs = valueOf<PathView>(rhs).getIDs();
-
-            return std::lexicographical_compare_three_way(lhsIDs.begin(), lhsIDs.end(), rhsIDs.begin(), rhsIDs.end());
-        }
+        case CellOrderClass::Path:
+            return valueOf<PathView>(lhs) <=> valueOf<PathView>(rhs);
         break;
 
         case CellOrderClass::Point:
@@ -474,6 +470,13 @@ std::strong_ordering db::operator<=>(const MapView lhs, const MapView rhs) {
     return std::strong_ordering::equal;
 }
 
+std::strong_ordering db::operator<=>(const PathView lhs, const PathView rhs) {
+    const std::span<const uint64_t> lhsIDs = lhs.getIDs();
+    const std::span<const uint64_t> rhsIDs = rhs.getIDs();
+
+    return std::lexicographical_compare_three_way(lhsIDs.begin(), lhsIDs.end(), rhsIDs.begin(), rhsIDs.end());
+}
+
 bool db::operator==(const MapView lhs, const MapView rhs) {
     const std::span<const MapEntryView> lhsEntries = lhs.entries();
     const std::span<const MapEntryView> rhsEntries = rhs.entries();

storage/CellOrder.h

@@ -49,6 +49,12 @@ std::strong_ordering operator<=>(ListView lhs, ListView rhs);
  */
 std::strong_ordering operator<=>(MapView lhs, MapView rhs);
 
+/**
+ * @brief Orders two paths by their IDs in path order, so a path sorts before every longer
+ * path it starts.
+ */
+std::strong_ordering operator<=>(PathView lhs, PathView rhs);
+
 /**
  * @brief Cypher's equality of two cells, lists or maps, at any depth: unknown where a null
  * meets a value and no other pair differs. == is equivalence instead, under which two nulls

storage/columns/Functions.h

@@ -477,10 +477,10 @@ class MapKeysFunction {
     std::vector<QueryListBuffer::ListItemVariant> _elements;
 };
 
-// The field of an instant a type-erased cell carries, which is what an UNWIND of a list of
-// instants hands each row. A cell holding a null answers null, as every function over a
-// cell does; one holding no instant is a type error only the row it is in can find out
-// about, so it throws.
+// The field of an instant a type-erased cell carries, as a map value or an element of a
+// mixed list does. A cell holding a null answers null, as every function over a cell does;
+// one holding no instant is a type error only the row it is in can find out about, so it
+// throws.
 template <DateTimePart part>
 class TaggedDateTimeComponentFunction {
 public:

test/query/ir/map/ListOfMapsTest.cpp

@@ -170,6 +170,11 @@ std::string renderCell(const Column* column, size_t row) {
         return list.has_value() ? renderList(*list) : "null";
     }
 
+    if (const auto* optMaps = dynamic_cast<const ColumnOptVector<MapView>*>(column)) {
+        const std::optional<MapView>& map = (*optMaps)[row];
+        return map.has_value() ? renderMap(*map) : "null";
+    }
+
     throw TuringException("ListOfMapsTest: unsupported output column type");
 }
 
@@ -193,7 +198,7 @@ class CollectingListSink : public NLOutputSink {
 
 // A list may hold a map, the way a map already holds a list. The map rides the list as the
 // one tagged cell it is, so it survives nesting, neighbours of other types and an UNWIND
-// that spreads the list back out.
+// that spreads the list back out into a map column, or into cells beside other types.
 class ListOfMapsTest : public TuringTest {
 protected:
     void initialize() override {
@@ -286,8 +291,6 @@ TEST_F(ListOfMapsTest, holdsAnEmptyMap) {
     expectRows("RETURN [{}]", {"[{}]"});
 }
 
-// An UNWIND spreads the list back out, so each map comes back as the one tagged cell it
-// was stored as
 TEST_F(ListOfMapsTest, unwindsToItsMaps) {
     expectRows("UNWIND [{a: 1}, {b: 2}] AS x RETURN x", {"{a: 1}", "{b: 2}"});
 }

test/query/ir/path/CMakeLists.txt

@@ -186,3 +186,5 @@ add_call_v3_test(test_query_ir_walk_read_past_a_scope WalkReadPastAScopeTest.cpp
 add_ir_tests(test_query_ir_path_in_case PathInCaseTest.cpp)
 
 add_ir_tests(test_query_ir_path_under_two_names PathUnderTwoNamesTest.cpp)
+
+add_call_v3_test(test_query_ir_path_sort_key PathSortKeyTest.cpp)

test/query/ir/path/PathSortKeyTest.cpp

@@ -0,0 +1,39 @@
+#include <gtest/gtest.h>
+
+#include <string>
+#include <vector>
+
+#include "CallV3Test.h"
+#include "StringRowSink.h"
+
+using namespace turing::test;
+
+namespace {
+
+using Rows = std::vector<StringRowSink::Row>;
+
+}
+
+// simpledb's KNOWS_WELL edges are Remy (0) -> Adam (1) and Adam -> Remy, so a path from Remy
+// sorts before the one from Adam.
+class PathSortKeyTest : public CallV3Test {
+protected:
+    void expectRows(const std::string& query, const Rows& expected) {
+        StringRowSink sink;
+        runQuery(query, sink);
+
+        EXPECT_EQ(sink.getRows(), expected) << query;
+    }
+};
+
+TEST_F(PathSortKeyTest, ordersMatchedPaths) {
+    expectRows("MATCH p = (a:Person)-[:KNOWS_WELL]->(b:Person) RETURN a.name ORDER BY p",
+               {{"Remy"}, {"Adam"}});
+    expectRows("MATCH p = (a:Person)-[:KNOWS_WELL]->(b:Person) RETURN a.name ORDER BY p DESC",
+               {{"Adam"}, {"Remy"}});
+}
+
+TEST_F(PathSortKeyTest, ordersAShorterPathBeforeTheLongerOneItStarts) {
+    expectRows("MATCH p = (a:Person {name: 'Remy'})-[:KNOWS_WELL*1..2]->(b) RETURN length(p) ORDER BY p",
+               {{"1"}, {"2"}});
+}

test/query/ir/temporal/DateTimeComponentCellTest.cpp

@@ -24,9 +24,8 @@
 using namespace db;
 using namespace turing::test;
 
-// An UNWIND hands its elements on as type-erased cells, which carry their type per row
-// rather than per column. A calendar field read off one has to read the instant out of the
-// cell, as size() and id() read a list and an entity out of one.
+// A calendar field read off an unwound instant, and off a type-erased cell, which carries
+// its type per row rather than per column: a map value or a list element.
 class DateTimeComponentCellTest : public TuringTest {
 protected:
     void initialize() override {
@@ -143,3 +142,9 @@ TEST_F(DateTimeComponentCellTest, filtersOnAFieldOfAnUnwoundInstant) {
                "WITH d WHERE d.year > 2020 RETURN d.month",
                {{"1"}});
 }
+
+TEST_F(DateTimeComponentCellTest, readsAFieldOfAnInstantHeldInACell) {
+    expectRows("WITH {a: datetime('2026-01-15T00:00:00Z')} AS m, [datetime('2019-03-02T00:00:00Z'), 1] AS l "
+               "RETURN m.a.year, l[0].month",
+               {{"2026", "3"}});
+}

test/query/ir/temporal/DurationComponentCellTest.cpp

@@ -24,7 +24,7 @@
 using namespace db;
 using namespace turing::test;
 
-// A unit read off a duration an UNWIND handed on as a type-erased cell.
+// A unit read off an unwound duration, and off a duration a type-erased cell holds.
 class DurationComponentCellTest : public TuringTest {
 protected:
     void initialize() override {
@@ -145,3 +145,9 @@ TEST_F(DurationComponentCellTest, readsARemainderOfAnUnwoundDuration) {
                "RETURN d.minutesOfHour, d.secondsOfMinute",
                {{"1", "1"}, {"-1", "-30"}, {"null", "null"}});
 }
+
+TEST_F(DurationComponentCellTest, readsAUnitOfADurationHeldInACell) {
+    expectRows("WITH {a: duration(90061000000)} AS m, [duration(2000000), 'x'] AS l "
+               "RETURN m.a.hours, l[0].seconds",
+               {{"25", "2"}});
+}

test/query/ir/unwind/CMakeLists.txt

@@ -62,3 +62,5 @@ add_call_v3_test(test_query_ir_unwind_computed UnwindComputedTest.cpp)
 
 add_turing_test(test_query_ir_unwind_computed_codegen UnwindComputedCodegenTest.cpp)
 target_include_directories(test_query_ir_unwind_computed_codegen PRIVATE ${IR_TEST_DIR})
+
+add_call_v3_test(test_query_ir_unwind_typed_list UnwindTypedListTest.cpp)