agora inbox for [email protected]help / color / mirror / Atom feed
[PATCH 15/18] doc review for multiranges 28+ messages / 3 participants [nested] [flat]
* [PATCH 15/18] doc review for multiranges @ 2021-01-24 03:02 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 28+ messages in thread From: Justin Pryzby @ 2021-01-24 03:02 UTC (permalink / raw) 6df7a9698bb036610c1e8c6d375e1be38cb26d5f --- doc/src/sgml/extend.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 6e3d82b85b..ec95b4eb01 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -448,7 +448,7 @@ of <type>anycompatible</type> and <type>anycompatiblenonarray</type> inputs, the array element types of <type>anycompatiblearray</type> inputs, the range subtypes of <type>anycompatiblerange</type> inputs, - and the multirange subtypes of <type>anycompatiablemultirange</type> + and the multirange subtypes of <type>anycompatiblemultirange</type> inputs. If <type>anycompatiblenonarray</type> is present then the common type is required to be a non-array type. Once a common type is identified, arguments in <type>anycompatible</type> -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0016-doc-review-for-logical-decoding-of-prepared-xacts.patch" ^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH 15/18] doc review for multiranges @ 2021-01-24 03:02 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 28+ messages in thread From: Justin Pryzby @ 2021-01-24 03:02 UTC (permalink / raw) 6df7a9698bb036610c1e8c6d375e1be38cb26d5f --- doc/src/sgml/extend.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 6e3d82b85b..ec95b4eb01 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -448,7 +448,7 @@ of <type>anycompatible</type> and <type>anycompatiblenonarray</type> inputs, the array element types of <type>anycompatiblearray</type> inputs, the range subtypes of <type>anycompatiblerange</type> inputs, - and the multirange subtypes of <type>anycompatiablemultirange</type> + and the multirange subtypes of <type>anycompatiblemultirange</type> inputs. If <type>anycompatiblenonarray</type> is present then the common type is required to be a non-array type. Once a common type is identified, arguments in <type>anycompatible</type> -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0016-doc-review-for-logical-decoding-of-prepared-xacts.patch" ^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH 15/18] doc review for multiranges @ 2021-01-24 03:02 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 28+ messages in thread From: Justin Pryzby @ 2021-01-24 03:02 UTC (permalink / raw) 6df7a9698bb036610c1e8c6d375e1be38cb26d5f --- doc/src/sgml/extend.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 6e3d82b85b..ec95b4eb01 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -448,7 +448,7 @@ of <type>anycompatible</type> and <type>anycompatiblenonarray</type> inputs, the array element types of <type>anycompatiblearray</type> inputs, the range subtypes of <type>anycompatiblerange</type> inputs, - and the multirange subtypes of <type>anycompatiablemultirange</type> + and the multirange subtypes of <type>anycompatiblemultirange</type> inputs. If <type>anycompatiblenonarray</type> is present then the common type is required to be a non-array type. Once a common type is identified, arguments in <type>anycompatible</type> -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0016-doc-review-for-logical-decoding-of-prepared-xacts.patch" ^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH 15/18] doc review for multiranges @ 2021-01-24 03:02 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 28+ messages in thread From: Justin Pryzby @ 2021-01-24 03:02 UTC (permalink / raw) 6df7a9698bb036610c1e8c6d375e1be38cb26d5f --- doc/src/sgml/extend.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 6e3d82b85b..ec95b4eb01 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -448,7 +448,7 @@ of <type>anycompatible</type> and <type>anycompatiblenonarray</type> inputs, the array element types of <type>anycompatiblearray</type> inputs, the range subtypes of <type>anycompatiblerange</type> inputs, - and the multirange subtypes of <type>anycompatiablemultirange</type> + and the multirange subtypes of <type>anycompatiblemultirange</type> inputs. If <type>anycompatiblenonarray</type> is present then the common type is required to be a non-array type. Once a common type is identified, arguments in <type>anycompatible</type> -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0016-doc-review-for-logical-decoding-of-prepared-xacts.patch" ^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH 15/18] doc review for multiranges @ 2021-01-24 03:02 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 28+ messages in thread From: Justin Pryzby @ 2021-01-24 03:02 UTC (permalink / raw) 6df7a9698bb036610c1e8c6d375e1be38cb26d5f --- doc/src/sgml/extend.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 6e3d82b85b..ec95b4eb01 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -448,7 +448,7 @@ of <type>anycompatible</type> and <type>anycompatiblenonarray</type> inputs, the array element types of <type>anycompatiblearray</type> inputs, the range subtypes of <type>anycompatiblerange</type> inputs, - and the multirange subtypes of <type>anycompatiablemultirange</type> + and the multirange subtypes of <type>anycompatiblemultirange</type> inputs. If <type>anycompatiblenonarray</type> is present then the common type is required to be a non-array type. Once a common type is identified, arguments in <type>anycompatible</type> -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0016-doc-review-for-logical-decoding-of-prepared-xacts.patch" ^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH 15/18] doc review for multiranges @ 2021-01-24 03:02 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 28+ messages in thread From: Justin Pryzby @ 2021-01-24 03:02 UTC (permalink / raw) 6df7a9698bb036610c1e8c6d375e1be38cb26d5f --- doc/src/sgml/extend.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 6e3d82b85b..ec95b4eb01 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -448,7 +448,7 @@ of <type>anycompatible</type> and <type>anycompatiblenonarray</type> inputs, the array element types of <type>anycompatiblearray</type> inputs, the range subtypes of <type>anycompatiblerange</type> inputs, - and the multirange subtypes of <type>anycompatiablemultirange</type> + and the multirange subtypes of <type>anycompatiblemultirange</type> inputs. If <type>anycompatiblenonarray</type> is present then the common type is required to be a non-array type. Once a common type is identified, arguments in <type>anycompatible</type> -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0016-doc-review-for-logical-decoding-of-prepared-xacts.patch" ^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH 15/18] doc review for multiranges @ 2021-01-24 03:02 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 28+ messages in thread From: Justin Pryzby @ 2021-01-24 03:02 UTC (permalink / raw) 6df7a9698bb036610c1e8c6d375e1be38cb26d5f --- doc/src/sgml/extend.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 6e3d82b85b..ec95b4eb01 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -448,7 +448,7 @@ of <type>anycompatible</type> and <type>anycompatiblenonarray</type> inputs, the array element types of <type>anycompatiblearray</type> inputs, the range subtypes of <type>anycompatiblerange</type> inputs, - and the multirange subtypes of <type>anycompatiablemultirange</type> + and the multirange subtypes of <type>anycompatiblemultirange</type> inputs. If <type>anycompatiblenonarray</type> is present then the common type is required to be a non-array type. Once a common type is identified, arguments in <type>anycompatible</type> -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0016-doc-review-for-logical-decoding-of-prepared-xacts.patch" ^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH 15/18] doc review for multiranges @ 2021-01-24 03:02 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 28+ messages in thread From: Justin Pryzby @ 2021-01-24 03:02 UTC (permalink / raw) 6df7a9698bb036610c1e8c6d375e1be38cb26d5f --- doc/src/sgml/extend.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 6e3d82b85b..ec95b4eb01 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -448,7 +448,7 @@ of <type>anycompatible</type> and <type>anycompatiblenonarray</type> inputs, the array element types of <type>anycompatiblearray</type> inputs, the range subtypes of <type>anycompatiblerange</type> inputs, - and the multirange subtypes of <type>anycompatiablemultirange</type> + and the multirange subtypes of <type>anycompatiblemultirange</type> inputs. If <type>anycompatiblenonarray</type> is present then the common type is required to be a non-array type. Once a common type is identified, arguments in <type>anycompatible</type> -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0016-doc-review-for-logical-decoding-of-prepared-xacts.patch" ^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH 15/18] doc review for multiranges @ 2021-01-24 03:02 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 28+ messages in thread From: Justin Pryzby @ 2021-01-24 03:02 UTC (permalink / raw) 6df7a9698bb036610c1e8c6d375e1be38cb26d5f --- doc/src/sgml/extend.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 6e3d82b85b..ec95b4eb01 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -448,7 +448,7 @@ of <type>anycompatible</type> and <type>anycompatiblenonarray</type> inputs, the array element types of <type>anycompatiblearray</type> inputs, the range subtypes of <type>anycompatiblerange</type> inputs, - and the multirange subtypes of <type>anycompatiablemultirange</type> + and the multirange subtypes of <type>anycompatiblemultirange</type> inputs. If <type>anycompatiblenonarray</type> is present then the common type is required to be a non-array type. Once a common type is identified, arguments in <type>anycompatible</type> -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0016-doc-review-for-logical-decoding-of-prepared-xacts.patch" ^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH 15/18] doc review for multiranges @ 2021-01-24 03:02 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 28+ messages in thread From: Justin Pryzby @ 2021-01-24 03:02 UTC (permalink / raw) 6df7a9698bb036610c1e8c6d375e1be38cb26d5f --- doc/src/sgml/extend.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 6e3d82b85b..ec95b4eb01 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -448,7 +448,7 @@ of <type>anycompatible</type> and <type>anycompatiblenonarray</type> inputs, the array element types of <type>anycompatiblearray</type> inputs, the range subtypes of <type>anycompatiblerange</type> inputs, - and the multirange subtypes of <type>anycompatiablemultirange</type> + and the multirange subtypes of <type>anycompatiblemultirange</type> inputs. If <type>anycompatiblenonarray</type> is present then the common type is required to be a non-array type. Once a common type is identified, arguments in <type>anycompatible</type> -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0016-doc-review-for-logical-decoding-of-prepared-xacts.patch" ^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH 15/18] doc review for multiranges @ 2021-01-24 03:02 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 28+ messages in thread From: Justin Pryzby @ 2021-01-24 03:02 UTC (permalink / raw) 6df7a9698bb036610c1e8c6d375e1be38cb26d5f --- doc/src/sgml/extend.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 6e3d82b85b..ec95b4eb01 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -448,7 +448,7 @@ of <type>anycompatible</type> and <type>anycompatiblenonarray</type> inputs, the array element types of <type>anycompatiblearray</type> inputs, the range subtypes of <type>anycompatiblerange</type> inputs, - and the multirange subtypes of <type>anycompatiablemultirange</type> + and the multirange subtypes of <type>anycompatiblemultirange</type> inputs. If <type>anycompatiblenonarray</type> is present then the common type is required to be a non-array type. Once a common type is identified, arguments in <type>anycompatible</type> -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0016-doc-review-for-logical-decoding-of-prepared-xacts.patch" ^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH 15/18] doc review for multiranges @ 2021-01-24 03:02 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 28+ messages in thread From: Justin Pryzby @ 2021-01-24 03:02 UTC (permalink / raw) 6df7a9698bb036610c1e8c6d375e1be38cb26d5f --- doc/src/sgml/extend.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 6e3d82b85b..ec95b4eb01 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -448,7 +448,7 @@ of <type>anycompatible</type> and <type>anycompatiblenonarray</type> inputs, the array element types of <type>anycompatiblearray</type> inputs, the range subtypes of <type>anycompatiblerange</type> inputs, - and the multirange subtypes of <type>anycompatiablemultirange</type> + and the multirange subtypes of <type>anycompatiblemultirange</type> inputs. If <type>anycompatiblenonarray</type> is present then the common type is required to be a non-array type. Once a common type is identified, arguments in <type>anycompatible</type> -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0016-doc-review-for-logical-decoding-of-prepared-xacts.patch" ^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH 15/18] doc review for multiranges @ 2021-01-24 03:02 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 28+ messages in thread From: Justin Pryzby @ 2021-01-24 03:02 UTC (permalink / raw) 6df7a9698bb036610c1e8c6d375e1be38cb26d5f --- doc/src/sgml/extend.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 6e3d82b85b..ec95b4eb01 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -448,7 +448,7 @@ of <type>anycompatible</type> and <type>anycompatiblenonarray</type> inputs, the array element types of <type>anycompatiblearray</type> inputs, the range subtypes of <type>anycompatiblerange</type> inputs, - and the multirange subtypes of <type>anycompatiablemultirange</type> + and the multirange subtypes of <type>anycompatiblemultirange</type> inputs. If <type>anycompatiblenonarray</type> is present then the common type is required to be a non-array type. Once a common type is identified, arguments in <type>anycompatible</type> -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0016-doc-review-for-logical-decoding-of-prepared-xacts.patch" ^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH 15/18] doc review for multiranges @ 2021-01-24 03:02 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 28+ messages in thread From: Justin Pryzby @ 2021-01-24 03:02 UTC (permalink / raw) 6df7a9698bb036610c1e8c6d375e1be38cb26d5f --- doc/src/sgml/extend.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 6e3d82b85b..ec95b4eb01 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -448,7 +448,7 @@ of <type>anycompatible</type> and <type>anycompatiblenonarray</type> inputs, the array element types of <type>anycompatiblearray</type> inputs, the range subtypes of <type>anycompatiblerange</type> inputs, - and the multirange subtypes of <type>anycompatiablemultirange</type> + and the multirange subtypes of <type>anycompatiblemultirange</type> inputs. If <type>anycompatiblenonarray</type> is present then the common type is required to be a non-array type. Once a common type is identified, arguments in <type>anycompatible</type> -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0016-doc-review-for-logical-decoding-of-prepared-xacts.patch" ^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH 15/18] doc review for multiranges @ 2021-01-24 03:02 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 28+ messages in thread From: Justin Pryzby @ 2021-01-24 03:02 UTC (permalink / raw) 6df7a9698bb036610c1e8c6d375e1be38cb26d5f --- doc/src/sgml/extend.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 6e3d82b85b..ec95b4eb01 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -448,7 +448,7 @@ of <type>anycompatible</type> and <type>anycompatiblenonarray</type> inputs, the array element types of <type>anycompatiblearray</type> inputs, the range subtypes of <type>anycompatiblerange</type> inputs, - and the multirange subtypes of <type>anycompatiablemultirange</type> + and the multirange subtypes of <type>anycompatiblemultirange</type> inputs. If <type>anycompatiblenonarray</type> is present then the common type is required to be a non-array type. Once a common type is identified, arguments in <type>anycompatible</type> -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0016-doc-review-for-logical-decoding-of-prepared-xacts.patch" ^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH 15/21] doc review for multiranges @ 2021-01-24 03:02 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 28+ messages in thread From: Justin Pryzby @ 2021-01-24 03:02 UTC (permalink / raw) 6df7a9698bb036610c1e8c6d375e1be38cb26d5f --- doc/src/sgml/extend.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 6e3d82b85b..ec95b4eb01 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -448,7 +448,7 @@ of <type>anycompatible</type> and <type>anycompatiblenonarray</type> inputs, the array element types of <type>anycompatiblearray</type> inputs, the range subtypes of <type>anycompatiblerange</type> inputs, - and the multirange subtypes of <type>anycompatiablemultirange</type> + and the multirange subtypes of <type>anycompatiblemultirange</type> inputs. If <type>anycompatiblenonarray</type> is present then the common type is required to be a non-array type. Once a common type is identified, arguments in <type>anycompatible</type> -- 2.17.0 --jI8keyz6grp/JLjh Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0016-doc-review-for-logical-decoding-of-prepared-xacts.patch" ^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH 15/18] doc review for multiranges @ 2021-01-24 03:02 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 28+ messages in thread From: Justin Pryzby @ 2021-01-24 03:02 UTC (permalink / raw) 6df7a9698bb036610c1e8c6d375e1be38cb26d5f --- doc/src/sgml/extend.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 6e3d82b85b..ec95b4eb01 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -448,7 +448,7 @@ of <type>anycompatible</type> and <type>anycompatiblenonarray</type> inputs, the array element types of <type>anycompatiblearray</type> inputs, the range subtypes of <type>anycompatiblerange</type> inputs, - and the multirange subtypes of <type>anycompatiablemultirange</type> + and the multirange subtypes of <type>anycompatiblemultirange</type> inputs. If <type>anycompatiblenonarray</type> is present then the common type is required to be a non-array type. Once a common type is identified, arguments in <type>anycompatible</type> -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0016-doc-review-for-logical-decoding-of-prepared-xacts.patch" ^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH 15/18] doc review for multiranges @ 2021-01-24 03:02 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 28+ messages in thread From: Justin Pryzby @ 2021-01-24 03:02 UTC (permalink / raw) 6df7a9698bb036610c1e8c6d375e1be38cb26d5f --- doc/src/sgml/extend.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 6e3d82b85b..ec95b4eb01 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -448,7 +448,7 @@ of <type>anycompatible</type> and <type>anycompatiblenonarray</type> inputs, the array element types of <type>anycompatiblearray</type> inputs, the range subtypes of <type>anycompatiblerange</type> inputs, - and the multirange subtypes of <type>anycompatiablemultirange</type> + and the multirange subtypes of <type>anycompatiblemultirange</type> inputs. If <type>anycompatiblenonarray</type> is present then the common type is required to be a non-array type. Once a common type is identified, arguments in <type>anycompatible</type> -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0016-doc-review-for-logical-decoding-of-prepared-xacts.patch" ^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH 15/18] doc review for multiranges @ 2021-01-24 03:02 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 28+ messages in thread From: Justin Pryzby @ 2021-01-24 03:02 UTC (permalink / raw) 6df7a9698bb036610c1e8c6d375e1be38cb26d5f --- doc/src/sgml/extend.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 6e3d82b85b..ec95b4eb01 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -448,7 +448,7 @@ of <type>anycompatible</type> and <type>anycompatiblenonarray</type> inputs, the array element types of <type>anycompatiblearray</type> inputs, the range subtypes of <type>anycompatiblerange</type> inputs, - and the multirange subtypes of <type>anycompatiablemultirange</type> + and the multirange subtypes of <type>anycompatiblemultirange</type> inputs. If <type>anycompatiblenonarray</type> is present then the common type is required to be a non-array type. Once a common type is identified, arguments in <type>anycompatible</type> -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0016-doc-review-for-logical-decoding-of-prepared-xacts.patch" ^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH 15/18] doc review for multiranges @ 2021-01-24 03:02 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 28+ messages in thread From: Justin Pryzby @ 2021-01-24 03:02 UTC (permalink / raw) 6df7a9698bb036610c1e8c6d375e1be38cb26d5f --- doc/src/sgml/extend.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 6e3d82b85b..ec95b4eb01 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -448,7 +448,7 @@ of <type>anycompatible</type> and <type>anycompatiblenonarray</type> inputs, the array element types of <type>anycompatiblearray</type> inputs, the range subtypes of <type>anycompatiblerange</type> inputs, - and the multirange subtypes of <type>anycompatiablemultirange</type> + and the multirange subtypes of <type>anycompatiblemultirange</type> inputs. If <type>anycompatiblenonarray</type> is present then the common type is required to be a non-array type. Once a common type is identified, arguments in <type>anycompatible</type> -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0016-doc-review-for-logical-decoding-of-prepared-xacts.patch" ^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH 15/18] doc review for multiranges @ 2021-01-24 03:02 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 28+ messages in thread From: Justin Pryzby @ 2021-01-24 03:02 UTC (permalink / raw) 6df7a9698bb036610c1e8c6d375e1be38cb26d5f --- doc/src/sgml/extend.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 6e3d82b85b..ec95b4eb01 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -448,7 +448,7 @@ of <type>anycompatible</type> and <type>anycompatiblenonarray</type> inputs, the array element types of <type>anycompatiblearray</type> inputs, the range subtypes of <type>anycompatiblerange</type> inputs, - and the multirange subtypes of <type>anycompatiablemultirange</type> + and the multirange subtypes of <type>anycompatiblemultirange</type> inputs. If <type>anycompatiblenonarray</type> is present then the common type is required to be a non-array type. Once a common type is identified, arguments in <type>anycompatible</type> -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0016-doc-review-for-logical-decoding-of-prepared-xacts.patch" ^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH 15/18] doc review for multiranges @ 2021-01-24 03:02 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 28+ messages in thread From: Justin Pryzby @ 2021-01-24 03:02 UTC (permalink / raw) 6df7a9698bb036610c1e8c6d375e1be38cb26d5f --- doc/src/sgml/extend.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 6e3d82b85b..ec95b4eb01 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -448,7 +448,7 @@ of <type>anycompatible</type> and <type>anycompatiblenonarray</type> inputs, the array element types of <type>anycompatiblearray</type> inputs, the range subtypes of <type>anycompatiblerange</type> inputs, - and the multirange subtypes of <type>anycompatiablemultirange</type> + and the multirange subtypes of <type>anycompatiblemultirange</type> inputs. If <type>anycompatiblenonarray</type> is present then the common type is required to be a non-array type. Once a common type is identified, arguments in <type>anycompatible</type> -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0016-doc-review-for-logical-decoding-of-prepared-xacts.patch" ^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH 15/18] doc review for multiranges @ 2021-01-24 03:02 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 28+ messages in thread From: Justin Pryzby @ 2021-01-24 03:02 UTC (permalink / raw) 6df7a9698bb036610c1e8c6d375e1be38cb26d5f --- doc/src/sgml/extend.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 6e3d82b85b..ec95b4eb01 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -448,7 +448,7 @@ of <type>anycompatible</type> and <type>anycompatiblenonarray</type> inputs, the array element types of <type>anycompatiblearray</type> inputs, the range subtypes of <type>anycompatiblerange</type> inputs, - and the multirange subtypes of <type>anycompatiablemultirange</type> + and the multirange subtypes of <type>anycompatiblemultirange</type> inputs. If <type>anycompatiblenonarray</type> is present then the common type is required to be a non-array type. Once a common type is identified, arguments in <type>anycompatible</type> -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0016-doc-review-for-logical-decoding-of-prepared-xacts.patch" ^ permalink raw reply [nested|flat] 28+ messages in thread
* Wrong results with grouping sets @ 2023-09-25 07:11 Richard Guo <[email protected]> 2023-10-07 10:29 ` Re: Wrong results with grouping sets Richard Guo <[email protected]> 2023-11-16 15:25 ` Re: Wrong results with grouping sets Alena Rybakina <[email protected]> 2023-12-07 08:22 ` Re: Wrong results with grouping sets Richard Guo <[email protected]> 0 siblings, 3 replies; 28+ messages in thread From: Richard Guo @ 2023-09-25 07:11 UTC (permalink / raw) To: pgsql-hackers I think I've come across a wrong result issue with grouping sets, as shown by the query below. -- result is correct with only grouping sets select a, b from (values (1, 1), (2, 2)) as t (a, b) where a = b group by grouping sets((a, b), (a)); a | b ---+--- 1 | 1 1 | 2 | 2 2 | (4 rows) -- result is NOT correct with grouping sets and distinct on select distinct on (a, b) a, b from (values (1, 1), (2, 2)) as t (a, b) where a = b group by grouping sets((a, b), (a)); a | b ---+--- 1 | 1 2 | 2 (2 rows) The distinct on expressions include both 'a' and 'b', so rows (1, 1) and (1, NULL) should not be considered equal. (The same for rows (2, 2) and (2, NULL).) I think the root cause is that when we generate distinct_pathkeys, we failed to realize that Var 'b' might be nullable by the grouping sets, so it's no longer always equal to Var 'a'. It's not correct to deem that the PathKey for 'b' is redundant and thus remove it from the pathkeys list. We have the same issue when generating sort_pathkeys. As a result, we may have the final output in the wrong order. There were several reports about this issue before, such as [1][2]. To fix this issue, I'm thinking that we mark the grouping expressions nullable by grouping sets with a dummy RTE for grouping sets, something like attached. In practice we do not need to create a real RTE for that, what we need is just a RT index. In the patch I use 0, because it's not a valid outer join relid, so it would not conflict with the outer-join-aware-Var infrastructure. If the grouping expression is a Var or PHV, we can just set its nullingrels, very straightforward. For an expression that is neither a Var nor a PHV, I'm not quite sure how to set the nullingrels. I tried the idea of wrapping it in a new PHV to carry the nullingrels, but that may cause some unnecessary plan diffs. In the patch for such an expression I just set the nullingrels of Vars or PHVs that are contained in it. This is not really 'correct' in theory, because it is the whole expression that can be nullable by grouping sets, not its individual vars. But it works in practice, because what we need is that the expression can be somehow distinguished from the same expression in ECs, and marking its vars is sufficient for this purpose. But what if the expression is variable-free? This is the point that needs more work. Fow now the patch just handles variable-free expressions of type Const, by wrapping it in a new PHV. There are some places where we need to artificially remove the RT index of grouping sets from the nullingrels, such as when we generate PathTarget for initial input to grouping nodes, or when we generate PathKeys for the grouping clauses, because the expressions there are logically below the grouping sets. We also need to do that in set_upper_references when we update the targetlist of an Agg node, so that we can perform exact match for nullingrels, rather than superset match. Since the fix depends on the nullingrels stuff, it seems not easy for back-patching. I'm not sure what we should do in back branches. Any thoughts? [1] https://www.postgresql.org/message-id/[email protected]... [2] https://www.postgresql.org/message-id/[email protected] Thanks Richard Attachments: [application/octet-stream] v1-0001-Mark-expressions-nullable-by-grouping-sets.patch (22.3K, ../../CAMbWs4_dp7e7oTwaiZeBX8+P1rXw4ThkZxh1QG81rhu9Z47VsQ@mail.gmail.com/3-v1-0001-Mark-expressions-nullable-by-grouping-sets.patch) download | inline diff: From f962e6d110c91e1ba615059eb74c37d633ac1824 Mon Sep 17 00:00:00 2001 From: Richard Guo <[email protected]> Date: Mon, 25 Sep 2023 09:39:18 +0800 Subject: [PATCH v1] Mark expressions nullable by grouping sets When generating window_pathkeys, distinct_pathkeys or sort_pathkeys, we failed to realize that the grouping/ordering expressions might be nullable by grouping sets. As a result, we may incorrectly deem that the PathKeys are redundant by EquivalenceClass processing and thus remove them from the pathkeys list. That would lead to wrong results in some cases. To fix it, mark expressions nullable by grouping sets with a dummy RTE. --- src/backend/optimizer/path/pathkeys.c | 11 ++ src/backend/optimizer/plan/planner.c | 156 +++++++++++++++++++- src/backend/optimizer/plan/setrefs.c | 20 ++- src/backend/optimizer/prep/prepjointree.c | 1 + src/include/nodes/parsenodes.h | 11 ++ src/include/nodes/pathnodes.h | 7 + src/include/optimizer/paths.h | 1 + src/test/regress/expected/groupingsets.out | 164 +++++++++++++++++---- src/test/regress/sql/groupingsets.sql | 38 +++++ 9 files changed, 379 insertions(+), 30 deletions(-) diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c index fdb60aaa8d..1637bc64de 100644 --- a/src/backend/optimizer/path/pathkeys.c +++ b/src/backend/optimizer/path/pathkeys.c @@ -26,6 +26,7 @@ #include "optimizer/pathnode.h" #include "optimizer/paths.h" #include "partitioning/partbounds.h" +#include "rewrite/rewriteManip.h" #include "utils/lsyscache.h" @@ -1141,6 +1142,7 @@ make_pathkeys_for_sortclauses(PlannerInfo *root, &sortclauses, tlist, false, + false, &sortable); /* It's caller error if not all clauses were sortable */ Assert(sortable); @@ -1158,6 +1160,9 @@ make_pathkeys_for_sortclauses(PlannerInfo *root, * give rise to redundant pathkeys are removed from the sortclauses list * (which therefore must be pass-by-reference in this version). * + * If remove_grouping_set_rtindex is true, then we need to remove the RT index + * of grouping sets from the sort exprs before we make PathKeys for them. + * * *sortable is set to true if all the sort clauses are in fact sortable. * If any are not, they are ignored except for setting *sortable false. * (In that case, the output pathkey list isn't really useful. However, @@ -1171,6 +1176,7 @@ make_pathkeys_for_sortclauses_extended(PlannerInfo *root, List **sortclauses, List *tlist, bool remove_redundant, + bool remove_grouping_set_rtindex, bool *sortable) { List *pathkeys = NIL; @@ -1189,6 +1195,11 @@ make_pathkeys_for_sortclauses_extended(PlannerInfo *root, *sortable = false; continue; } + if (remove_grouping_set_rtindex && + bms_is_member(sortcl->tleSortGroupRef, root->nullable_sortgroup_refs)) + sortkey = (Expr *) remove_nulling_relids((Node *) sortkey, + bms_make_singleton(GROUPING_SET_RTINDEX), + NULL); pathkey = make_pathkey_from_sortop(root, sortkey, sortcl->sortop, diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 44efb1f4eb..0c87046aee 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -50,6 +50,7 @@ #include "optimizer/paramassign.h" #include "optimizer/pathnode.h" #include "optimizer/paths.h" +#include "optimizer/placeholder.h" #include "optimizer/plancat.h" #include "optimizer/planmain.h" #include "optimizer/planner.h" @@ -133,6 +134,9 @@ static Node *preprocess_expression(PlannerInfo *root, Node *expr, int kind); static void preprocess_qual_conditions(PlannerInfo *root, Node *jtnode); static void grouping_planner(PlannerInfo *root, double tuple_fraction); static grouping_sets_data *preprocess_grouping_sets(PlannerInfo *root); +static void markNullableByGroupingSets(PlannerInfo *root); +static void markTargetEntryNullable(PlannerInfo *root, TargetEntry *tle, + int rtindex); static List *remap_to_groupclause_idx(List *groupClause, List *gsets, int *tleref_to_colnum_map); static void preprocess_rowmarks(PlannerInfo *root); @@ -641,6 +645,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse, root->plan_params = NIL; root->outer_params = NULL; root->planner_cxt = CurrentMemoryContext; + root->nullable_sortgroup_refs = NULL; root->init_plans = NIL; root->cte_plan_ids = NIL; root->multiexpr_params = NIL; @@ -2037,6 +2042,9 @@ preprocess_grouping_sets(PlannerInfo *root) gd->unsortable_refs = NULL; gd->unsortable_sets = NIL; + /* Mark the exprs in TargetEntrys nullable by the grouping sets */ + markNullableByGroupingSets(root); + /* * We don't currently make any attempt to optimize the groupClause when * there are grouping sets, so just duplicate it in processed_groupClause. @@ -2198,6 +2206,126 @@ preprocess_grouping_sets(PlannerInfo *root) return gd; } +/* + * If the expression of a TargetEntry is nullable by grouping sets, set its + * nullingrels to include the RT index of grouping sets. + * + * We also compute nullable_sortgroup_refs in this function. + */ +static void +markNullableByGroupingSets(PlannerInfo *root) +{ + Query *parse = root->parse; + ListCell *lc; + + /* nothing to do if there are no grouping sets */ + if (parse->groupingSets == NIL) + return; + + foreach(lc, parse->groupClause) + { + SortGroupClause *gc = lfirst_node(SortGroupClause, lc); + Index ref = gc->tleSortGroupRef; + ListCell *l; + + foreach(l, parse->groupingSets) + { + List *content = lfirst(l); + + /* + * The current TargetEntry is a group clause but not contained in + * this grouping set, so it would be nulled by the grouping sets. + * Mark it so. + */ + if (!list_member_int(content, ref)) + { + TargetEntry *tle; + + tle = get_sortgroupref_tle(ref, parse->targetList); + markTargetEntryNullable(root, tle, GROUPING_SET_RTINDEX); + + root->nullable_sortgroup_refs = + bms_add_member(root->nullable_sortgroup_refs, ref); + + break; + } + } + } +} + +/* + * Mark the expr of the given TargetEntry nullable by the 'rtindex'. + */ +static void +markTargetEntryNullable(PlannerInfo *root, TargetEntry *tle, int rtindex) +{ + Node *newnode; + + newnode = (Node *) copyObject(tle->expr); + if (IsA(newnode, Var)) + { + Var *var = (Var *) newnode; + + var->varnullingrels = bms_add_member(var->varnullingrels, rtindex); + } + else if (IsA(newnode, PlaceHolderVar)) + { + PlaceHolderVar *phv = (PlaceHolderVar *) newnode; + + phv->phnullingrels = bms_add_member(phv->phnullingrels, rtindex); + } + else + { + /* + * XXX Where should we add the nullingrels if tle->expr is neither a + * Var nor a PlaceHolderVar? Wrap it in a new PlaceHolderVar to carry + * the nullingrels? + */ + + Relids relids = pull_varnos(root, newnode); + + /* + * If tle->expr is not variable-free, we set the nullingrels of Vars or + * PHVs that are contained in the expr. This is not really 'correct' + * in theory, because it is the whole expression that can be nullable + * by grouping sets, not its individual vars. But it works in + * practice, because what we need is that the expression can be somehow + * distinguished from the same expression in ECs, and marking its vars + * is sufficient for this purpose. + */ + if (!bms_is_empty(relids)) + { + newnode = add_nulling_relids(newnode, + relids, + bms_make_singleton(rtindex)); + } + else /* variable-free? */ + { + /* + * For Const, we can wrap it in a new PlaceHolderVar to carry the + * nullingrels. + */ + if (IsA(newnode, Const)) + { + PlaceHolderVar *newphv; + Relids phrels = + get_relids_in_jointree((Node *) root->parse->jointree, + true, false); + + newphv = make_placeholder_expr(root, (Expr *) newnode, phrels); + newphv->phnullingrels = + bms_add_member(newphv->phnullingrels, rtindex); + newnode = (Node *) newphv; + } + /* + * TODO What if the variable-free expression is not Const? + */ + } + } + + tle->expr = (Expr *) newnode; +} + /* * Given a groupclause and a list of GroupingSetData, return equivalent sets * (without annotation) mapped to indexes into the given groupclause. @@ -3439,9 +3567,20 @@ standard_qp_callback(PlannerInfo *root, void *extra) if (grouping_is_sortable(groupClause)) { - root->group_pathkeys = make_pathkeys_for_sortclauses(root, - groupClause, - tlist); + bool sortable; + + /* + * Note that the groupClause is logically below the grouping sets. + * So set remove_grouping_set_rtindex to true. + */ + root->group_pathkeys = + make_pathkeys_for_sortclauses_extended(root, + &groupClause, + tlist, + false, + true, + &sortable); + Assert(sortable); root->num_groupby_pathkeys = list_length(root->group_pathkeys); } else @@ -3467,6 +3606,7 @@ standard_qp_callback(PlannerInfo *root, void *extra) &root->processed_groupClause, tlist, true, + false, &sortable); if (!sortable) { @@ -3517,6 +3657,7 @@ standard_qp_callback(PlannerInfo *root, void *extra) &root->processed_distinctClause, tlist, true, + false, &sortable); if (!sortable) root->distinct_pathkeys = NIL; @@ -5377,7 +5518,15 @@ make_group_input_target(PlannerInfo *root, PathTarget *final_target) { /* * It's a grouping column, so add it to the input target as-is. + * + * Note that the target is logically below the grouping sets. So + * if the grouping expr is nullable by grouping sets, we need to + * remove the RT index of grouping sets from its nullingrels. */ + if (bms_is_member(sgref, root->nullable_sortgroup_refs)) + expr = (Expr *) remove_nulling_relids((Node *) expr, + bms_make_singleton(GROUPING_SET_RTINDEX), + NULL); add_column_to_pathtarget(input_target, expr, sgref); } else @@ -6058,6 +6207,7 @@ make_pathkeys_for_window(PlannerInfo *root, WindowClause *wc, &wc->partitionClause, tlist, true, + false, &sortable); Assert(sortable); diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index 5700bfb5cd..0213cfcc42 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -26,6 +26,7 @@ #include "optimizer/subselect.h" #include "optimizer/tlist.h" #include "parser/parse_relation.h" +#include "rewrite/rewriteManip.h" #include "tcop/utility.h" #include "utils/lsyscache.h" #include "utils/syscache.h" @@ -2419,14 +2420,29 @@ set_upper_references(PlannerInfo *root, Plan *plan, int rtoffset) /* If it's a sort/group item, first try to match by sortref */ if (tle->ressortgroupref != 0) { + Expr *expr = tle->expr; + + /* + * If it's an Agg node for grouping sets, any Vars and PHVs + * appearing here should have nullingrels that include the effects + * of the grouping sets, ie they will have nullingrels equal to the + * input Vars/PHVs' nullingrels plus the RT index of grouping sets. + * In order to perform exact match, we can remove the RT index of + * grouping sets first. + */ + if (IsA(plan, Agg) && + bms_is_member(tle->ressortgroupref, root->nullable_sortgroup_refs)) + expr = (Expr *) remove_nulling_relids((Node *) expr, + bms_make_singleton(GROUPING_SET_RTINDEX), + NULL); newexpr = (Node *) - search_indexed_tlist_for_sortgroupref(tle->expr, + search_indexed_tlist_for_sortgroupref(expr, tle->ressortgroupref, subplan_itlist, OUTER_VAR); if (!newexpr) newexpr = fix_upper_expr(root, - (Node *) tle->expr, + (Node *) expr, subplan_itlist, OUTER_VAR, rtoffset, diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c index 73ff40721c..75c3d7230c 100644 --- a/src/backend/optimizer/prep/prepjointree.c +++ b/src/backend/optimizer/prep/prepjointree.c @@ -988,6 +988,7 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte, subroot->plan_params = NIL; subroot->outer_params = NULL; subroot->planner_cxt = CurrentMemoryContext; + subroot->nullable_sortgroup_refs = NULL; subroot->init_plans = NIL; subroot->cte_plan_ids = NIL; subroot->multiexpr_params = NIL; diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index fef4c714b8..af066c3a67 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -1023,6 +1023,17 @@ typedef enum RTEKind * present during parsing or rewriting */ } RTEKind; +/* + * RT index for grouping sets. + * + * There might be vars in the target list that are nulled by grouping sets, so + * we need a RT index for grouping sets to mark these vars in nullingrels. In + * practice we do not need to create a real RangeTblEntry for grouping sets. + * + * Here we use 0 because it's not a valid outer join relid. + */ +#define GROUPING_SET_RTINDEX 0 + typedef struct RangeTblEntry { pg_node_attr(custom_read_write, custom_query_jumble) diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index 5702fbba60..e9498f7d96 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -265,6 +265,13 @@ struct PlannerInfo */ Relids all_query_rels; + /* + * nullable_sortgroup_refs is a Relids set of all the ressortgrouprefs of + * TargetEntrys that are nullable by grouping sets. This is computed in + * preprocess_grouping_sets. + */ + Relids nullable_sortgroup_refs; + /* * join_rel_list is a list of all join-relation RelOptInfos we have * considered in this planning run. For small problems we just scan the diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h index 50bc3b503a..7f260fc3f0 100644 --- a/src/include/optimizer/paths.h +++ b/src/include/optimizer/paths.h @@ -235,6 +235,7 @@ extern List *make_pathkeys_for_sortclauses_extended(PlannerInfo *root, List **sortclauses, List *tlist, bool remove_redundant, + bool remove_grouping_set_rtindex, bool *sortable); extern void initialize_mergeclause_eclasses(PlannerInfo *root, RestrictInfo *restrictinfo); diff --git a/src/test/regress/expected/groupingsets.out b/src/test/regress/expected/groupingsets.out index a3b9aaca84..9cb3971511 100644 --- a/src/test/regress/expected/groupingsets.out +++ b/src/test/regress/expected/groupingsets.out @@ -442,19 +442,22 @@ select * from ( group by grouping sets(1, 2) ) ss where x = 1 and q1 = 123; - QUERY PLAN --------------------------------------------- + QUERY PLAN +-------------------------------------------------- Subquery Scan on ss Output: ss.x, ss.q1, ss.sum Filter: ((ss.x = 1) AND (ss.q1 = 123)) -> GroupAggregate Output: (1), i1.q1, sum(i1.q2) - Group Key: 1 + Group Key: (1) Sort Key: i1.q1 Group Key: i1.q1 - -> Seq Scan on public.int8_tbl i1 - Output: 1, i1.q1, i1.q2 -(10 rows) + -> Sort + Output: (1), i1.q1, i1.q2 + Sort Key: (1) + -> Seq Scan on public.int8_tbl i1 + Output: 1, i1.q1, i1.q2 +(13 rows) select * from ( select 1 as x, q1, sum(q2) @@ -736,15 +739,18 @@ select a, b, sum(v.x) -- Test reordering of grouping sets explain (costs off) select * from gstest1 group by grouping sets((a,b,v),(v)) order by v,b,a; - QUERY PLAN ------------------------------------------------------------------------------- - GroupAggregate - Group Key: "*VALUES*".column3, "*VALUES*".column2, "*VALUES*".column1 - Group Key: "*VALUES*".column3 - -> Sort - Sort Key: "*VALUES*".column3, "*VALUES*".column2, "*VALUES*".column1 - -> Values Scan on "*VALUES*" -(6 rows) + QUERY PLAN +------------------------------------------------------------------------------------ + Incremental Sort + Sort Key: "*VALUES*".column3, "*VALUES*".column2, "*VALUES*".column1 + Presorted Key: "*VALUES*".column3 + -> GroupAggregate + Group Key: "*VALUES*".column3, "*VALUES*".column2, "*VALUES*".column1 + Group Key: "*VALUES*".column3 + -> Sort + Sort Key: "*VALUES*".column3, "*VALUES*".column2, "*VALUES*".column1 + -> Values Scan on "*VALUES*" +(9 rows) -- Agg level check. This query should error out. select (select grouping(a,b) from gstest2) from gstest2 group by a,b; @@ -816,16 +822,18 @@ select a,count(*) from gstest2 group by rollup(a) having a is distinct from 1 or explain (costs off) select a,count(*) from gstest2 group by rollup(a) having a is distinct from 1 order by a; - QUERY PLAN ----------------------------------- - GroupAggregate - Group Key: a - Group Key: () - Filter: (a IS DISTINCT FROM 1) - -> Sort - Sort Key: a - -> Seq Scan on gstest2 -(7 rows) + QUERY PLAN +---------------------------------------- + Sort + Sort Key: a + -> GroupAggregate + Group Key: a + Group Key: () + Filter: (a IS DISTINCT FROM 1) + -> Sort + Sort Key: a + -> Seq Scan on gstest2 +(9 rows) select v.c, (select count(*) from gstest2 group by () having v.c) from (values (false),(true)) v(c) order by v.c; @@ -2150,4 +2158,110 @@ select (select grouping(v1)) from (values ((select 1))) v(v1) group by v1; 0 (1 row) +-- expressions nullable by grouping sets +explain (costs off) +select distinct on (a, b) a, b +from (values (1, 1), (2, 2)) as t (a, b) where a = b +group by grouping sets((a, b), (a)) +order by a, b; + QUERY PLAN +---------------------------------------------------------------- + Unique + -> Sort + Sort Key: "*VALUES*".column1, "*VALUES*".column2 + -> HashAggregate + Hash Key: "*VALUES*".column1, "*VALUES*".column2 + Hash Key: "*VALUES*".column1 + -> Values Scan on "*VALUES*" + Filter: (column1 = column2) +(8 rows) + +select distinct on (a, b) a, b +from (values (1, 1), (2, 2)) as t (a, b) where a = b +group by grouping sets((a, b), (a)) +order by a, b; + a | b +---+--- + 1 | 1 + 1 | + 2 | 2 + 2 | +(4 rows) + +explain (costs off) +select distinct on (a, b+1) a, b+1 +from (values (1, 0), (2, 1)) as t (a, b) where a = b+1 +group by grouping sets((a, b+1), (a)) +order by a, b+1; + QUERY PLAN +---------------------------------------------------------------------- + Unique + -> Sort + Sort Key: "*VALUES*".column1, (("*VALUES*".column2 + 1)) + -> HashAggregate + Hash Key: "*VALUES*".column1, ("*VALUES*".column2 + 1) + Hash Key: "*VALUES*".column1 + -> Values Scan on "*VALUES*" + Filter: (column1 = (column2 + 1)) +(8 rows) + +select distinct on (a, b+1) a, b+1 +from (values (1, 0), (2, 1)) as t (a, b) where a = b+1 +group by grouping sets((a, b+1), (a)) +order by a, b+1; + a | ?column? +---+---------- + 1 | 1 + 1 | + 2 | 2 + 2 | +(4 rows) + +explain (costs off) +select a, b +from (values (1, 1), (2, 2)) as t (a, b) where a = b +group by grouping sets((a, b), (a)) +order by a, b nulls first; + QUERY PLAN +---------------------------------------------------------------- + Sort + Sort Key: "*VALUES*".column1, "*VALUES*".column2 NULLS FIRST + -> HashAggregate + Hash Key: "*VALUES*".column1, "*VALUES*".column2 + Hash Key: "*VALUES*".column1 + -> Values Scan on "*VALUES*" + Filter: (column1 = column2) +(7 rows) + +select a, b +from (values (1, 1), (2, 2)) as t (a, b) where a = b +group by grouping sets((a, b), (a)) +order by a, b nulls first; + a | b +---+--- + 1 | + 1 | 1 + 2 | + 2 | 2 +(4 rows) + +explain (costs off) +select 1 as one group by rollup(one) order by one nulls first; + QUERY PLAN +----------------------------- + Sort + Sort Key: (1) NULLS FIRST + -> MixedAggregate + Hash Key: 1 + Group Key: () + -> Result +(6 rows) + +select 1 as one group by rollup(one) order by one nulls first; + one +----- + + 1 +(2 rows) + -- end diff --git a/src/test/regress/sql/groupingsets.sql b/src/test/regress/sql/groupingsets.sql index 90ba27257a..c6216edbdc 100644 --- a/src/test/regress/sql/groupingsets.sql +++ b/src/test/regress/sql/groupingsets.sql @@ -589,4 +589,42 @@ explain (costs off) select (select grouping(v1)) from (values ((select 1))) v(v1) group by v1; select (select grouping(v1)) from (values ((select 1))) v(v1) group by v1; +-- expressions nullable by grouping sets +explain (costs off) +select distinct on (a, b) a, b +from (values (1, 1), (2, 2)) as t (a, b) where a = b +group by grouping sets((a, b), (a)) +order by a, b; + +select distinct on (a, b) a, b +from (values (1, 1), (2, 2)) as t (a, b) where a = b +group by grouping sets((a, b), (a)) +order by a, b; + +explain (costs off) +select distinct on (a, b+1) a, b+1 +from (values (1, 0), (2, 1)) as t (a, b) where a = b+1 +group by grouping sets((a, b+1), (a)) +order by a, b+1; + +select distinct on (a, b+1) a, b+1 +from (values (1, 0), (2, 1)) as t (a, b) where a = b+1 +group by grouping sets((a, b+1), (a)) +order by a, b+1; + +explain (costs off) +select a, b +from (values (1, 1), (2, 2)) as t (a, b) where a = b +group by grouping sets((a, b), (a)) +order by a, b nulls first; + +select a, b +from (values (1, 1), (2, 2)) as t (a, b) where a = b +group by grouping sets((a, b), (a)) +order by a, b nulls first; + +explain (costs off) +select 1 as one group by rollup(one) order by one nulls first; +select 1 as one group by rollup(one) order by one nulls first; + -- end -- 2.31.0 ^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Wrong results with grouping sets 2023-09-25 07:11 Wrong results with grouping sets Richard Guo <[email protected]> @ 2023-10-07 10:29 ` Richard Guo <[email protected]> 2 siblings, 0 replies; 28+ messages in thread From: Richard Guo @ 2023-10-07 10:29 UTC (permalink / raw) To: pgsql-hackers; +Cc: Tom Lane <[email protected]> On Mon, Sep 25, 2023 at 3:11 PM Richard Guo <[email protected]> wrote: > I think the root cause is that when we generate distinct_pathkeys, we > failed to realize that Var 'b' might be nullable by the grouping sets, > so it's no longer always equal to Var 'a'. It's not correct to deem > that the PathKey for 'b' is redundant and thus remove it from the > pathkeys list. > > We have the same issue when generating sort_pathkeys. As a result, we > may have the final output in the wrong order. There were several > reports about this issue before, such as [1][2]. > > To fix this issue, I'm thinking that we mark the grouping expressions > nullable by grouping sets with a dummy RTE for grouping sets, something > like attached. > Hi Tom, I'm wondering if you've got a chance to look into this issue. What do you think about the fix? Thanks Richard ^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Wrong results with grouping sets 2023-09-25 07:11 Wrong results with grouping sets Richard Guo <[email protected]> @ 2023-11-16 15:25 ` Alena Rybakina <[email protected]> 2023-11-17 03:25 ` Re: Wrong results with grouping sets Richard Guo <[email protected]> 2 siblings, 1 reply; 28+ messages in thread From: Alena Rybakina @ 2023-11-16 15:25 UTC (permalink / raw) To: Richard Guo <[email protected]>; pgsql-hackers Hi! Thank you for your work on the subject. On 25.09.2023 10:11, Richard Guo wrote: > I think I've come across a wrong result issue with grouping sets, as > shown by the query below. > > -- result is correct with only grouping sets > select a, b > from (values (1, 1), (2, 2)) as t (a, b) where a = b > group by grouping sets((a, b), (a)); > a | b > ---+--- > 1 | 1 > 1 | > 2 | 2 > 2 | > (4 rows) > > -- result is NOT correct with grouping sets and distinct on > select distinct on (a, b) a, b > from (values (1, 1), (2, 2)) as t (a, b) where a = b > group by grouping sets((a, b), (a)); > a | b > ---+--- > 1 | 1 > 2 | 2 > (2 rows) > > The distinct on expressions include both 'a' and 'b', so rows (1, 1) and > (1, NULL) should not be considered equal. (The same for rows (2, 2) and > (2, NULL).) I noticed that this query worked correctly in the main branch with the inequality operator: postgres=# select distinct on (a, b) a, b from (values (3, 1), (2, 2)) as t (a, b) where a > b group by grouping sets((a, b), (a)); a | b ---+--- 3 | 1 3 | (2 rows) So, I think you are right) > I think the root cause is that when we generate distinct_pathkeys, we > failed to realize that Var 'b' might be nullable by the grouping sets, > so it's no longer always equal to Var 'a'. It's not correct to deem > that the PathKey for 'b' is redundant and thus remove it from the > pathkeys list. > > We have the same issue when generating sort_pathkeys. As a result, we > may have the final output in the wrong order. There were several > reports about this issue before, such as [1][2]. > > To fix this issue, I'm thinking that we mark the grouping expressions > nullable by grouping sets with a dummy RTE for grouping sets, something > like attached. In practice we do not need to create a real RTE for > that, what we need is just a RT index. In the patch I use 0, because > it's not a valid outer join relid, so it would not conflict with the > outer-join-aware-Var infrastructure. > > If the grouping expression is a Var or PHV, we can just set its > nullingrels, very straightforward. For an expression that is neither a > Var nor a PHV, I'm not quite sure how to set the nullingrels. I tried > the idea of wrapping it in a new PHV to carry the nullingrels, but that > may cause some unnecessary plan diffs. In the patch for such an > expression I just set the nullingrels of Vars or PHVs that are contained > in it. This is not really 'correct' in theory, because it is the whole > expression that can be nullable by grouping sets, not its individual > vars. But it works in practice, because what we need is that the > expression can be somehow distinguished from the same expression in ECs, > and marking its vars is sufficient for this purpose. But what if the > expression is variable-free? This is the point that needs more work. > Fow now the patch just handles variable-free expressions of type Const, > by wrapping it in a new PHV. > > There are some places where we need to artificially remove the RT index > of grouping sets from the nullingrels, such as when we generate > PathTarget for initial input to grouping nodes, or when we generate > PathKeys for the grouping clauses, because the expressions there are > logically below the grouping sets. We also need to do that in > set_upper_references when we update the targetlist of an Agg node, so > that we can perform exact match for nullingrels, rather than superset > match. > > Since the fix depends on the nullingrels stuff, it seems not easy for > back-patching. I'm not sure what we should do in back branches. > > Any thoughts? > > [1] > https://www.postgresql.org/message-id/[email protected]... > [2] > https://www.postgresql.org/message-id/[email protected] > I looked at your patch and noticed a few things: 1. I think you should add a test with the cube operator, because I noticed that the order of the query in the result has also changed: master: postgres=# select a,b from (values(1,1),(2,2)) as t (a,b) where a=b group by cube (a, (a,b)) order by b,a; a | b ---+--- 1 | 1 1 | 1 1 | 2 | 2 2 | 2 2 | | (7 rows) with patch: postgres=# select a, b from (values (1, 1), (2, 2)) as t (a, b) where a = b group by cube(a, (a, b)) order by b,a; a | b ---+--- 1 | 1 1 | 1 2 | 2 2 | 2 1 | 2 | | (7 rows) -- Regards, Alena Rybakina ^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Wrong results with grouping sets 2023-09-25 07:11 Wrong results with grouping sets Richard Guo <[email protected]> 2023-11-16 15:25 ` Re: Wrong results with grouping sets Alena Rybakina <[email protected]> @ 2023-11-17 03:25 ` Richard Guo <[email protected]> 0 siblings, 0 replies; 28+ messages in thread From: Richard Guo @ 2023-11-17 03:25 UTC (permalink / raw) To: Alena Rybakina <[email protected]>; +Cc: pgsql-hackers On Thu, Nov 16, 2023 at 11:25 PM Alena Rybakina <[email protected]> wrote: > I noticed that this query worked correctly in the main branch with the > inequality operator: > > postgres=# select distinct on (a, b) a, b from (values (3, 1), (2, 2)) as > t (a, b) where a > b group by grouping sets((a, b), (a)); a | b ---+--- 3 | > 1 3 | (2 rows) > > So, I think you are right) > Thanks for taking an interest in this patch and verifying it. > I looked at your patch and noticed a few things: > > 1. I think you should add a test with the cube operator, because I noticed > that the order of the query in the result has also changed: > Hmm, I'm not sure if that's necessary. The wrong result order you saw here is caused by the same reason explained above: the planner fails to realize that Var 'a' and 'b' are nullable by the grouping sets, making them no longer always equal to each other. This issue should have been covered in the tests added by v1 patch. Thanks Richard ^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Wrong results with grouping sets 2023-09-25 07:11 Wrong results with grouping sets Richard Guo <[email protected]> @ 2023-12-07 08:22 ` Richard Guo <[email protected]> 2 siblings, 0 replies; 28+ messages in thread From: Richard Guo @ 2023-12-07 08:22 UTC (permalink / raw) To: pgsql-hackers; +Cc: Tom Lane <[email protected]> On Mon, Sep 25, 2023 at 3:11 PM Richard Guo <[email protected]> wrote: > If the grouping expression is a Var or PHV, we can just set its > nullingrels, very straightforward. For an expression that is neither a > Var nor a PHV, I'm not quite sure how to set the nullingrels. I tried > the idea of wrapping it in a new PHV to carry the nullingrels, but that > may cause some unnecessary plan diffs. In the patch for such an > expression I just set the nullingrels of Vars or PHVs that are contained > in it. This is not really 'correct' in theory, because it is the whole > expression that can be nullable by grouping sets, not its individual > vars. But it works in practice, because what we need is that the > expression can be somehow distinguished from the same expression in ECs, > and marking its vars is sufficient for this purpose. But what if the > expression is variable-free? This is the point that needs more work. > Fow now the patch just handles variable-free expressions of type Const, > by wrapping it in a new PHV. > For a variable-free expression, if it contains volatile functions, SRFs, aggregates, or window functions, it would not be treated as a member of EC that is redundant (see get_eclass_for_sort_expr()). That means it would not be removed from the pathkeys list, so we do not need to set the nullingrels for it. Otherwise we can just wrap the expression in a PlaceHolderVar. Attached is an updated patch to do that. BTW, this wrong results issue has existed ever since grouping sets was introduced in v9.5, and there were field reports complaining about this issue. I think it would be great if we can get rid of it. I'm still not sure what we should do in back branches. But let's fix it at least on v16 and later. Thanks Richard Attachments: [application/octet-stream] v2-0001-Mark-expressions-nullable-by-grouping-sets.patch (22.4K, ../../CAMbWs49NXEwP_xp6hmjax5R88g0UbeRT1=5_tX0=j_EcdbSV_A@mail.gmail.com/3-v2-0001-Mark-expressions-nullable-by-grouping-sets.patch) download | inline diff: From 92d45af9b621307d96787df9a1b0884b5cbbd2f4 Mon Sep 17 00:00:00 2001 From: Richard Guo <[email protected]> Date: Mon, 25 Sep 2023 09:39:18 +0800 Subject: [PATCH v2] Mark expressions nullable by grouping sets When generating window_pathkeys, distinct_pathkeys or sort_pathkeys, we failed to realize that the grouping/ordering expressions might be nullable by grouping sets. As a result, we may incorrectly deem that the PathKeys are redundant by EquivalenceClass processing and thus remove them from the pathkeys list. That would lead to wrong results in some cases. To fix it, mark expressions nullable by grouping sets with a dummy RTE. --- src/backend/optimizer/path/pathkeys.c | 11 ++ src/backend/optimizer/plan/planner.c | 154 ++++++++++++++++++- src/backend/optimizer/plan/setrefs.c | 20 ++- src/backend/optimizer/prep/prepjointree.c | 1 + src/include/nodes/parsenodes.h | 11 ++ src/include/nodes/pathnodes.h | 7 + src/include/optimizer/paths.h | 1 + src/test/regress/expected/groupingsets.out | 164 +++++++++++++++++---- src/test/regress/sql/groupingsets.sql | 38 +++++ 9 files changed, 377 insertions(+), 30 deletions(-) diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c index fdb60aaa8d..1637bc64de 100644 --- a/src/backend/optimizer/path/pathkeys.c +++ b/src/backend/optimizer/path/pathkeys.c @@ -26,6 +26,7 @@ #include "optimizer/pathnode.h" #include "optimizer/paths.h" #include "partitioning/partbounds.h" +#include "rewrite/rewriteManip.h" #include "utils/lsyscache.h" @@ -1141,6 +1142,7 @@ make_pathkeys_for_sortclauses(PlannerInfo *root, &sortclauses, tlist, false, + false, &sortable); /* It's caller error if not all clauses were sortable */ Assert(sortable); @@ -1158,6 +1160,9 @@ make_pathkeys_for_sortclauses(PlannerInfo *root, * give rise to redundant pathkeys are removed from the sortclauses list * (which therefore must be pass-by-reference in this version). * + * If remove_grouping_set_rtindex is true, then we need to remove the RT index + * of grouping sets from the sort exprs before we make PathKeys for them. + * * *sortable is set to true if all the sort clauses are in fact sortable. * If any are not, they are ignored except for setting *sortable false. * (In that case, the output pathkey list isn't really useful. However, @@ -1171,6 +1176,7 @@ make_pathkeys_for_sortclauses_extended(PlannerInfo *root, List **sortclauses, List *tlist, bool remove_redundant, + bool remove_grouping_set_rtindex, bool *sortable) { List *pathkeys = NIL; @@ -1189,6 +1195,11 @@ make_pathkeys_for_sortclauses_extended(PlannerInfo *root, *sortable = false; continue; } + if (remove_grouping_set_rtindex && + bms_is_member(sortcl->tleSortGroupRef, root->nullable_sortgroup_refs)) + sortkey = (Expr *) remove_nulling_relids((Node *) sortkey, + bms_make_singleton(GROUPING_SET_RTINDEX), + NULL); pathkey = make_pathkey_from_sortop(root, sortkey, sortcl->sortop, diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index a8cea5efe1..31c173385c 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -50,6 +50,7 @@ #include "optimizer/paramassign.h" #include "optimizer/pathnode.h" #include "optimizer/paths.h" +#include "optimizer/placeholder.h" #include "optimizer/plancat.h" #include "optimizer/planmain.h" #include "optimizer/planner.h" @@ -133,6 +134,9 @@ static Node *preprocess_expression(PlannerInfo *root, Node *expr, int kind); static void preprocess_qual_conditions(PlannerInfo *root, Node *jtnode); static void grouping_planner(PlannerInfo *root, double tuple_fraction); static grouping_sets_data *preprocess_grouping_sets(PlannerInfo *root); +static void markNullableByGroupingSets(PlannerInfo *root); +static void markTargetEntryNullable(PlannerInfo *root, TargetEntry *tle, + int rtindex); static List *remap_to_groupclause_idx(List *groupClause, List *gsets, int *tleref_to_colnum_map); static void preprocess_rowmarks(PlannerInfo *root); @@ -641,6 +645,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse, root->plan_params = NIL; root->outer_params = NULL; root->planner_cxt = CurrentMemoryContext; + root->nullable_sortgroup_refs = NULL; root->init_plans = NIL; root->cte_plan_ids = NIL; root->multiexpr_params = NIL; @@ -2031,6 +2036,9 @@ preprocess_grouping_sets(PlannerInfo *root) gd->unsortable_refs = NULL; gd->unsortable_sets = NIL; + /* Mark the exprs in TargetEntrys nullable by the grouping sets */ + markNullableByGroupingSets(root); + /* * We don't currently make any attempt to optimize the groupClause when * there are grouping sets, so just duplicate it in processed_groupClause. @@ -2192,6 +2200,124 @@ preprocess_grouping_sets(PlannerInfo *root) return gd; } +/* + * If the expression of a TargetEntry is nullable by grouping sets, set its + * nullingrels to include the RT index of grouping sets. + * + * We also compute nullable_sortgroup_refs in this function. + */ +static void +markNullableByGroupingSets(PlannerInfo *root) +{ + Query *parse = root->parse; + ListCell *lc; + + /* nothing to do if there are no grouping sets */ + if (parse->groupingSets == NIL) + return; + + foreach(lc, parse->groupClause) + { + SortGroupClause *gc = lfirst_node(SortGroupClause, lc); + Index ref = gc->tleSortGroupRef; + ListCell *l; + + foreach(l, parse->groupingSets) + { + List *content = lfirst(l); + + /* + * The current TargetEntry is a group clause but not contained in + * this grouping set, so it would be nulled by the grouping sets. + * Mark it so. + */ + if (!list_member_int(content, ref)) + { + TargetEntry *tle; + + tle = get_sortgroupref_tle(ref, parse->targetList); + markTargetEntryNullable(root, tle, GROUPING_SET_RTINDEX); + + root->nullable_sortgroup_refs = + bms_add_member(root->nullable_sortgroup_refs, ref); + + break; + } + } + } +} + +/* + * Mark the expr of the given TargetEntry nullable by the 'rtindex'. + */ +static void +markTargetEntryNullable(PlannerInfo *root, TargetEntry *tle, int rtindex) +{ + Node *newnode; + + newnode = (Node *) copyObject(tle->expr); + if (IsA(newnode, Var)) + { + Var *var = (Var *) newnode; + + var->varnullingrels = bms_add_member(var->varnullingrels, rtindex); + } + else if (IsA(newnode, PlaceHolderVar)) + { + PlaceHolderVar *phv = (PlaceHolderVar *) newnode; + + phv->phnullingrels = bms_add_member(phv->phnullingrels, rtindex); + } + else + { + Relids relids = pull_varnos(root, newnode); + + /* + * If tle->expr is not variable-free, we set the nullingrels of Vars or + * PHVs that are contained in the expr. This is not really 'correct' + * in theory, because it is the whole expression that can be nullable + * by grouping sets, not its individual vars. But it works in + * practice, because what we need is that the expression can be somehow + * distinguished from the same expression in ECs, and marking its vars + * is sufficient for this purpose. + */ + if (!bms_is_empty(relids)) + { + newnode = add_nulling_relids(newnode, + relids, + bms_make_singleton(rtindex)); + } + else /* variable-free? */ + { + /* + * If tle->expr is variable-free and does not contain volatile + * functions, set-returning functions, aggregates, or window + * functions, it might be treated as a member of EC that is + * redundant. So we need to wrap it in a new PlaceHolderVar to + * carry the nullingrels. Otherwise we do not bother to make any + * changes. + */ + if (!contain_volatile_functions(newnode) && + !expression_returns_set(newnode) && + !contain_agg_clause(newnode) && + !contain_window_function(newnode)) + { + PlaceHolderVar *newphv; + Relids phrels = + get_relids_in_jointree((Node *) root->parse->jointree, + true, false); + + newphv = make_placeholder_expr(root, (Expr *) newnode, phrels); + newphv->phnullingrels = + bms_add_member(newphv->phnullingrels, rtindex); + newnode = (Node *) newphv; + } + } + } + + tle->expr = (Expr *) newnode; +} + /* * Given a groupclause and a list of GroupingSetData, return equivalent sets * (without annotation) mapped to indexes into the given groupclause. @@ -3434,9 +3560,20 @@ standard_qp_callback(PlannerInfo *root, void *extra) if (grouping_is_sortable(groupClause)) { - root->group_pathkeys = make_pathkeys_for_sortclauses(root, - groupClause, - tlist); + bool sortable; + + /* + * Note that the groupClause is logically below the grouping sets. + * So set remove_grouping_set_rtindex to true. + */ + root->group_pathkeys = + make_pathkeys_for_sortclauses_extended(root, + &groupClause, + tlist, + false, + true, + &sortable); + Assert(sortable); root->num_groupby_pathkeys = list_length(root->group_pathkeys); } else @@ -3462,6 +3599,7 @@ standard_qp_callback(PlannerInfo *root, void *extra) &root->processed_groupClause, tlist, true, + false, &sortable); if (!sortable) { @@ -3512,6 +3650,7 @@ standard_qp_callback(PlannerInfo *root, void *extra) &root->processed_distinctClause, tlist, true, + false, &sortable); if (!sortable) root->distinct_pathkeys = NIL; @@ -5372,7 +5511,15 @@ make_group_input_target(PlannerInfo *root, PathTarget *final_target) { /* * It's a grouping column, so add it to the input target as-is. + * + * Note that the target is logically below the grouping sets. So + * if the grouping expr is nullable by grouping sets, we need to + * remove the RT index of grouping sets from its nullingrels. */ + if (bms_is_member(sgref, root->nullable_sortgroup_refs)) + expr = (Expr *) remove_nulling_relids((Node *) expr, + bms_make_singleton(GROUPING_SET_RTINDEX), + NULL); add_column_to_pathtarget(input_target, expr, sgref); } else @@ -6053,6 +6200,7 @@ make_pathkeys_for_window(PlannerInfo *root, WindowClause *wc, &wc->partitionClause, tlist, true, + false, &sortable); Assert(sortable); diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index 4bb68ac90e..095d6f00e7 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -26,6 +26,7 @@ #include "optimizer/subselect.h" #include "optimizer/tlist.h" #include "parser/parse_relation.h" +#include "rewrite/rewriteManip.h" #include "tcop/utility.h" #include "utils/lsyscache.h" #include "utils/syscache.h" @@ -2419,14 +2420,29 @@ set_upper_references(PlannerInfo *root, Plan *plan, int rtoffset) /* If it's a sort/group item, first try to match by sortref */ if (tle->ressortgroupref != 0) { + Expr *expr = tle->expr; + + /* + * If it's an Agg node for grouping sets, any Vars and PHVs + * appearing here should have nullingrels that include the effects + * of the grouping sets, ie they will have nullingrels equal to the + * input Vars/PHVs' nullingrels plus the RT index of grouping sets. + * In order to perform exact match, we can remove the RT index of + * grouping sets first. + */ + if (IsA(plan, Agg) && + bms_is_member(tle->ressortgroupref, root->nullable_sortgroup_refs)) + expr = (Expr *) remove_nulling_relids((Node *) expr, + bms_make_singleton(GROUPING_SET_RTINDEX), + NULL); newexpr = (Node *) - search_indexed_tlist_for_sortgroupref(tle->expr, + search_indexed_tlist_for_sortgroupref(expr, tle->ressortgroupref, subplan_itlist, OUTER_VAR); if (!newexpr) newexpr = fix_upper_expr(root, - (Node *) tle->expr, + (Node *) expr, subplan_itlist, OUTER_VAR, rtoffset, diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c index 73ff40721c..75c3d7230c 100644 --- a/src/backend/optimizer/prep/prepjointree.c +++ b/src/backend/optimizer/prep/prepjointree.c @@ -988,6 +988,7 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte, subroot->plan_params = NIL; subroot->outer_params = NULL; subroot->planner_cxt = CurrentMemoryContext; + subroot->nullable_sortgroup_refs = NULL; subroot->init_plans = NIL; subroot->cte_plan_ids = NIL; subroot->multiexpr_params = NIL; diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index e494309da8..1243f2de2d 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -1016,6 +1016,17 @@ typedef enum RTEKind * present during parsing or rewriting */ } RTEKind; +/* + * RT index for grouping sets. + * + * There might be vars in the target list that are nullable by grouping sets, + * so we need a RT index for grouping sets to mark these vars in nullingrels. + * In practice we do not need to create a real RangeTblEntry for grouping sets. + * + * Here we use 0 because it's not a valid outer join relid. + */ +#define GROUPING_SET_RTINDEX 0 + typedef struct RangeTblEntry { pg_node_attr(custom_read_write, custom_query_jumble) diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index ed85dc7414..a60da21fe4 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -265,6 +265,13 @@ struct PlannerInfo */ Relids all_query_rels; + /* + * nullable_sortgroup_refs is a Relids set of all the ressortgrouprefs of + * TargetEntrys that are nullable by grouping sets. This is computed in + * preprocess_grouping_sets. + */ + Relids nullable_sortgroup_refs; + /* * join_rel_list is a list of all join-relation RelOptInfos we have * considered in this planning run. For small problems we just scan the diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h index 9e7408c7ec..033e796130 100644 --- a/src/include/optimizer/paths.h +++ b/src/include/optimizer/paths.h @@ -234,6 +234,7 @@ extern List *make_pathkeys_for_sortclauses_extended(PlannerInfo *root, List **sortclauses, List *tlist, bool remove_redundant, + bool remove_grouping_set_rtindex, bool *sortable); extern void initialize_mergeclause_eclasses(PlannerInfo *root, RestrictInfo *restrictinfo); diff --git a/src/test/regress/expected/groupingsets.out b/src/test/regress/expected/groupingsets.out index a3b9aaca84..9cb3971511 100644 --- a/src/test/regress/expected/groupingsets.out +++ b/src/test/regress/expected/groupingsets.out @@ -442,19 +442,22 @@ select * from ( group by grouping sets(1, 2) ) ss where x = 1 and q1 = 123; - QUERY PLAN --------------------------------------------- + QUERY PLAN +-------------------------------------------------- Subquery Scan on ss Output: ss.x, ss.q1, ss.sum Filter: ((ss.x = 1) AND (ss.q1 = 123)) -> GroupAggregate Output: (1), i1.q1, sum(i1.q2) - Group Key: 1 + Group Key: (1) Sort Key: i1.q1 Group Key: i1.q1 - -> Seq Scan on public.int8_tbl i1 - Output: 1, i1.q1, i1.q2 -(10 rows) + -> Sort + Output: (1), i1.q1, i1.q2 + Sort Key: (1) + -> Seq Scan on public.int8_tbl i1 + Output: 1, i1.q1, i1.q2 +(13 rows) select * from ( select 1 as x, q1, sum(q2) @@ -736,15 +739,18 @@ select a, b, sum(v.x) -- Test reordering of grouping sets explain (costs off) select * from gstest1 group by grouping sets((a,b,v),(v)) order by v,b,a; - QUERY PLAN ------------------------------------------------------------------------------- - GroupAggregate - Group Key: "*VALUES*".column3, "*VALUES*".column2, "*VALUES*".column1 - Group Key: "*VALUES*".column3 - -> Sort - Sort Key: "*VALUES*".column3, "*VALUES*".column2, "*VALUES*".column1 - -> Values Scan on "*VALUES*" -(6 rows) + QUERY PLAN +------------------------------------------------------------------------------------ + Incremental Sort + Sort Key: "*VALUES*".column3, "*VALUES*".column2, "*VALUES*".column1 + Presorted Key: "*VALUES*".column3 + -> GroupAggregate + Group Key: "*VALUES*".column3, "*VALUES*".column2, "*VALUES*".column1 + Group Key: "*VALUES*".column3 + -> Sort + Sort Key: "*VALUES*".column3, "*VALUES*".column2, "*VALUES*".column1 + -> Values Scan on "*VALUES*" +(9 rows) -- Agg level check. This query should error out. select (select grouping(a,b) from gstest2) from gstest2 group by a,b; @@ -816,16 +822,18 @@ select a,count(*) from gstest2 group by rollup(a) having a is distinct from 1 or explain (costs off) select a,count(*) from gstest2 group by rollup(a) having a is distinct from 1 order by a; - QUERY PLAN ----------------------------------- - GroupAggregate - Group Key: a - Group Key: () - Filter: (a IS DISTINCT FROM 1) - -> Sort - Sort Key: a - -> Seq Scan on gstest2 -(7 rows) + QUERY PLAN +---------------------------------------- + Sort + Sort Key: a + -> GroupAggregate + Group Key: a + Group Key: () + Filter: (a IS DISTINCT FROM 1) + -> Sort + Sort Key: a + -> Seq Scan on gstest2 +(9 rows) select v.c, (select count(*) from gstest2 group by () having v.c) from (values (false),(true)) v(c) order by v.c; @@ -2150,4 +2158,110 @@ select (select grouping(v1)) from (values ((select 1))) v(v1) group by v1; 0 (1 row) +-- expressions nullable by grouping sets +explain (costs off) +select distinct on (a, b) a, b +from (values (1, 1), (2, 2)) as t (a, b) where a = b +group by grouping sets((a, b), (a)) +order by a, b; + QUERY PLAN +---------------------------------------------------------------- + Unique + -> Sort + Sort Key: "*VALUES*".column1, "*VALUES*".column2 + -> HashAggregate + Hash Key: "*VALUES*".column1, "*VALUES*".column2 + Hash Key: "*VALUES*".column1 + -> Values Scan on "*VALUES*" + Filter: (column1 = column2) +(8 rows) + +select distinct on (a, b) a, b +from (values (1, 1), (2, 2)) as t (a, b) where a = b +group by grouping sets((a, b), (a)) +order by a, b; + a | b +---+--- + 1 | 1 + 1 | + 2 | 2 + 2 | +(4 rows) + +explain (costs off) +select distinct on (a, b+1) a, b+1 +from (values (1, 0), (2, 1)) as t (a, b) where a = b+1 +group by grouping sets((a, b+1), (a)) +order by a, b+1; + QUERY PLAN +---------------------------------------------------------------------- + Unique + -> Sort + Sort Key: "*VALUES*".column1, (("*VALUES*".column2 + 1)) + -> HashAggregate + Hash Key: "*VALUES*".column1, ("*VALUES*".column2 + 1) + Hash Key: "*VALUES*".column1 + -> Values Scan on "*VALUES*" + Filter: (column1 = (column2 + 1)) +(8 rows) + +select distinct on (a, b+1) a, b+1 +from (values (1, 0), (2, 1)) as t (a, b) where a = b+1 +group by grouping sets((a, b+1), (a)) +order by a, b+1; + a | ?column? +---+---------- + 1 | 1 + 1 | + 2 | 2 + 2 | +(4 rows) + +explain (costs off) +select a, b +from (values (1, 1), (2, 2)) as t (a, b) where a = b +group by grouping sets((a, b), (a)) +order by a, b nulls first; + QUERY PLAN +---------------------------------------------------------------- + Sort + Sort Key: "*VALUES*".column1, "*VALUES*".column2 NULLS FIRST + -> HashAggregate + Hash Key: "*VALUES*".column1, "*VALUES*".column2 + Hash Key: "*VALUES*".column1 + -> Values Scan on "*VALUES*" + Filter: (column1 = column2) +(7 rows) + +select a, b +from (values (1, 1), (2, 2)) as t (a, b) where a = b +group by grouping sets((a, b), (a)) +order by a, b nulls first; + a | b +---+--- + 1 | + 1 | 1 + 2 | + 2 | 2 +(4 rows) + +explain (costs off) +select 1 as one group by rollup(one) order by one nulls first; + QUERY PLAN +----------------------------- + Sort + Sort Key: (1) NULLS FIRST + -> MixedAggregate + Hash Key: 1 + Group Key: () + -> Result +(6 rows) + +select 1 as one group by rollup(one) order by one nulls first; + one +----- + + 1 +(2 rows) + -- end diff --git a/src/test/regress/sql/groupingsets.sql b/src/test/regress/sql/groupingsets.sql index 90ba27257a..c6216edbdc 100644 --- a/src/test/regress/sql/groupingsets.sql +++ b/src/test/regress/sql/groupingsets.sql @@ -589,4 +589,42 @@ explain (costs off) select (select grouping(v1)) from (values ((select 1))) v(v1) group by v1; select (select grouping(v1)) from (values ((select 1))) v(v1) group by v1; +-- expressions nullable by grouping sets +explain (costs off) +select distinct on (a, b) a, b +from (values (1, 1), (2, 2)) as t (a, b) where a = b +group by grouping sets((a, b), (a)) +order by a, b; + +select distinct on (a, b) a, b +from (values (1, 1), (2, 2)) as t (a, b) where a = b +group by grouping sets((a, b), (a)) +order by a, b; + +explain (costs off) +select distinct on (a, b+1) a, b+1 +from (values (1, 0), (2, 1)) as t (a, b) where a = b+1 +group by grouping sets((a, b+1), (a)) +order by a, b+1; + +select distinct on (a, b+1) a, b+1 +from (values (1, 0), (2, 1)) as t (a, b) where a = b+1 +group by grouping sets((a, b+1), (a)) +order by a, b+1; + +explain (costs off) +select a, b +from (values (1, 1), (2, 2)) as t (a, b) where a = b +group by grouping sets((a, b), (a)) +order by a, b nulls first; + +select a, b +from (values (1, 1), (2, 2)) as t (a, b) where a = b +group by grouping sets((a, b), (a)) +order by a, b nulls first; + +explain (costs off) +select 1 as one group by rollup(one) order by one nulls first; +select 1 as one group by rollup(one) order by one nulls first; + -- end -- 2.31.0 ^ permalink raw reply [nested|flat] 28+ messages in thread
end of thread, other threads:[~2023-12-07 08:22 UTC | newest] Thread overview: 28+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-01-24 03:02 [PATCH 15/18] doc review for multiranges Justin Pryzby <[email protected]> 2021-01-24 03:02 [PATCH 15/18] doc review for multiranges Justin Pryzby <[email protected]> 2021-01-24 03:02 [PATCH 15/18] doc review for multiranges Justin Pryzby <[email protected]> 2021-01-24 03:02 [PATCH 15/18] doc review for multiranges Justin Pryzby <[email protected]> 2021-01-24 03:02 [PATCH 15/18] doc review for multiranges Justin Pryzby <[email protected]> 2021-01-24 03:02 [PATCH 15/18] doc review for multiranges Justin Pryzby <[email protected]> 2021-01-24 03:02 [PATCH 15/18] doc review for multiranges Justin Pryzby <[email protected]> 2021-01-24 03:02 [PATCH 15/18] doc review for multiranges Justin Pryzby <[email protected]> 2021-01-24 03:02 [PATCH 15/18] doc review for multiranges Justin Pryzby <[email protected]> 2021-01-24 03:02 [PATCH 15/18] doc review for multiranges Justin Pryzby <[email protected]> 2021-01-24 03:02 [PATCH 15/18] doc review for multiranges Justin Pryzby <[email protected]> 2021-01-24 03:02 [PATCH 15/18] doc review for multiranges Justin Pryzby <[email protected]> 2021-01-24 03:02 [PATCH 15/18] doc review for multiranges Justin Pryzby <[email protected]> 2021-01-24 03:02 [PATCH 15/18] doc review for multiranges Justin Pryzby <[email protected]> 2021-01-24 03:02 [PATCH 15/18] doc review for multiranges Justin Pryzby <[email protected]> 2021-01-24 03:02 [PATCH 15/21] doc review for multiranges Justin Pryzby <[email protected]> 2021-01-24 03:02 [PATCH 15/18] doc review for multiranges Justin Pryzby <[email protected]> 2021-01-24 03:02 [PATCH 15/18] doc review for multiranges Justin Pryzby <[email protected]> 2021-01-24 03:02 [PATCH 15/18] doc review for multiranges Justin Pryzby <[email protected]> 2021-01-24 03:02 [PATCH 15/18] doc review for multiranges Justin Pryzby <[email protected]> 2021-01-24 03:02 [PATCH 15/18] doc review for multiranges Justin Pryzby <[email protected]> 2021-01-24 03:02 [PATCH 15/18] doc review for multiranges Justin Pryzby <[email protected]> 2021-01-24 03:02 [PATCH 15/18] doc review for multiranges Justin Pryzby <[email protected]> 2023-09-25 07:11 Wrong results with grouping sets Richard Guo <[email protected]> 2023-10-07 10:29 ` Re: Wrong results with grouping sets Richard Guo <[email protected]> 2023-11-16 15:25 ` Re: Wrong results with grouping sets Alena Rybakina <[email protected]> 2023-11-17 03:25 ` Re: Wrong results with grouping sets Richard Guo <[email protected]> 2023-12-07 08:22 ` Re: Wrong results with grouping sets Richard Guo <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox