Skip to content

Add EXECUTE command - #30

Open
ants wants to merge 26 commits into
plproxy:masterfrom
cybertec-postgresql:execute-command
Open

ants wants to merge 26 commits into
plproxy:masterfrom
cybertec-postgresql:execute-command

Conversation

@ants

@ants ants commented Feb 21, 2018

Copy link
Copy Markdown
Contributor

Currently PostgreSQL parallel execution does not work when query is run with plpgsql RETURN QUERY. To work around this it would be good to be able to send dynamically generated SQL from plproxy to remote servers. To enable this I propose to add a EXECUTE command to plproxy language. It fetches query from the argument list and sends to servers. Interacts nicely with RUN ON and SPLIT.

Example:

CREATE FUNCTION parallel_execute(query text) RETURNS SETOF record AS $$
    CLUSTER 'mycluster';
    RUN ON ALL;
    EXECUTE query;
$$ LANGUAGE plproxy;

SELECT SUM(psum) total_sum
    FROM parallel_execute('SELECT SUM(x) psum FROM big_table') partial_sums(psum int8);

Somewhat unrelatedly, optionally lift power-of-2 restriction from number of partitions when disable_hashing is specified. Responsibility of mapping to partition numbers is then delegated to the user. Plproxy only error checks the resulting partition number.

ants and others added 20 commits February 20, 2018 16:29
plproxy_setup_tuplestore belongs in main.c

Fix memory leak with creating tuples from text by introducing
a new memory context for temporary storage.
Binary mode is currently disabled, but let's be future proof.
I removed too much of the original code...
Instead, the code used to check "!func->ret_scalar" by mistake.
When streaming results, the order in which rows show up can vary.
Add ORDER BY clauses to the regression tests to make the result repeatable.
This requires moving result_map from ProxyFunction to ProxyConnection,
so that we can have one per connection.

That was not necessary before, because we got all results from one connection
in a single PGresult, and we could afford to initialize the mapping whenever
we got a result.

Now, with single-row mode enabled, the result rows don't arrive connection
by connection, so we have to maintain one result_map per connection, because
setting up the mapping for every single result would be way too expensive.

The result_map is now allocated in the persistent cluster_mem context, so
we have to take care to free it whenever the ProxyConnection is cleaned
or freed.

In passing, rearrange the code in plproxy_tuple_from_result to have less
indentation.

Add a regression test for the column name mapping functionality that was
previously missing.
@laurenz
laurenz deleted the execute-command branch April 11, 2018 13:36
@markokr

markokr commented Sep 15, 2019

Copy link
Copy Markdown
Contributor

How is this different from current dynamic queries?

@ants

ants commented Feb 5, 2020

Copy link
Copy Markdown
Contributor Author

Sorry for late response, did not notice the response here. The main difference is that instead of running a function on the partition side it runs the query directly. This was needed in this case specifically because PostgreSQL parallel execution does not work within functions. It's more efficient to let PostgreSQL handle intra node parallelism because that allows for aggregating things before sending them over the wire.

@ants
ants restored the execute-command branch February 26, 2020 14:53
@ants

ants commented Feb 26, 2020

Copy link
Copy Markdown
Contributor Author

Updated on top of latest version. Outstanding issues - 9.6 and earlier are missing tuplestore_tuple_count(), probably needs to be worked around by keeping the count on plproxy side.

Also the 2.9 base release doesn't support 9.5 and earlier due to missing ALLOCSET_SMALLS_SIZES definition.

@markokr

markokr commented Aug 14, 2020

Copy link
Copy Markdown
Contributor

Sorry for delays, I only occasionally have time for pl/proxy maintenance. I took bit deeper look and have now more questions:

  1. EXECUTE - why is SELECT not sufficent? The idea of needing to process results from free-form textual queries seems iffy.
  2. The big tuplestore refactor - whats up with that? Is it necessary for purely EXECUTE? It looks like there might be generic resultset streaming hidden there somewhere, which sounds interesting, but I am not understanding very well what is going on.
  3. disable_hashing - something like that has been asked before. Perhaps it would more general to have cluster option to switch to mod-based partition lookup setup, to lose the 2^n requirement, and user can keep using whatever hash they want.

@markokr

markokr commented Sep 26, 2020

Copy link
Copy Markdown
Contributor

I'm sorry, but this PR is not reviewable, the branch contains mixed devlopment commits for two different features - EXECUTE and streaming resultset. Please submit them separately.

Development branch as PR is kind of acceptable only if it contains only one feature and commits can be squashed, but not in this case.

I did add support for non-power-of-2 clusters to 2.10, so this part is taken care of.

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.

3 participants