Repository: duckdb/duckdb (language: python)

Issue:
LIST(DISTINCT V ORDER BY V) ignores DISTINCT keyword
### What happens?

When the DISTINCT modifier is used inside of the LIST aggregate it produces an expected result. When the ORDER BY modifier is also added the DISTINCT modifier is ignored.

### To Reproduce

```sql
select list(distinct v % 2) c2 from generate_series(20) s(v);
```
```
┌─────────┐
│   c2    │
│ int64[] │
├─────────┤
│ [0, 1]  │
└─────────┘
```
```sql
select list(distinct v % 2 order by v) c2 from generate_series(20) s(v, i);
```
```
┌─────────────────────────────────────────────────────────────────┐
│                               c2                                │
│                             int64[]                             │
├─────────────────────────────────────────────────────────────────┤
│ [0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0] │
└─────────────────────────────────────────────────────────────────┘
```

### OS:

OSX

### DuckDB Version:

v1.1.2 f680b7d08f

### DuckDB Client:

CLI

### Hardware:

_No response_

### Full Name:

Max Seiden

### Affiliation:

Sigma Computing

### What is the latest build you tested with? If possible, we recommend testing with the latest nightly build.

I have tested with a stable release

### Did you include all relevant data sets for reproducing the issue?

Yes

### Did you include all code required to reproduce the issue?

- [X] Yes, I have

### Did you include all relevant configuration (e.g., CPU architecture, Python version, Linux distribution) to reproduce the issue?

- [X] Yes, I have


Discussion / hints:
Thanks for the report. The problem is that this is not valid:

```cpp
// If the aggregate is DISTINCT then the ORDER BYs need to be functional dependencies of the arguments.
```

but we are not checking for it correctly.

Fix patch:
diff --git a/src/planner/binder/expression/bind_aggregate_expression.cpp b/src/planner/binder/expression/bind_aggregate_expression.cpp
index 97d690c3dcca..4ab4d1a7511d 100644
--- a/src/planner/binder/expression/bind_aggregate_expression.cpp
+++ b/src/planner/binder/expression/bind_aggregate_expression.cpp
@@ -21,16 +21,31 @@
 
 namespace duckdb {
 
-static bool ExtractFunctionalDependencies(column_binding_set_t &deps, const unique_ptr<Expression> &expr) {
-	if (expr->type == ExpressionType::BOUND_COLUMN_REF) {
-		auto &colref = expr->Cast<BoundColumnRefExpression>();
-		deps.insert(colref.binding);
+static bool IsFunctionallyDependent(const unique_ptr<Expression> &expr, const vector<unique_ptr<Expression>> &deps) {
+	//	Volatile expressions can't depend on anything else
+	if (expr->IsVolatile()) {
+		return false;
+	}
+	//	Constant expressions are always FD
+	if (expr->IsFoldable()) {
+		return true;
+	}
+	// If the expression matches ANY of the dependencies, then it is FD on them
+	for (const auto &dep : deps) {
+		// We don't need to check volatility of the dependencies because we checked it for the expression.
+		if (expr->Equals(*dep)) {
+			return true;
+		}
 	}
 
-	bool is_volatile = expr->IsVolatile();
-	ExpressionIterator::EnumerateChildren(
-	    *expr, [&](unique_ptr<Expression> &child) { is_volatile |= ExtractFunctionalDependencies(deps, child); });
-	return is_volatile;
+	// The expression doesn't match any dependency, so check ALL children.
+	bool has_children = false;
+	bool are_dependent = true;
+	ExpressionIterator::EnumerateChildren(*expr, [&](unique_ptr<Expression> &child) {
+		has_children = true;
+		are_dependent &= IsFunctionallyDependent(child, deps);
+	});
+	return has_children && are_dependent;
 }
 
 static Value NegatePercentileValue(const Value &v, const bool desc) {
@@ -265,26 +280,9 @@ BindResult BaseSelectBinder::BindAggregate(FunctionExpression &aggr, AggregateFu
 
 	// If the aggregate is DISTINCT then the ORDER BYs need to be functional dependencies of the arguments.
 	if (aggr.distinct && order_bys) {
-		column_binding_set_t child_dependencies;
-		bool children_volatile = false;
-		for (const auto &child : children) {
-			children_volatile |= ExtractFunctionalDependencies(child_dependencies, child);
-		}
-
-		column_binding_set_t order_dependencies;
-		bool order_volatile = false;
+		bool in_args = true;
 		for (const auto &order_by : order_bys->orders) {
-			order_volatile |= ExtractFunctionalDependencies(order_dependencies, order_by.expression);
-		}
-
-		bool in_args = !children_volatile && !order_volatile;
-		if (in_args) {
-			for (const auto &binding : order_dependencies) {
-				if (!child_dependencies.count(binding)) {
-					in_args = false;
-					break;
-				}
-			}
+			in_args &= IsFunctionallyDependent(order_by.expression, children);
 		}
 
 		if (!in_args) {


Test patch:
diff --git a/test/sql/function/list/list_sort.test b/test/sql/function/list/list_sort.test
index 8389e025cb5a..6fd05052aa81 100644
--- a/test/sql/function/list/list_sort.test
+++ b/test/sql/function/list/list_sort.test
@@ -460,6 +460,12 @@ order by id;
 ----
 Binder Error: In a DISTINCT aggregate, ORDER BY expressions must appear in the argument list
 
+statement error
+select list(distinct v % 2 order by v) c2 
+from generate_series(20) s(v, i);
+----
+Binder Error: In a DISTINCT aggregate, ORDER BY expressions must appear in the argument list
+
 # bug fixes test for #5694
 
 require parquet