Fix describe (#546)

Author: evertlammerts

external/duckdb

@@ -1,5 +1,6 @@
 #include "duckdb_python/pyconnection/pyconnection.hpp"
 
+#include "duckdb/catalog/catalog.hpp"
 #include "duckdb/common/arrow/arrow.hpp"
 #include "duckdb/common/types.hpp"
 #include "duckdb/common/types/vector.hpp"

src/pyconnection.cpp

@@ -287,10 +287,12 @@ struct DescribeAggregateInfo {
 	bool numeric_only;
 };
 
-vector<string> CreateExpressionList(const vector<ColumnDefinition> &columns,
-                                    const vector<DescribeAggregateInfo> &aggregates) {
-	vector<string> expressions;
-	expressions.reserve(columns.size());
+// Build the Describe query as two levels. The inner single-group aggregate computes one
+// scalar per (column, aggregate); the outer projection pivots them into rows via parallel
+// UNNESTs. This avoids GROUP BY ALL, which core rejects for UNNEST/UNLIST expressions.
+void CreateDescribeExpressions(const vector<ColumnDefinition> &columns, const vector<DescribeAggregateInfo> &aggregates,
+                               vector<string> &inner_aggregates, vector<string> &outer_projection) {
+	outer_projection.reserve(columns.size() + 1);
 
 	string aggr_names = "UNNEST([";
 	for (idx_t i = 0; i < aggregates.size(); i++) {
@@ -303,33 +305,35 @@ vector<string> CreateExpressionList(const vector<ColumnDefinition> &columns,
 	}
 	aggr_names += "])";
 	aggr_names += " AS aggr";
-	expressions.push_back(aggr_names);
+	outer_projection.push_back(aggr_names);
+
 	for (idx_t c = 0; c < columns.size(); c++) {
 		auto &col = columns[c];
-		string expr = "UNNEST([";
+		bool numeric = col.GetType().IsNumeric();
+		string unnest = "UNNEST([";
 		for (idx_t i = 0; i < aggregates.size(); i++) {
+			auto alias = "__describe_" + std::to_string(c) + "_" + std::to_string(i);
 			if (i > 0) {
-				expr += ", ";
-			}
-			if (aggregates[i].numeric_only && !col.GetType().IsNumeric()) {
-				expr += "NULL";
-				continue;
+				unnest += ", ";
 			}
-			expr += aggregates[i].name;
-			expr += "(";
-			expr += SQLIdentifier(col.GetName());
-			expr += ")";
-			if (col.GetType().IsNumeric()) {
-				expr += "::DOUBLE";
+			unnest += SQLIdentifier(alias);
+			string stat;
+			if (aggregates[i].numeric_only && !numeric) {
+				stat = "NULL";
 			} else {
-				expr += "::VARCHAR";
+				stat = aggregates[i].name;
+				stat += "(";
+				stat += SQLIdentifier(col.GetName());
+				stat += ")";
+				stat += numeric ? "::DOUBLE" : "::VARCHAR";
 			}
+			stat += " AS " + SQLIdentifier(alias);
+			inner_aggregates.push_back(stat);
 		}
-		expr += "])";
-		expr += " AS " + SQLIdentifier(col.GetName());
-		expressions.push_back(expr);
+		unnest += "])";
+		unnest += " AS " + SQLIdentifier(col.GetName());
+		outer_projection.push_back(unnest);
 	}
-	return expressions;
 }
 
 std::unique_ptr<DuckDBPyRelation> DuckDBPyRelation::Describe() {
@@ -338,8 +342,11 @@ std::unique_ptr<DuckDBPyRelation> DuckDBPyRelation::Describe() {
 	aggregates = {DescribeAggregateInfo("count"),        DescribeAggregateInfo("mean", true),
 	              DescribeAggregateInfo("stddev", true), DescribeAggregateInfo("min"),
 	              DescribeAggregateInfo("max"),          DescribeAggregateInfo("median", true)};
-	auto expressions = CreateExpressionList(columns, aggregates);
-	return DeriveRelation(rel->Aggregate(expressions));
+	vector<string> inner_aggregates;
+	vector<string> outer_projection;
+	CreateDescribeExpressions(columns, aggregates, inner_aggregates, outer_projection);
+	auto stats = rel->Aggregate(inner_aggregates);
+	return DeriveRelation(stats->Project(outer_projection));
 }
 
 string DuckDBPyRelation::ToSQL() {

src/pyrelation.cpp

@@ -747,35 +747,58 @@ def test_2145_parquet_glob_through_arrow(self, duckdb_cursor, tmp_path):
 # ===========================================================================
 
 
+# pyarrow view type filter pushdown canaries.
+# No released pyarrow fully executes view type filters (compare plus array_filter/take); checked
+# against 25.0.0. These stay xfail until one does. They run .to_table() so scanner construction
+# alone does not flip them: pyarrow accepts a binary_view predicate at build time but still fails
+# inside array_filter at execution. When one XPASSes: bump its version below, enable view type
+# filter pushdown, and drop the matching TestUnsupportedTypes fallback. See the view type filter
+# pushdown tracking issue.
+_PA_VERSION = Version(pa.__version__)
+_PYARROW_STRING_VIEW_FILTER_VERSION = Version("99")
+_PYARROW_BINARY_VIEW_FILTER_VERSION = Version("99")
+
+
 class TestCanaries:
     """Markers for behaviours we expect to change upstream eventually."""
 
     # ----- pyarrow capabilities ----------------------------------------
 
     @pytest.mark.xfail(
+        _PA_VERSION < _PYARROW_STRING_VIEW_FILTER_VERSION,
         raises=pa_lib.ArrowNotImplementedError,
-        reason="pyarrow does not yet implement string_view filter compare kernels",
+        reason="pyarrow does not execute string_view filters fully (equal kernel)",
         strict=True,
     )
     def test_pyarrow_gains_string_view_filter_support(self):
-        """When pyarrow adds string_view comparison kernels this will xpass.
+        """XPASSes when pyarrow executes a string_view filter fully.
 
-        At that point we should remove the post-scan fallback in TestUnsupportedTypes.
+        Runs .to_table() so it reflects execution, not just scanner construction. When it
+        XPASSes: bump the version, enable view type pushdown, drop the TestUnsupportedTypes
+        fallback.
         """
         filter_expr = pa_ds.field("col") == pa_ds.scalar("val1")
         table = pa.table({"col": pa.array(["val1", "val2"], type=pa.string_view())})
-        pa_ds.dataset(table).scanner(columns=["col"], filter=filter_expr)
+        result = pa_ds.dataset(table).scanner(columns=["col"], filter=filter_expr).to_table()
+        assert result.num_rows == 1
 
     @pytest.mark.xfail(
+        _PA_VERSION < _PYARROW_BINARY_VIEW_FILTER_VERSION,
         raises=pa_lib.ArrowNotImplementedError,
-        reason="pyarrow does not yet implement binary_view filter compare kernels",
+        reason="pyarrow does not execute binary_view filters fully (array_filter kernel)",
         strict=True,
     )
     def test_pyarrow_gains_binary_view_filter_support(self):
-        """When pyarrow adds binary_view comparison kernels this will xpass."""
+        """XPASSes when pyarrow executes a binary_view filter fully.
+
+        Runs .to_table() so it reflects execution, not just scanner construction. When it
+        XPASSes: bump the version, enable view type pushdown, drop the TestUnsupportedTypes
+        fallback.
+        """
         filter_expr = pa_ds.field("col") == pa_ds.scalar(pa.scalar(b"bin1", pa.binary_view()))
         table = pa.table({"col": pa.array([b"bin1", b"bin2"], type=pa.binary_view())})
-        pa_ds.dataset(table).scanner(columns=["col"], filter=filter_expr)
+        result = pa_ds.dataset(table).scanner(columns=["col"], filter=filter_expr).to_table()
+        assert result.num_rows == 1
 
     # ----- DuckDB optimizer decisions we expect to change --------------