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

Support EXPLAIN ANALYZE EXECUTE and EXPLAIN EXECUTE #4068

Merged
merged 1 commit into from
Aug 10, 2020

Conversation

pykello
Copy link
Contributor

@pykello pykello commented Jul 25, 2020

DESCRIPTION: Support EXPLAIN ANALYZE EXECUTE and EXPLAIN EXECUTE.

Fixes #4074, #2009, #982.

To support EXPLAIN ANALYZE EXECUTE:

  • Modify the adaptive executor to send queries for multi-query tasks one-by-one, instead of a single semi-colon separated string. This is because pqSendQueryParams() doesn't accept multiple queries.
  • Modify worker_save_query_explain_analyze() to use the bound parameters when executing the task query.

To support EXPLAIN EXECUTE: Fix FetchRemoteExplainFromWorkers to send bound params along with the EXPLAIN query.

@codecov
Copy link

codecov bot commented Jul 25, 2020

Codecov Report

Merging #4068 into master will increase coverage by 0.03%.
The diff coverage is 98.48%.

@@            Coverage Diff             @@
##           master    #4068      +/-   ##
==========================================
+ Coverage   90.69%   90.73%   +0.03%     
==========================================
  Files         186      186              
  Lines       35762    35804      +42     
==========================================
+ Hits        32436    32486      +50     
+ Misses       3326     3318       -8     

@pykello pykello force-pushed the explain_analyze_execute branch 3 times, most recently from cab0edf to 5634d3a Compare July 27, 2020 20:58
@pykello pykello changed the title WIP: Support EXPLAIN ANALYZE EXECUTE WIP: Support EXPLAIN ANALYZE EXECUTE and EXPLAIN EXECUTE Jul 27, 2020
@pykello pykello force-pushed the explain_analyze_execute branch 7 times, most recently from c145f5e to cb61b40 Compare July 28, 2020 01:45
@pykello pykello marked this pull request as ready for review July 28, 2020 01:45
@pykello pykello requested a review from marcocitus July 28, 2020 01:55
@pykello pykello changed the title WIP: Support EXPLAIN ANALYZE EXECUTE and EXPLAIN EXECUTE Support EXPLAIN ANALYZE EXECUTE and EXPLAIN EXECUTE Jul 28, 2020
/*
* ExecutorBoundParams returns the bound parameters of the current ongoing call
* to ExecutorRun. This is meant to be used by UDFs which need to access bound
* parameters.
Copy link
Member

Choose a reason for hiding this comment

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

what happens when a PL/pgSQL UDF calls other PL/pgSQL UDFs? We'd be in multiple levels of ExecutorRun. Could we lose the executorBoundParams?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed & added a test which would fail if this is not handled properly.

@pykello pykello force-pushed the explain_analyze_execute branch 2 times, most recently from b34e3be to 027e858 Compare July 29, 2020 02:44
@@ -128,6 +134,9 @@ CitusExecutorRun(QueryDesc *queryDesc,
{
DestReceiver *dest = queryDesc->dest;

ParamListInfo savedBoundParams = executorBoundParams;
executorBoundParams = queryDesc->params;
Copy link
Member

Choose a reason for hiding this comment

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

Could use a comment on why we do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

@pykello pykello merged commit b8d826a into master Aug 10, 2020
@pykello pykello deleted the explain_analyze_execute branch August 10, 2020 20:59
@metdos metdos added this to the 9.5 Release milestone Sep 1, 2020
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.

EXPLAIN ANALYZE EXECUTE with parameters and reference table fails
3 participants