mihaibudiu commented on code in PR #3811:
URL: https://github.com/apache/calcite/pull/3811#discussion_r1626558428
##########
core/src/test/java/org/apache/calcite/rel/logical/ToLogicalConverterTest.java:
##########
@@ -467,6 +467,22 @@ private void verify(RelNode rel, String expectedPhysical,
String expectedLogical
verify(rel(sql), expectedPhysical, expectedLogical);
}
+ @Test void testWindowExclude() {
+ String sql = "SELECT sum(\"salary\") over (order by \"hire_date\" rows
between "
+ + "unbounded preceding and current row exclude group) FROM
\"employee\"";
+ String expectedPhysical = ""
+ + "EnumerableProject($0=[$17])\n"
+ + " EnumerableWindow(window#0=[window(order by [9] rows between"
+ + " UNBOUNDED PRECEDING and CURRENT ROW EXCLUDE GROUP aggs
[SUM($11)])])\n"
+ + " JdbcToEnumerableConverter\n"
+ + " JdbcTableScan(table=[[foodmart, employee]])\n";
+ String expectedLogical = ""
+ + "LogicalProject($0=[$17])\n"
+ + " LogicalWindow(window#0=[window(order by [9] rows between"
+ + " UNBOUNDED PRECEDING and CURRENT ROW EXCLUDE GROUP aggs
[SUM($11)])])\n"
+ + " LogicalTableScan(table=[[foodmart, employee]])\n";
+ verify(rel(sql), expectedPhysical, expectedLogical);
+ }
Review Comment:
missing empty line?
do you want to test the other kinds of exclusions too?
##########
core/src/main/java/org/apache/calcite/rex/RexBuilder.java:
##########
@@ -389,6 +389,7 @@ public RexNode makeOver(
ImmutableList<RexFieldCollation> orderKeys,
RexWindowBound lowerBound,
RexWindowBound upperBound,
+ RexWindowExclusion exclude,
Review Comment:
Thes look like breaking API changes which have to be documented in
history.md.
I don't know if the policy is to leave around the old API with a default
value for the new parameter.
##########
core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java:
##########
@@ -969,6 +977,25 @@ private void
addRexFieldCollationList(List<RexFieldCollation> list,
}
}
+ private @Nullable RexWindowExclusion toRexWindowExclusion(@Nullable
Map<String, Object> map) {
+ if (map == null) {
+ return null;
+ }
+ final String type = get(map, "type");
+ switch (type) {
+ case "CURRENT_ROW":
+ return RexWindowExclusion.EXCLUDE_CURRENT_ROW;
+ case "GROUP":
+ return RexWindowExclusion.EXCLUDE_GROUP;
+ case "TIES":
+ return RexWindowExclusion.EXCLUDE_TIES;
+ case "NO OTHERS":
+ return RexWindowExclusion.EXCLUDE_NO_OTHER;
+ default:
+ throw new UnsupportedOperationException(
+ "cannot convert type to rex window exclusion " + type);
Review Comment:
I would write this as "cannot convert '" + type + "'to rex window exclusion"
##########
core/src/main/java/org/apache/calcite/rel/core/Window.java:
##########
@@ -358,7 +365,8 @@ public RelCollation collation() {
public boolean isAlwaysNonEmpty() {
int lowerKey = lowerBound.getOrderKey();
int upperKey = upperBound.getOrderKey();
- return lowerKey > -1 && lowerKey <= upperKey;
+ return lowerKey > -1 && lowerKey <= upperKey
Review Comment:
wouldn't the "no ties" also apply here?
--
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: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
