From 5320adfcde8b31b5d3a22e0e84b3cf673b903b61 Mon Sep 17 00:00:00 2001 From: Stefan Bischof Date: Fri, 16 Oct 2026 17:50:35 +0200 Subject: [PATCH] fix(dialect): the n-th value is an aggregate over the group's rows NTH_VALUE ran as a window over the groups and showed other members' values. Dialects without an ordered array aggregate refuse the measure. Signed-off-by: Stefan Bischof --- .../api/generator/AggregationGenerator.java | 40 ++++++++++++------- .../db/clickhouse/ClickHouseDialect.java | 9 +++-- .../db/clickhouse/ClickHouseDialectTest.java | 2 +- .../sql/dialect/db/duckdb/DuckDbDialect.java | 6 ++- .../dialect/db/duckdb/DuckDbNthValueTest.java | 34 ++++++++++++++++ .../daanse/sql/dialect/db/h2/H2Dialect.java | 6 ++- .../MicrosoftSqlServerDialect.java | 11 +++-- .../sql/dialect/db/mysql/MySqlDialect.java | 9 +++-- .../sql/dialect/db/oracle/OracleDialect.java | 11 +++-- .../db/postgresql/PostgreSqlDialect.java | 6 ++- 10 files changed, 98 insertions(+), 36 deletions(-) create mode 100644 dialect/db/duckdb/src/test/java/org/eclipse/daanse/sql/dialect/db/duckdb/DuckDbNthValueTest.java diff --git a/dialect/api/src/main/java/org/eclipse/daanse/sql/dialect/api/generator/AggregationGenerator.java b/dialect/api/src/main/java/org/eclipse/daanse/sql/dialect/api/generator/AggregationGenerator.java index 54556d0..c9527b0 100644 --- a/dialect/api/src/main/java/org/eclipse/daanse/sql/dialect/api/generator/AggregationGenerator.java +++ b/dialect/api/src/main/java/org/eclipse/daanse/sql/dialect/api/generator/AggregationGenerator.java @@ -109,27 +109,37 @@ default StringBuilder buildPercentileFunction(String functionName, double percen * @param supportsNullsHandling whether the engine accepts {@code IGNORE NULLS}/ * {@code RESPECT NULLS} */ - default StringBuilder buildNthValueFunction(String functionName, CharSequence operand, boolean ignoreNulls, - Integer n, List columns, boolean supportsNullsHandling) { + /** + * The n-th value of the rows of the group, in the order of the columns, as an ordered array aggregate: + * {@code ARRAY_AGG(operand ORDER BY ...) FILTER (WHERE operand IS NOT NULL)}. + * An aggregate, not the window function {@code NTH_VALUE ... OVER (ORDER BY ...)}: a window in a grouped + * query runs over the groups, so a cell showed the values of other members' groups (V-82). n counts from 1; a + * group with fewer values gives NULL. + * + * @param arrayAgg the dialect's ordered array aggregate, {@code ARRAY_AGG} or {@code list} + * @param open what stands before the array, {@code "("} or {@code "list_extract("} + * @param close what follows it and picks the element: {@code ")[" + n + "]"} or {@code ", " + n + ")"} + */ + default StringBuilder buildNthValueOfArray(String arrayAgg, CharSequence operand, boolean ignoreNulls, + List columns, String open, String close) { StringBuilder buf = new StringBuilder(64); - buf.append(functionName); - buf.append(OPEN_PAREN); - buf.append(operand); - buf.append(COMMA); - buf.append(n == null || n < 1 ? 1 : n); - buf.append(CLOSE_PAREN); - if (supportsNullsHandling) { - buf.append(ignoreNulls ? IGNORE_NULLS : RESPECT_NULLS); - } - buf.append(OVER); + buf.append(open).append(arrayAgg).append('(').append(operand); if (columns != null && !columns.isEmpty()) { - buf.append(ORDER_BY); - buf.append(buildOrderedColumnsClause(columns)); + buf.append(" ORDER BY ").append(buildOrderedColumnsClause(columns)); } - buf.append(CLOSE_PAREN); + buf.append(')'); + if (ignoreNulls) { + buf.append(" FILTER (WHERE ").append(operand).append(" IS NOT NULL)"); + } + buf.append(close); return buf; } + /** n as the element to pick: at least 1. */ + static int nthIndex(Integer n) { + return n == null || n < 1 ? 1 : n; + } + default CharSequence buildOrderedColumnsClause(List columns) { StringBuilder buf = new StringBuilder(64); if (columns == null) { diff --git a/dialect/db/clickhouse/src/main/java/org/eclipse/daanse/sql/dialect/db/clickhouse/ClickHouseDialect.java b/dialect/db/clickhouse/src/main/java/org/eclipse/daanse/sql/dialect/db/clickhouse/ClickHouseDialect.java index 8df4577..32c6f6f 100644 --- a/dialect/db/clickhouse/src/main/java/org/eclipse/daanse/sql/dialect/db/clickhouse/ClickHouseDialect.java +++ b/dialect/db/clickhouse/src/main/java/org/eclipse/daanse/sql/dialect/db/clickhouse/ClickHouseDialect.java @@ -233,16 +233,19 @@ public java.util.Optional generateListAgg(CharSequence operand, boolean return java.util.Optional.of((buf).toString()); } + /** + * No ordered array aggregate to pick the n-th value from; the window function NTH_VALUE runs over the groups of + * a grouped query, not over the rows of one group, so it is no answer either (V-82). The measure is refused. + */ @Override public java.util.Optional generateNthValueAgg(CharSequence operand, boolean ignoreNulls, Integer n, List columns) { - return java.util.Optional - .of((buildNthValueFunction("nth_value", operand, ignoreNulls, n, columns, false)).toString()); + return java.util.Optional.empty(); } @Override public boolean supportsNthValue() { - return true; + return false; } @Override diff --git a/dialect/db/clickhouse/src/test/java/org/eclipse/daanse/sql/dialect/db/clickhouse/ClickHouseDialectTest.java b/dialect/db/clickhouse/src/test/java/org/eclipse/daanse/sql/dialect/db/clickhouse/ClickHouseDialectTest.java index e9ee7f6..64f7153 100644 --- a/dialect/db/clickhouse/src/test/java/org/eclipse/daanse/sql/dialect/db/clickhouse/ClickHouseDialectTest.java +++ b/dialect/db/clickhouse/src/test/java/org/eclipse/daanse/sql/dialect/db/clickhouse/ClickHouseDialectTest.java @@ -152,7 +152,7 @@ class WindowFunctionTests { @Test void testSupportsNthValue() { - assertTrue(dialect.supportsNthValue()); + assertFalse(dialect.supportsNthValue()); // V-82: no ordered array aggregate, the window form ran over the groups } @Test diff --git a/dialect/db/duckdb/src/main/java/org/eclipse/daanse/sql/dialect/db/duckdb/DuckDbDialect.java b/dialect/db/duckdb/src/main/java/org/eclipse/daanse/sql/dialect/db/duckdb/DuckDbDialect.java index ddc1749..846c585 100644 --- a/dialect/db/duckdb/src/main/java/org/eclipse/daanse/sql/dialect/db/duckdb/DuckDbDialect.java +++ b/dialect/db/duckdb/src/main/java/org/eclipse/daanse/sql/dialect/db/duckdb/DuckDbDialect.java @@ -12,6 +12,7 @@ */ package org.eclipse.daanse.sql.dialect.db.duckdb; +import org.eclipse.daanse.sql.dialect.api.generator.AggregationGenerator; import java.util.List; import java.util.Optional; import java.util.regex.Matcher; @@ -383,11 +384,12 @@ public StringBuilder buildPercentileFunction(String functionName, double percent return buf; } + /** The n-th value of the group's rows in their order: an ordered list, its n-th element (V-82). */ @Override public java.util.Optional generateNthValueAgg(CharSequence operand, boolean ignoreNulls, Integer n, List columns) { - return java.util.Optional - .of((buildNthValueFunction("NTH_VALUE", operand, ignoreNulls, n, columns, false)).toString()); + return java.util.Optional.of(buildNthValueOfArray("list", operand, ignoreNulls, columns, "list_extract(", + ", " + AggregationGenerator.nthIndex(n) + ")").toString()); } /** diff --git a/dialect/db/duckdb/src/test/java/org/eclipse/daanse/sql/dialect/db/duckdb/DuckDbNthValueTest.java b/dialect/db/duckdb/src/test/java/org/eclipse/daanse/sql/dialect/db/duckdb/DuckDbNthValueTest.java new file mode 100644 index 0000000..346b1b3 --- /dev/null +++ b/dialect/db/duckdb/src/test/java/org/eclipse/daanse/sql/dialect/db/duckdb/DuckDbNthValueTest.java @@ -0,0 +1,34 @@ +/* + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * SmartCity Jena - initial + */ +package org.eclipse.daanse.sql.dialect.db.duckdb; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.List; + +import org.junit.jupiter.api.Test; + +/** + * The n-th value is an aggregate over the rows of the group -- an ordered list and its n-th element -- not the + * window NTH_VALUE ... OVER, which in a grouped query runs over the groups (V-82). + */ +class DuckDbNthValueTest { + + @Test + void theNthValueIsTheNthElementOfTheGroupsList() { + assertEquals("list_extract(list(\"VALUE\") FILTER (WHERE \"VALUE\" IS NOT NULL), 2)", + new DuckDbDialect().generateNthValueAgg("\"VALUE\"", true, 2, List.of()).orElseThrow()); + assertEquals("list_extract(list(\"VALUE\"), 1)", + new DuckDbDialect().generateNthValueAgg("\"VALUE\"", false, 0, List.of()).orElseThrow()); + } +} diff --git a/dialect/db/h2/src/main/java/org/eclipse/daanse/sql/dialect/db/h2/H2Dialect.java b/dialect/db/h2/src/main/java/org/eclipse/daanse/sql/dialect/db/h2/H2Dialect.java index ace7d69..20cd640 100644 --- a/dialect/db/h2/src/main/java/org/eclipse/daanse/sql/dialect/db/h2/H2Dialect.java +++ b/dialect/db/h2/src/main/java/org/eclipse/daanse/sql/dialect/db/h2/H2Dialect.java @@ -13,6 +13,7 @@ */ package org.eclipse.daanse.sql.dialect.db.h2; +import org.eclipse.daanse.sql.dialect.api.generator.AggregationGenerator; import java.util.List; import java.util.Optional; @@ -187,11 +188,12 @@ public java.util.Optional generateListAgg(CharSequence operand, boolean return java.util.Optional.of((buf).toString()); } + /** The n-th value of the group's rows in their order: an ordered array, its n-th element (V-82). */ @Override public java.util.Optional generateNthValueAgg(CharSequence operand, boolean ignoreNulls, Integer n, List columns) { - return java.util.Optional - .of((buildNthValueFunction("NTH_VALUE", operand, ignoreNulls, n, columns, true)).toString()); + return java.util.Optional.of(buildNthValueOfArray("ARRAY_AGG", operand, ignoreNulls, columns, "(", + ")[" + AggregationGenerator.nthIndex(n) + "]").toString()); } @Override diff --git a/dialect/db/mssqlserver/src/main/java/org/eclipse/daanse/sql/dialect/db/mssqlserver/MicrosoftSqlServerDialect.java b/dialect/db/mssqlserver/src/main/java/org/eclipse/daanse/sql/dialect/db/mssqlserver/MicrosoftSqlServerDialect.java index b501e97..dcd31cd 100644 --- a/dialect/db/mssqlserver/src/main/java/org/eclipse/daanse/sql/dialect/db/mssqlserver/MicrosoftSqlServerDialect.java +++ b/dialect/db/mssqlserver/src/main/java/org/eclipse/daanse/sql/dialect/db/mssqlserver/MicrosoftSqlServerDialect.java @@ -469,21 +469,24 @@ public java.util.Optional generateListAgg(CharSequence operand, boolean return java.util.Optional.of((buf).toString()); } + /** + * No ordered array aggregate to pick the n-th value from; the window function NTH_VALUE runs over the groups of + * a grouped query, not over the rows of one group, so it is no answer either (V-82). The measure is refused. + */ @Override public java.util.Optional generateNthValueAgg(CharSequence operand, boolean ignoreNulls, Integer n, List columns) { - return java.util.Optional - .of((buildNthValueFunction("NTH_VALUE", operand, ignoreNulls, n, columns, true)).toString()); + return java.util.Optional.empty(); } @Override public boolean supportsNthValue() { - return true; + return false; } @Override public boolean supportsNthValueIgnoreNulls() { - return true; + return false; } @Override diff --git a/dialect/db/mysql/src/main/java/org/eclipse/daanse/sql/dialect/db/mysql/MySqlDialect.java b/dialect/db/mysql/src/main/java/org/eclipse/daanse/sql/dialect/db/mysql/MySqlDialect.java index e29e012..b89e0a1 100644 --- a/dialect/db/mysql/src/main/java/org/eclipse/daanse/sql/dialect/db/mysql/MySqlDialect.java +++ b/dialect/db/mysql/src/main/java/org/eclipse/daanse/sql/dialect/db/mysql/MySqlDialect.java @@ -552,16 +552,19 @@ public java.util.Optional generateListAgg(CharSequence operand, boolean return java.util.Optional.of((buf).toString()); } + /** + * No ordered array aggregate to pick the n-th value from; the window function NTH_VALUE runs over the groups of + * a grouped query, not over the rows of one group, so it is no answer either (V-82). The measure is refused. + */ @Override public java.util.Optional generateNthValueAgg(CharSequence operand, boolean ignoreNulls, Integer n, List columns) { - return java.util.Optional - .of((buildNthValueFunction("NTH_VALUE", operand, ignoreNulls, n, columns, false)).toString()); + return java.util.Optional.empty(); } @Override public boolean supportsNthValue() { - return true; + return false; } @Override diff --git a/dialect/db/oracle/src/main/java/org/eclipse/daanse/sql/dialect/db/oracle/OracleDialect.java b/dialect/db/oracle/src/main/java/org/eclipse/daanse/sql/dialect/db/oracle/OracleDialect.java index 4b1b5bd..f66f2b5 100644 --- a/dialect/db/oracle/src/main/java/org/eclipse/daanse/sql/dialect/db/oracle/OracleDialect.java +++ b/dialect/db/oracle/src/main/java/org/eclipse/daanse/sql/dialect/db/oracle/OracleDialect.java @@ -528,21 +528,24 @@ public java.util.Optional generateListAgg(CharSequence operand, boolean return java.util.Optional.of((buf).toString()); } + /** + * No ordered array aggregate to pick the n-th value from; the window function NTH_VALUE runs over the groups of + * a grouped query, not over the rows of one group, so it is no answer either (V-82). The measure is refused. + */ @Override public java.util.Optional generateNthValueAgg(CharSequence operand, boolean ignoreNulls, Integer n, List columns) { - return java.util.Optional - .of((buildNthValueFunction("NTH_VALUE", operand, ignoreNulls, n, columns, true)).toString()); + return java.util.Optional.empty(); } @Override public boolean supportsNthValue() { - return true; + return false; } @Override public boolean supportsNthValueIgnoreNulls() { - return true; + return false; } @Override diff --git a/dialect/db/postgresql/src/main/java/org/eclipse/daanse/sql/dialect/db/postgresql/PostgreSqlDialect.java b/dialect/db/postgresql/src/main/java/org/eclipse/daanse/sql/dialect/db/postgresql/PostgreSqlDialect.java index f342438..55ab7aa 100644 --- a/dialect/db/postgresql/src/main/java/org/eclipse/daanse/sql/dialect/db/postgresql/PostgreSqlDialect.java +++ b/dialect/db/postgresql/src/main/java/org/eclipse/daanse/sql/dialect/db/postgresql/PostgreSqlDialect.java @@ -22,6 +22,7 @@ package org.eclipse.daanse.sql.dialect.db.postgresql; +import org.eclipse.daanse.sql.dialect.api.generator.AggregationGenerator; import java.sql.ResultSetMetaData; import java.sql.SQLException; import java.sql.Types; @@ -428,11 +429,12 @@ public java.util.Optional generatePercentileCont(double percentile, bool .of((buildPercentileFunction("PERCENTILE_CONT", percentile, desc, tableName, columnName)).toString()); } + /** The n-th value of the group's rows in their order: an ordered array, its n-th element (V-82). */ @Override public java.util.Optional generateNthValueAgg(CharSequence operand, boolean ignoreNulls, Integer n, List columns) { - return java.util.Optional - .of((buildNthValueFunction("NTH_VALUE", operand, ignoreNulls, n, columns, false)).toString()); + return java.util.Optional.of(buildNthValueOfArray("ARRAY_AGG", operand, ignoreNulls, columns, "(", + ")[" + AggregationGenerator.nthIndex(n) + "]").toString()); } @Override