Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ADBDEV-5527: Explain Analyze doesn't show statistics for queries with DML inside CTE #1174

Merged
merged 3 commits into from
Jan 16, 2025

Conversation

mos65o2
Copy link

@mos65o2 mos65o2 commented Jan 10, 2025

Change the portal strategy for modifying CTE case

In vanilla PG and Greenplum queries containing modifying CTE are executed
with portal strategy PORTAL_ONE_MOD_WITH, which involves storing the result
of the query in portal's tuple store, which the final result is then fetched
from. This strategy is fair enough and helps us avoid executing
modifying operation several times, for example, when one need to fetch
the query result partially. However, in Greenplum case executing this
strategy at QE nodes is redundant, because clients communicate with the
system throught the QD node. Therefore, this commit proposes another
type of strategy for execution at QE nodes.

Moreover, when a query contains modifying CTE, EXPLAIN ANALYZE reports most of
plan nodes as "never executed", even though the corresponding nodes have been
fully executed and have returned the tuples. This problem arise because
of PORTAL_ONE_MOD_WITH execution strategy, according to which the extra
pq message with tuple description is sent to QD, and the statistics
can't be retrieved properly in cdbexplain_recvExecStats function.

Changes cherry-pick commit: updated test outputs and matchsubs.

(cherry picked from commit c9b7b02)


Commits in the PR shouldn't be squashed to preserve authorship.

mos65o2 pushed a commit that referenced this pull request Jan 10, 2025
In vanilla PG and Greenplum queries containing modifying CTE are executed
with portal strategy PORTAL_ONE_MOD_WITH, which involves storing the result
of the query in portal's tuple store, which the final result is then fetched
from. This strategy is fair enough and helps us avoid executing
modifying operation several times, for example, when one need to fetch
the query result partially. However, in Greenplum case executing this
strategy at QE nodes is redundant, because clients communicate with the
system throught the QD node. Therefore, this commit proposes another
type of strategy for execution at QE nodes.

Moreover, when a query contains modifying CTE, EXPLAIN ANALYZE reports most of
plan nodes as "never executed", even though the corresponding nodes have been
fully executed and have returned the tuples. This problem arise because
of PORTAL_ONE_MOD_WITH execution strategy, according to which the extra
pq message with tuple description is sent to QD, and the statistics
can't be retrieved properly in cdbexplain_recvExecStats function.

(cherry picked from commit c9b7b02)
mos65o2 pushed a commit that referenced this pull request Jan 10, 2025
In vanilla PG and Greenplum queries containing modifying CTE are executed
with portal strategy PORTAL_ONE_MOD_WITH, which involves storing the result
of the query in portal's tuple store, which the final result is then fetched
from. This strategy is fair enough and helps us avoid executing
modifying operation several times, for example, when one need to fetch
the query result partially. However, in Greenplum case executing this
strategy at QE nodes is redundant, because clients communicate with the
system throught the QD node. Therefore, this commit proposes another
type of strategy for execution at QE nodes.

Moreover, when a query contains modifying CTE, EXPLAIN ANALYZE reports most of
plan nodes as "never executed", even though the corresponding nodes have been
fully executed and have returned the tuples. This problem arise because
of PORTAL_ONE_MOD_WITH execution strategy, according to which the extra
pq message with tuple description is sent to QD, and the statistics
can't be retrieved properly in cdbexplain_recvExecStats function.

(cherry picked from commit c9b7b02)
@mos65o2 mos65o2 marked this pull request as ready for review January 13, 2025 11:17
@silent-observer
Copy link

Please specify changes from the original commit in the new commit description (just "updated test outputs and matchsubs", if I didn't miss anything else)

In vanilla PG and Greenplum queries containing modifying CTE are executed
with portal strategy PORTAL_ONE_MOD_WITH, which involves storing the result
of the query in portal's tuple store, which the final result is then fetched
from. This strategy is fair enough and helps us avoid executing
modifying operation several times, for example, when one need to fetch
the query result partially. However, in Greenplum case executing this
strategy at QE nodes is redundant, because clients communicate with the
system throught the QD node. Therefore, this commit proposes another
type of strategy for execution at QE nodes.

Moreover, when a query contains modifying CTE, EXPLAIN ANALYZE reports most of
plan nodes as "never executed", even though the corresponding nodes have been
fully executed and have returned the tuples. This problem arise because
of PORTAL_ONE_MOD_WITH execution strategy, according to which the extra
pq message with tuple description is sent to QD, and the statistics
can't be retrieved properly in cdbexplain_recvExecStats function.

Changes cherry-pick commit: updated test outputs and matchsubs.

(cherry picked from commit c9b7b02)
Copy link

@silent-observer silent-observer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Stolb27 Stolb27 enabled auto-merge (rebase) January 16, 2025 05:58
@Stolb27 Stolb27 merged commit 8a55955 into adb-7.2.0 Jan 16, 2025
5 checks passed
@Stolb27 Stolb27 deleted the ADBDEV-5527 branch January 16, 2025 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants