On Wed, Jun 5, 2024 at 5:42 PM Richard Guo <guofengli...@gmail.com> wrote: > I found a bug in the v6 patch. The following query would trigger the > Assert in make_restrictinfo that the given subexpression should not be > an AND clause. > > select max(a) from t group by a > b and a = b having a > b and a = b; > > This is because the expression 'a > b and a = b' in the HAVING clause is > replaced by a Var that references the GROUP RTE. When we preprocess the > columns of the GROUP RTE, we do not know whether the grouped expression > is a havingQual or not, so we do not perform make_ands_implicit for it. > As a result, after we replace the group Var in the HAVING clause with > the underlying grouping expression, we will have a havingQual that is an > AND clause. > > As we know, in the planner we need to first preprocess all the columns > of the GROUP RTE. We also need to replace any Vars in the targetlist > and HAVING clause that reference the GROUP RTE with the underlying > grouping expressions. To fix the mentioned issue, I choose the perform > this replacement before we preprocess the targetlist and havingQual, so > that the make_ands_implicit would be performed when we preprocess the > havingQual.
I've realized that there is something wrong with this conclusion. If we perform the replacement of GROUP Vars with the underlying grouping expressions before we've done with expression preprocessing on targetlist and havingQual, we may end up with failing to match the expressions that are part of grouping items to lower target items. Consider: create table t (a int, b int); insert into t values (1, 2); select a < b and b < 3 from t group by rollup(a < b and b < 3) having a < b and b < 3; The expression preprocessing process would convert the HAVING clause to implicit-AND format and thus it would fail to be matched to lower target items. Another example is: create table t1 (a boolean); insert into t1 values (true); select not a from t1 group by rollup(not a) having not not a; This HAVING clause 'not not a' would be reduced to 'a' and thus fail to be matched to lower tlist. I fixed this issue in v13 by performing the replacement of GROUP Vars after we've done with expression preprocessing on targetlist and havingQual. An ensuing effect of this approach is that a HAVING clause may contain expressions that are not fully preprocessed if they are part of grouping items. This is not an issue as long as the clause remains in HAVING. But if the clause is moved or copied into WHERE, we need to re-preprocess these expressions. Please see the attached for the changes. Thanks Richard
v13-0001-Introduce-an-RTE-for-the-grouping-step.patch
Description: Binary data
v13-0002-Mark-expressions-nullable-by-grouping-sets.patch
Description: Binary data
v13-0003-Unwrap-a-PlaceHolderVar-when-safe.patch
Description: Binary data
