icexelloss commented on code in PR #34311:
URL: https://github.com/apache/arrow/pull/34311#discussion_r1126894399


##########
cpp/src/arrow/compute/row/grouper.h:
##########
@@ -30,6 +30,49 @@
 namespace arrow {
 namespace compute {
 
+/// \brief A segment of contiguous rows for grouping
+struct ARROW_EXPORT GroupingSegment {
+  int64_t offset;
+  int64_t length;
+  bool is_open;
+  bool extends;
+};
+
+inline bool operator==(const GroupingSegment& segment1, const GroupingSegment& 
segment2) {
+  return segment1.offset == segment2.offset && segment1.length == 
segment2.length &&
+         segment1.is_open == segment2.is_open && segment1.extends == 
segment2.extends;
+}
+inline bool operator!=(const GroupingSegment& segment1, const GroupingSegment& 
segment2) {
+  return !(segment1 == segment2);
+}
+
+/// \brief Computes grouping segments for a batch. Each segment covers rows 
with identical
+/// values in the batch. The values in the batch are often selected as keys 
from a larger
+/// batch.
+class ARROW_EXPORT GroupingSegmenter {
+ public:
+  virtual ~GroupingSegmenter() = default;
+
+  /// \brief Construct a GroupingSegmenter which receives the specified key 
types
+  static Result<std::unique_ptr<GroupingSegmenter>> Make(
+      const std::vector<TypeHolder>& key_types, bool nullable_keys = false,
+      ExecContext* ctx = default_exec_context());

Review Comment:
   Oh I see - you are trying to create a "Make" interface for both 
`RowSegmenter` and `Grouper` class. IIUC, the common interface is only used in 
templating test since `RowSegmenter` and `Grouper` don't really share the same 
interface. Actually I found the templating of these two classes together quite 
confusing - all it is testing is the "Make" function. We might as well having 
them tested separately to avoid confusion.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to