Hi, I noticed that in the flurry of LLM-driven bug hunting, Richard's work in this release has remained relatively lightly impacted, which is probably a testament to him having done a good job with the work. However, I thought it would be a good idea to probe for problems and unfortunately Claude was able to find a few. Three of the four findings are just bugs; they need to be fixed, but they're not really a big deal. The fourth one is much more debatable: it's not a bug, but a question about whether the eager aggregation patch implements correct behavior.
Here's the problem: in our normal mental model of how queries work, aggregation happens after joins. The whole idea of eager aggregation is to do part of the aggregation work before performing all of the joins. Normally, this is invisible to the user, except to the extent that it influences query performance. But when some operator or function involved in the query has side effects, the change becomes user-visible. Here is an example: Setup: CREATE TABLE t1 (a int, b int); CREATE TABLE t2 (b int, c int); INSERT INTO t1 SELECT i, i FROM generate_series(1, 100) i; INSERT INTO t2 SELECT i % 10, CASE WHEN i % 10 = 0 THEN 0 ELSE i END FROM generate_series(1, 1000) i; ANALYZE t1, t2; Test query: SELECT t1.a, sum(100 / t2.c) FROM t1 JOIN t2 ON t1.b = t2.b GROUP BY t1.a; Without eager aggregation, the division operator is only applied to rows that survive the join, so the query completes successfully. With eager aggregation, the operator can be applied to rows that won't end up finding a join partner, so the query errors out with "ERROR: division by zero". This runs contrary to my mental model, and also to the statements on the "SELECT" reference page that aggregation happens after joins: step 3 eliminates rows that do not match the WHERE clause, and grouping happens in step 4. I think you can make an argument that what this page describes is not a totally absolute categorization scheme, and/or that it only covers results and not side effects, but in general we do mostly follow that ordering from the point of user-visible effects, and here we don't. 3a08a2a8b4fd36a9fa0da0253d1ca053c19047d5 established the precedent that we shouldn't alter the execution count of functions by pushing them below a join, but erred in believing that only volatile functions were an issue. Hence, while I don't think it's a complete slam-dunk that this behavior is definitively and undeniably wrong, I do think that it is generally contrary to the query behavior we usually try to deliver. I attach the SQL scripts for the other problems. finding1.sql produces the wrong answer and finding2.sql and finding3.sql produce internal errors. -- Robert Haas
finding1.sql
Description: Binary data
finding2.sql
Description: Binary data
finding3.sql
Description: Binary data
