Starting on Window Functions #13458

imply-cheddar · 2022-11-30T00:20:31Z

Description

This commit adds support for Window functions to Druid. The intent is to support window functions similarly to what postgresql does (https://www.postgresql.org/docs/current/functions-window.html) as well as Drill (https://drill.apache.org/docs/sql-window-functions-introduction/)

The commit is large and there is more work to do, but this lays the foundation of supporting window functions. This PR is best read from a few entry points:

The windowing functions are implemented as Processors. Looking at this interface will give an idea of how the actual window functions are implemented. The biggest notion here is that Processors deal with a RowsAndColumns object that represents a full partition. Generally speaking, they believe that something else has prepared the RowsAndColumns for them which proves to be a nice simplifying assumption for their implementation.
RowsAndColumns is an interface that represents a set of... rows and columns. If you look at the interface, it is rather minimalist. The idea here is to lean into the idea of .as() as has existed on Segments for a long time. We use it to effectively build a "menu" of common, generic functionality that can be done on a batch of data and then Operators/Processors can be written in terms of that common, generic functionality. The implementation in this PR provides naive implementations of this functionality, as we get deeper in future PRs, this functionality will get fleshed out and specialized more to avoid object copies and megamorphism while further offering love for vectorized processing.
Operator is an interface introduced here. WindowOperatorQuery is an operator-defined query for running Window operations on top of the results of a sub-query. All of the logic for the operators is handled in WindowOperatorQueryQueryToolChest.merge right now. Essentially, this PR has been co-opted to get Operators introduced into the Druid code base. This means we are using window queries as an initial jumping off point for Operators to be introduced into the code flow. We expect more and more iteration on this to expand the capabilities deeper and perhaps make a future world where Operator-only queries are a thing that Druid supports.
CalciteWindowQueryTest leverages the recent changes that brought us the QueryTestBuilder to have fully file-driven tests. For the window functions, we needed more data, so I've moved the wikipedia dataset that was checked in as part of environment to be in the resources of the test jar. We then index that and reference it in the tests. The tests sit in calcite/tests/window of resources, each file is a SQL query, the window Operator structure that we expect to be built from it and the expected results. Hopefully, this will simplify the addition of test cases making it easier for someone to add a test without necessarily knowing how to work with the code and fix it.

That said, this PR introduces various things, but it is not complete yet. This is a first step and there are still sharp edges/unimplemented functionality. That said, what exists here does work for a subset of use cases and is a meaningful milestone that can be committed while we iterate on fleshing out and finishing up the functionality. As such, I'd like to get this reviewed and committed before making it even larger. All interfaces introduced in this PR are experimental and the "windowOperator" query is also intended to be experimental. Given that this is still experimental, I am intending to merge this PR as an undocumented feature, which we will document better as we get the sharp edges resolved.

Sharp Edges

The window function support does not yet support "frames" (ROWS BETWEEN 2 PRECEEDING AND 2 FOLLOWING style clauses).
The support is not yet fully aware of the difference between RANGE and ROWS when evaluating peers. (The built-in functions are all implemented with the correctly defined semantics, similar to what the postgresql document says above)
All window functions in one query must use the same windowing definition (the code cannot currently support 2 different PARTITION BY X clauses)
The windowing logic will not re-sort the data. It assumes that the sub-query was written such that data is pre-sorted in the way that the windowing logic expects.

These sharp edges are in the weeds enough that this support should still be considered experimental and it should exist as an undocumented feature. Subsequent PRs will smooth out these sharp edges.

Release note

None as the intent is for this to be an undocumented, experimental addition.

This PR has:

been self-reviewed.
added documentation for new or modified features or behaviors.
added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.

This is an initial take on how to use Processors for Window Processing. A Processor is an interface that transforms RowsAndColumns objects. RowsAndColumns objects are essentially combinations of rows and columns. The intention is that these Processors are the start of a set of operators that more closely resemble what DB engineers would be accustomed to seeing.

can run them end-to-end. This code can be used to actually run a query, so yay!

…ql-window

This is an initial take on how to use Processors for Window Processing. A Processor is an interface that transforms RowsAndColumns objects. RowsAndColumns objects are essentially combinations of rows and columns. The intention is that these Processors are the start of a set of operators that more closely resemble what DB engineers would be accustomed to seeing.

can run them end-to-end. This code can be used to actually run a query, so yay!

This introduces the SQL-bindings for the parsing and planning of window queries. There are limitations like only a single partitioning can be planned and there is a baked in assumption that the data is pre-sorted. Both of these are pretty sharp edges, so the functionality is still not ready for general consumption, but this is a start that works.

Added wikipedia data to the indexes available to the SQL queries and tests validating the windowing functionality as it exists now.

lgtm-com · 2022-11-30T01:45:21Z

This pull request introduces 2 alerts and fixes 1 when merging 851acb8 into 50963ed - view on LGTM.com

new alerts:

1 for Dereferenced variable may be null
1 for Boxed variable is never null

fixed alerts:

1 for Contradictory type checks

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

vogievetsky · 2022-11-30T18:41:39Z

sql/src/test/resources/calcite/tests/window/wikipediaSimplePartition.sqlTest

@@ -0,0 +1,1019 @@
+type: "operatorValidation"
+
+sql: "


Try sql: | for sweet multi line strings in yaml!

lgtm-com · 2022-12-02T08:22:00Z

This pull request introduces 2 alerts and fixes 1 when merging 299d735 into 138a6de - view on LGTM.com

new alerts:

1 for Dereferenced variable may be null
1 for Boxed variable is never null

fixed alerts:

1 for Contradictory type checks

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

lgtm-com · 2022-12-05T07:01:29Z

This pull request introduces 2 alerts and fixes 1 when merging 935864c into 9177419 - view on LGTM.com

new alerts:

1 for Dereferenced variable may be null
1 for Boxed variable is never null

fixed alerts:

1 for Contradictory type checks

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

gianm

I'm looking forward to seeing where we can take this, and especially to seeing a general OperatorQuery & integrating with frames and MSQ. It looks like there are pathways to do that in the future, so that's good. It's also good that the SQL functionality is off by default and not documented, since people might otherwise stumble into it when trying to run window functions. Having it be obscure in that way will help us evolve it quickly.

Let's get this out there so we can start testing and evolving it. I left a few relatively minor line comments asking for various clarifications and also asking for the SQL planning stuff to be even more switched-off-by-default than it currently is. With those changes it would look good to me.

gianm · 2022-12-05T06:06:41Z

processing/src/main/java/org/apache/druid/query/rowsandcols/column/ColumnAccessor.java

+   * Get the number of cells
+   * @return the number of cells
+   */
+  int numCells();


I get why this is called a cell, but it would make more sense to me if it was called a row.

gianm · 2022-12-05T06:14:00Z

processing/src/main/java/org/apache/druid/query/operator/WindowOperatorQuery.java

+import java.util.Map;
+import java.util.Objects;
+
+public class WindowOperatorQuery extends BaseQuery<RowsAndColumns>


Is this meant to be supplanted by an operator query at some point? Would both live side by side?

I hope that eventually we have a "pure" OperatorQuery that we can just switch this to extend and then "be done". I'm keeping it named WindowOperator for now to make it easier to create a new OperatorQuery if it turns out that this is done wrong.

gianm · 2022-12-05T06:15:24Z

processing/src/main/java/org/apache/druid/query/operator/WindowOperatorQuery.java

+  @JsonCreator
+  public WindowOperatorQuery(
+      @JsonProperty("dataSource") DataSource dataSource,
+      @JsonProperty("intervals") QuerySegmentSpec querySegmentSpec,


Is it too wild to consider combining DataSource and QuerySegmentSpec as part of this effort? Once I really internalized what they each mean it feels like it's weird that they're separate.

I am reminded that the equivalent MSQ concept here is InputSpec, where the TableInputSpec combines datasource, intervals, and filter. Wonder if it would make sense to use that here.

I'm sympathetic to your ask, but I think it's a bit too much for this PR at this point in time. That said, I think I can remove intervals entirely from this query spec...

gianm · 2022-12-05T06:27:39Z

processing/src/main/java/org/apache/druid/query/operator/WindowOperatorQuery.java

+import java.util.Map;
+import java.util.Objects;
+
+public class WindowOperatorQuery extends BaseQuery<RowsAndColumns>


What are the preconditions for the data from the dataSource of this query? Would make good javadoc.

gianm · 2022-12-05T06:28:56Z

processing/src/main/java/org/apache/druid/query/operator/NaivePartitioningOperator.java

+ * This naive partitioning operator assumes that it's child operator always gives it RowsAndColumns objects that are
+ * a superset of the partitions that it needs to provide.  It will never attempt to make a partition larger than a
+ * single RowsAndColumns object that it is given from its child Operator.  A different operator should be used
+ * if that is an important bit of functionality to have.


Any preconditions? Do things need to be sorted any particular way?

gianm · 2022-12-05T07:43:29Z

sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidQuery.java

+            Windowing.fromCalciteStuff(
+                partialQuery,
+                plannerContext,
+                sourceRowSignature, // TODO(gianm): window can only apply to the source data, because SCAN -> WINDOW


I think this would be fine as a regular comment. It's saying that it's ok to use the sourceRowSignature here, because PartialDruidQuery (which enforces ordering of operators) only allows WINDOW to apply on top of SCAN. And SCAN is where the sourceRowSignature comes from.

gianm · 2022-12-05T07:45:19Z

sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidQuery.java

@@ -870,6 +911,12 @@ public Query<?> getQuery()
   */
  private Query<?> computeQuery()
  {
+    // TODO(gianm): structure


I think with this comment I was thinking that the code could be structured better. But that was a while ago. I'm over it.

gianm · 2022-12-05T07:48:21Z

sql/src/main/java/org/apache/druid/sql/calcite/rel/PartialDruidQuery.java

    WHERE_FILTER,
    SELECT_PROJECT,

-    // AGGREGATE, HAING_FILTER, AGGREGATE_PROJECT can only be present on aggregating queries.
+    // AGGREGATE, HAING_FILTER, AGGREGATE_PROJECT can only be present on non-WINDOW aggregating queries.


HAVING_FILTER (spelling)

gianm · 2022-12-05T07:54:07Z

sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidRules.java

@@ -86,10 +87,16 @@ public static List<RelOptRule> rules(PlannerContext plannerContext)
            PartialDruidQuery.Stage.SORT_PROJECT,
            PartialDruidQuery::withSortProject
        ),
+        new DruidQueryRule<>(


Could you adjust things so this rule isn't added unless windowsAreForClosers: true? Will limit the impact if it turns out that something is wrong with the rule.

gianm · 2022-12-05T07:54:12Z

sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidRules.java

        DruidOuterQueryRule.AGGREGATE,
        DruidOuterQueryRule.WHERE_FILTER,
        DruidOuterQueryRule.SELECT_PROJECT,
        DruidOuterQueryRule.SORT,
+        DruidOuterQueryRule.WINDOW,


Same with this one.

rohangarg

Thanks a lot for the feature Eric! :)

Posting a partial review for now - will do the remaining soon surely. Thanks for the patience!

rohangarg · 2022-12-04T14:04:15Z

processing/src/main/java/org/apache/druid/query/rowsandcols/AppendableRowsAndColumns.java

+import org.apache.druid.query.rowsandcols.column.Column;
+
+/**
+ * A RowsAndColumns that supposed appending columns.  This interface is particularly useful because even if there is


Suggested change

* A RowsAndColumns that supposed appending columns. This interface is particularly useful because even if there is

* A RowsAndColumns that supports appending columns. This interface is particularly useful because even if there is

rohangarg · 2022-12-04T17:42:22Z

processing/src/main/java/org/apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java

+import java.util.Optional;
+import java.util.function.Function;
+
+public class ArrayListRowsAndColumns<RowType> implements RowsAndColumns


This is very similar to RowBasedStorageAdapter - but I guess we can't reuse it since there are lot of differences in the interface

Yes, it was basically copied from that, but created specifically to assume that it had an ArrayList (and therefore be randomly accessible)

rohangarg · 2022-12-04T17:47:03Z

processing/src/main/java/org/apache/druid/query/rowsandcols/DefaultGroupPartitioner.java

+import java.util.List;
+
+@SuppressWarnings("unused")
+public class DefaultGroupPartitioner implements GroupPartitioner


Currently this and the default sorted partitioner are same right? Are you thinking that in future, the GroupPartitioner may also allow hash based partitioning?

Yes, with the one exception that this one isn't used anymore. It's really just sitting around in the code for posterity rather than any specific usage. Which probably means it's worth deleting. I'd prefer to do that in a subsequent PR though.

rohangarg · 2022-12-04T17:54:05Z

processing/src/main/java/org/apache/druid/query/rowsandcols/DefaultGroupPartitioner.java

+      int currGroup = 0;
+      int prevGroupVal = 0;
+      for (int i = 1; i < retVal.length; ++i) {
+        if (retVal[i] == prevGroupVal) {


this seems incorrect, should we always just do accessor.compareCells(i - 1, i) and then check the result of that comparison?
I think that retVal[i] will always be 0 in this check

retVal[i] will always be 0 for the first column that is walked. For the second column, it will only be all-zero if all of the values of the first column were the same. Basically, the algorithm is computing the groupings one column at a time, and the value of retVal up until then is the grouping of all of the previous columns. This check is validating that we are still in an equivalency group based on all of the previous columns (if row i and row i-1 are different based on the previous columns, then the value of our new column doesn't matter, row i and row i-1 will be part of different groupings)

rohangarg · 2022-12-04T18:01:04Z

processing/src/main/java/org/apache/druid/query/rowsandcols/DefaultOnHeapAggregatable.java

+    for (Aggregator agg : aggs) {
+      retVal.add(agg.get());
+    }
+    return retVal;


we should close the aggregators before returning

Yes, you are right. I realized that in another PR and have that change coming in the follow-on PR.

rohangarg · 2022-12-04T18:06:23Z

processing/src/main/java/org/apache/druid/query/rowsandcols/DefaultOnHeapAggregatable.java

+    int rowId = currRow.get();
+    while (rowId < numRows) {
+      for (int i = 0; i < aggs.length; ++i) {
+        aggs[i].aggregate();


maybe could add a new method which returns the current state upon calling aggregate - the default impl. would do aggregate + get. shouldn't do it incase seems no future use-case for it.

Adding methods to AggregatorFactory must be done with care, that's an interface that is the most likely to be implemented externally.

Either way, the aggregate-then-get algorithm implemented here is technically incorrect (sketch aggs will update the object that they return from get()), so I have a follow on PR that does window framing properly and actually implements things correctly.

rohangarg · 2022-12-04T18:06:37Z

processing/src/main/java/org/apache/druid/query/rowsandcols/DefaultOnHeapAggregatable.java

+      rowId = currRow.incrementAndGet();
+    }
+
+    return retVal;


we should close the aggregators before returning

follow-on PR

rohangarg · 2022-12-04T18:13:45Z

processing/src/main/java/org/apache/druid/query/rowsandcols/OnHeapCumulativeAggregatable.java

+   * @param aggFactories definition of aggregations to be done
+   * @return a list of objects, one per AggregatorFactory.  That is, the length of the return list should be equal to
+   * the length of the aggFactories list passed as an argument, while the length of the internal {@code Object[]} will
+   * be equivalent to the number of rows


can we transpose the dimensions (aggs, rows) of this 2D structure to (rows, aggs)? that could also potentially allow for returning a streaming structure from aggregateCumulative to lessen memory reqs. The stream could be consumed by the higher operators.

In a follow-on PR, I've changed this interface to return a RowsAndColumns. The input is also a RowsAndColumns, there is an assumption that we are returning batches of rows, rather than returning row-at-a-time. We can still stream batches, just not rows. I think it should be fine/good to assume a stream of batches.

rohangarg · 2022-12-04T18:23:45Z

processing/src/main/java/org/apache/druid/query/rowsandcols/GroupPartitioner.java

+   * @return the groupings, rows with the same int value are in the same group.  There is no sort-order implied by the
+   * int values.
+   */
+  int[] computeGroupings(List<String> columns);


just for reminder, that int[] would limit the processing to INT_MAX values even if we have memory left. if we want to support partitioning more than that values, we could consider using BigArrays from fast-util or something like that.

Yeah... this interface is probably gonna die anyway, it's not actually used in this PR anyway.

rohangarg · 2022-12-04T18:29:35Z

processing/src/main/java/org/apache/druid/query/rowsandcols/DefaultOnHeapAggregatable.java

+{
+  private final RowsAndColumns rac;
+
+  public DefaultOnHeapAggregatable(


I'm not sure what the correct name would be, but this name seems a bit off. The methods of this class independently build aggregate tables on the RowsAndColumns object.
I mean that this class is mostly stateless wrt the aggregate table.

If you're gonna complain about naming, you gotta give an alternative name that you like better :P.

I'm fine with changing names and can do it on the subsequent PR that adds window framing, but, what's to say that I change the name and it's still not loved? ;)

lgtm-com · 2022-12-06T01:39:54Z

This pull request introduces 2 alerts when merging 8cc8235 into 658a9c2 - view on LGTM.com

new alerts:

1 for Dereferenced variable may be null
1 for Boxed variable is never null

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

clintropolis

rad 🤘

nothing blocker so i'm going to leave a checkmark, played around with this in debugger and everything seems to be working well enough

I agree with @gianm about 'cell' terminology, while correct it is a bit jarring since we don't really use that elsewhere and am afraid it would be a bit extra cognitive load unless we plan to standardize other areas of the code to use cell instead of row. it looks like you have removed a lot of it, but there are still a handful of references in method names and javadocs, compareCells, getActualCell, etc. I don't think it needs changed right now though

🚀

clintropolis · 2022-12-06T02:14:30Z

processing/src/test/java/org/apache/druid/query/operator/WindowOperatorQueryTest.java

+        new ArrayList<>()
+    );
+  }
+


could you add a json serde test (unless i missed it somewhere) in follow-up PR?

clintropolis · 2022-12-06T05:34:49Z

processing/src/main/java/org/apache/druid/query/operator/window/WindowAggregateProcessor.java

+    AppendableRowsAndColumns retVal = RowsAndColumns.expectAppendable(inputPartition);
+
+    if (aggregations != null) {
+      OnHeapAggregatable aggregatable = inputPartition.as(OnHeapAggregatable.class);


is this a stub? (none of the RowsAndColumns implementations seem to implement this...)

This is a common pattern for all of the "semantic interfaces" actually. For all of them, there is a default implementation that can be fallen back to (the default is generally implemented in terms of "find necessary columns, use their ColumnAccessor and do logic), but we always ask the RowsAndColumns first so that it can offer a more optimal implementation if it wants. This PR doesn't yet have more optimal implementations, but I expect that I will have to create some to truly make the SortOperator that I will need work in any sort of meaningful manner. So, until that PR, this will look very barren and then suddenly it will either become super magical or make sense or both.

ah sorry for the confusion, the pattern makes perfect sense to me, I guess I was just wondering if you had any concrete plans or if this was more of a placeholder at this time which you've clarified, thanks 👍

clintropolis · 2022-12-06T05:35:21Z

processing/src/main/java/org/apache/druid/query/operator/window/WindowAggregateProcessor.java

+    }
+
+    if (cumulativeAggregations != null) {
+      OnHeapCumulativeAggregatable cummulativeAgg = inputPartition.as(OnHeapCumulativeAggregatable.class);


same comment about stub

clintropolis · 2022-12-06T05:38:00Z

...src/main/java/org/apache/druid/query/operator/window/ranking/WindowRankingProcessorBase.java

+  {
+    final AppendableRowsAndColumns retVal = RowsAndColumns.expectAppendable(incomingPartition);
+
+    SortedGroupPartitioner groupPartitioner = incomingPartition.as(SortedGroupPartitioner.class);


same question re: stub

clintropolis · 2022-12-06T05:45:03Z

processing/src/main/java/org/apache/druid/query/rowsandcols/column/ColumnAccessor.java

+   * @param rowNum the cell id, 0-indexed
+   * @return the primitive {@code int} representation of the cell.  Returns {@code 0} If {@link #isNull} is true.
+   */
+  int getInt(int rowNum);


what are the plans for this? when should it be used? or is it just for the row number thing?

gianm

Looks good, although please fix the comments in PartialDruidQuery#canAccept. Looks like they got mangled somehow.

gianm · 2022-12-06T07:42:41Z

sql/src/main/java/org/apache/druid/sql/calcite/rel/PartialDruidQuery.java

-      // Cannot do post-sort stages without a sort.
-      return false;
-    } else {
+    } else // Cannot do post-sort stages without a sort.


Something is weird with the comments here… automated rewrite gone wrong?

YEah, looks like it re-wrote the final one to actually just return instead of be a branch and then had a hard time with the comments...

rohangarg

Thanks for the response on the review. LGTM given that the needed changes from the review will be posted in the follow-up!

Thanks a lot for adding the support! 🚀

lgtm-com · 2022-12-06T09:48:52Z

This pull request introduces 2 alerts when merging edb06d8 into 136322d - view on LGTM.com

new alerts:

1 for Dereferenced variable may be null
1 for Boxed variable is never null

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

imply-cheddar and others added 24 commits November 7, 2022 15:52

Wire up windowed processors with a query type that

54e8c65

can run them end-to-end. This code can be used to actually run a query, so yay!

wip

a5eead8

Merge remote-tracking branch 'imply-cheddar/window-operations' into s…

f3b61ef

…ql-window

WindowOperatorQuery SQL bindings

f5ccb09

Wire up windowed processors with a query type that

cbcb264

can run them end-to-end. This code can be used to actually run a query, so yay!

Wire up windowed processors with a query type that

32971ac

can run them end-to-end. This code can be used to actually run a query, so yay!

Some SQL tests for window functions

2148a78

Added wikipedia data to the indexes available to the SQL queries and tests validating the windowing functionality as it exists now.

Add Javadoc to interfaces

ffde5a9

Reformat Code and optimize imports

b72a1b2

Add license header

b5bfad4

2 whitespaces in the copyright clob

5131674

Test and style fixes

5f43d28

Fix dependencies

a979d83

Forbidden APIs

d012b86

Checkstyle

598c373

Tests for coverage

f989459

Support SQL-compatible mode in tests

1300de7

Some initial tests for RowsAndColumns directly

2a2023f

Tests and style

70488b7

Expand branch coverage for tests

f3d3270

Tests and style

851acb8

vogievetsky reviewed Nov 30, 2022

View reviewed changes

abhishekagarwal87 mentioned this pull request Dec 1, 2022

Convert SQL Lag query to Druid #13474

Closed

Make windowing off by default

299d735

vogievetsky mentioned this pull request Dec 3, 2022

Web console: add arrayOfDoublesSketch and other small fixes #13486

Merged

imply-cheddar added 2 commits December 5, 2022 12:53

Merge remote-tracking branch 'apache/master' into windowinging

ea566d9

Make it compile after merge.

935864c

gianm reviewed Dec 5, 2022

View reviewed changes

Changes from code review

f38945c

rohangarg reviewed Dec 5, 2022

View reviewed changes

rohangarg added Feature Area - Querying labels Dec 5, 2022

Checkstyle and tests

8cc8235

clintropolis approved these changes Dec 6, 2022

View reviewed changes

Merge remote-tracking branch 'apache/master' into windowinging

01cea90

gianm approved these changes Dec 6, 2022

View reviewed changes

Fix comment

edb06d8

rohangarg approved these changes Dec 6, 2022

View reviewed changes

cheddar merged commit 83261f9 into apache:master Dec 6, 2022

imply-cheddar deleted the windowinging branch December 6, 2022 23:54

clintropolis added this to the 26.0 milestone Apr 10, 2023

gianm mentioned this pull request Apr 25, 2023

Community roadmap 2023 #14157

Open

	* A RowsAndColumns that supposed appending columns. This interface is particularly useful because even if there is
	* A RowsAndColumns that supports appending columns. This interface is particularly useful because even if there is

Starting on Window Functions #13458

Starting on Window Functions #13458

Conversation

imply-cheddar commented Nov 30, 2022

Description

Sharp Edges

Release note

lgtm-com bot commented Nov 30, 2022

Choose a reason for hiding this comment

lgtm-com bot commented Dec 2, 2022

lgtm-com bot commented Dec 5, 2022

gianm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rohangarg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgtm-com bot commented Dec 6, 2022

clintropolis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gianm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rohangarg left a comment

Choose a reason for hiding this comment

lgtm-com bot commented Dec 6, 2022