pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
pgsql: Fix slot type handling for Agg nodes performing internal sorts.
2+ messages / 1 participants
[nested] [flat]

* pgsql: Fix slot type handling for Agg nodes performing internal sorts.
@ 2019-07-25 21:34 Andres Freund <andres@anarazel.de>
  0 siblings, 0 replies; 2+ messages in thread

From: Andres Freund @ 2019-07-25 21:34 UTC (permalink / raw)
  To: pgsql-committers@lists.postgresql.org

Fix slot type handling for Agg nodes performing internal sorts.

Since 15d8f8312 we assert that - and since 7ef04e4d2cb2, 4da597edf1
rely on - the slot type for an expression's
ecxt_{outer,inner,scan}tuple not changing, unless explicitly flagged
as such. That allows to either skip deforming (for a virtual tuple
slot) or optimize the code for JIT accelerated deforming
appropriately (for other known slot types).

This assumption was sometimes violated for grouping sets, when
nodeAgg.c internally uses tuplesorts, and the child node doesn't
return a TTSOpsMinimalTuple type slot. Detect that case, and flag that
the outer slot might not be "fixed".

It's probably worthwhile to optimize this further in the future, and
more granularly determine whether the slot is fixed. As we already
instantiate per-phase transition and equal expressions, we could
cheaply set the slot type appropriately for each phase.  But that's a
separate change from this bugfix.

This commit does include a very minor optimization by avoiding to
create a slot for handling tuplesorts, if no such sorts are
performed. Previously we created that slot unnecessarily in the common
case of computing all grouping sets via hashing. The code looked too
confusing without that, as the conditions for needing a sort slot and
flagging that the slot type isn't fixed, are the same.

Reported-By: Ashutosh Sharma
Author: Andres Freund
Discussion: https://postgr.es/m/CAE9k0PmNaMD2oHTEAhRyxnxpaDaYkuBYkLa1dpOpn=RS0iS2AQ@mail.gmail.com
Backpatch: 12-, where the bug was introduced in 15d8f8312

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/af3deff3f2ac79585481181cb198b04c67486c09

Modified Files
--------------
src/backend/executor/nodeAgg.c             | 34 ++++++++++++++++--
src/test/regress/expected/groupingsets.out | 55 ++++++++++++++++++++++++++++++
src/test/regress/sql/groupingsets.sql      | 12 +++++++
3 files changed, 99 insertions(+), 2 deletions(-)



^ permalink  raw  reply  [nested|flat] 2+ messages in thread

* pgsql: Fix slot type handling for Agg nodes performing internal sorts.
@ 2019-07-25 21:34 Andres Freund <andres@anarazel.de>
  0 siblings, 0 replies; 2+ messages in thread

From: Andres Freund @ 2019-07-25 21:34 UTC (permalink / raw)
  To: pgsql-committers@lists.postgresql.org

Fix slot type handling for Agg nodes performing internal sorts.

Since 15d8f8312 we assert that - and since 7ef04e4d2cb2, 4da597edf1
rely on - the slot type for an expression's
ecxt_{outer,inner,scan}tuple not changing, unless explicitly flagged
as such. That allows to either skip deforming (for a virtual tuple
slot) or optimize the code for JIT accelerated deforming
appropriately (for other known slot types).

This assumption was sometimes violated for grouping sets, when
nodeAgg.c internally uses tuplesorts, and the child node doesn't
return a TTSOpsMinimalTuple type slot. Detect that case, and flag that
the outer slot might not be "fixed".

It's probably worthwhile to optimize this further in the future, and
more granularly determine whether the slot is fixed. As we already
instantiate per-phase transition and equal expressions, we could
cheaply set the slot type appropriately for each phase.  But that's a
separate change from this bugfix.

This commit does include a very minor optimization by avoiding to
create a slot for handling tuplesorts, if no such sorts are
performed. Previously we created that slot unnecessarily in the common
case of computing all grouping sets via hashing. The code looked too
confusing without that, as the conditions for needing a sort slot and
flagging that the slot type isn't fixed, are the same.

Reported-By: Ashutosh Sharma
Author: Andres Freund
Discussion: https://postgr.es/m/CAE9k0PmNaMD2oHTEAhRyxnxpaDaYkuBYkLa1dpOpn=RS0iS2AQ@mail.gmail.com
Backpatch: 12-, where the bug was introduced in 15d8f8312

Branch
------
REL_12_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/8677c62eaeea002712dd71dc678a7a0566e59858

Modified Files
--------------
src/backend/executor/nodeAgg.c             | 34 ++++++++++++++++--
src/test/regress/expected/groupingsets.out | 55 ++++++++++++++++++++++++++++++
src/test/regress/sql/groupingsets.sql      | 12 +++++++
3 files changed, 99 insertions(+), 2 deletions(-)



^ permalink  raw  reply  [nested|flat] 2+ messages in thread


end of thread, other threads:[~2019-07-25 21:34 UTC | newest]

Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-07-25 21:34 pgsql: Fix slot type handling for Agg nodes performing internal sorts. Andres Freund <andres@anarazel.de>
2019-07-25 21:34 pgsql: Fix slot type handling for Agg nodes performing internal sorts. Andres Freund <andres@anarazel.de>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox