Skip to content

Revert #10 #14

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.carto.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# Changelog

## 10.1.2+carto-1

**Release data**: 2018-XX-XX

Changes
- Revert [#10](https://github.com/CartoDB/postgres/pull/10) as the patch was incomplete and causing crashes

## 10.1.1+carto-1

**Release date**: 2017-12-05
Expand Down
16 changes: 12 additions & 4 deletions doc/src/sgml/parallel.sgml
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,9 @@ EXPLAIN SELECT * FROM pgbench_accounts WHERE filler LIKE '%x%';
<para>
The query writes any data or locks any database rows. If a query
contains a data-modifying operation either at the top level or within
a CTE, no parallel plans for that query will be generated. As an
exception, the commands <literal>CREATE TABLE</>, <literal>SELECT
INTO</>, and <literal>CREATE MATERIALIZED VIEW</> which create a new
table and populate it can use a parallel plan.
a CTE, no parallel plans for that query will be generated. This is a
limitation of the current implementation which could be lifted in a
future release.
</para>
</listitem>

Expand Down Expand Up @@ -242,6 +241,15 @@ EXPLAIN SELECT * FROM pgbench_accounts WHERE filler LIKE '%x%';
</para>
</listitem>

<listitem>
<para>
A prepared statement is executed using a <literal>CREATE TABLE .. AS
EXECUTE ..</literal> statement. This construct converts what otherwise
would have been a read-only operation into a read-write operation,
making it ineligible for parallel query.
</para>
</listitem>

<listitem>
<para>
The transaction isolation level is serializable. This situation
Expand Down
16 changes: 7 additions & 9 deletions src/backend/access/heap/heapam.c
Original file line number Diff line number Diff line change
Expand Up @@ -2581,17 +2581,15 @@ heap_prepare_insert(Relation relation, HeapTuple tup, TransactionId xid,
CommandId cid, int options)
{
/*
* Parallel operations are required to be strictly read-only in a parallel
* worker. Parallel inserts are not safe even in the leader in the
* general case, because group locking means that heavyweight locks for
* relation extension or GIN page locks will not conflict between members
* of a lock group, but we don't prohibit that case here because there are
* useful special cases that we can safely allow, such as CREATE TABLE AS.
*/
if (IsParallelWorker())
* For now, parallel operations are required to be strictly read-only.
* Unlike heap_update() and heap_delete(), an insert should never create a
* combo CID, so it might be possible to relax this restriction, but not
* without more thought and testing.
*/
if (IsInParallelMode())
ereport(ERROR,
(errcode(ERRCODE_INVALID_TRANSACTION_STATE),
errmsg("cannot insert tuples in a parallel worker")));
errmsg("cannot insert tuples during a parallel operation")));

if (relation->rd_rel->relhasoids)
{
Expand Down
4 changes: 2 additions & 2 deletions src/backend/commands/createas.c
Original file line number Diff line number Diff line change
Expand Up @@ -326,8 +326,8 @@ ExecCreateTableAs(CreateTableAsStmt *stmt, const char *queryString,
query = linitial_node(Query, rewritten);
Assert(query->commandType == CMD_SELECT);

/* plan the query */
plan = pg_plan_query(query, CURSOR_OPT_PARALLEL_OK, params);
/* plan the query --- note we disallow parallelism */
plan = pg_plan_query(query, 0, params);

/*
* Use a snapshot with an updated command ID to ensure this query sees
Expand Down
4 changes: 3 additions & 1 deletion src/backend/commands/explain.c
Original file line number Diff line number Diff line change
Expand Up @@ -400,14 +400,16 @@ ExplainOneUtility(Node *utilityStmt, IntoClause *into, ExplainState *es,
* We have to rewrite the contained SELECT and then pass it back to
* ExplainOneQuery. It's probably not really necessary to copy the
* contained parsetree another time, but let's be safe.
*
* Like ExecCreateTableAs, disallow parallelism in the plan.
*/
CreateTableAsStmt *ctas = (CreateTableAsStmt *) utilityStmt;
List *rewritten;

rewritten = QueryRewrite(castNode(Query, copyObject(ctas->query)));
Assert(list_length(rewritten) == 1);
ExplainOneQuery(linitial_node(Query, rewritten),
CURSOR_OPT_PARALLEL_OK, ctas->into, es,
0, ctas->into, es,
queryString, params, queryEnv);
}
else if (IsA(utilityStmt, DeclareCursorStmt))
Expand Down
6 changes: 4 additions & 2 deletions src/backend/executor/execMain.c
Original file line number Diff line number Diff line change
Expand Up @@ -1697,9 +1697,11 @@ ExecutePlan(EState *estate,

/*
* If the plan might potentially be executed multiple times, we must force
* it to run without parallelism, because we might exit early.
* it to run without parallelism, because we might exit early. Also
* disable parallelism when writing into a relation, because no database
* changes are allowed in parallel mode.
*/
if (!execute_once)
if (!execute_once || dest->mydest == DestIntoRel)
use_parallel_mode = false;

estate->es_use_parallel_mode = use_parallel_mode;
Expand Down
10 changes: 0 additions & 10 deletions src/backend/optimizer/plan/planner.c
Original file line number Diff line number Diff line change
Expand Up @@ -257,16 +257,6 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
* to values that don't permit parallelism, or if parallel-unsafe
* functions are present in the query tree.
*
* (Note that we do allow CREATE TABLE AS, SELECT INTO, and CREATE
* MATERIALIZED VIEW to use parallel plans, but this is safe only because
* the command is writing into a completely new table which workers won't
* be able to see. If the workers could see the table, the fact that
* group locking would cause them to ignore the leader's heavyweight
* relation extension lock and GIN page locks would make this unsafe.
* We'll have to fix that somehow if we want to allow parallel inserts in
* general; updates and deletes have additional problems especially around
* combo CIDs.)
*
* For now, we don't try to use parallel mode if we're running inside a
* parallel worker. We might eventually be able to relax this
* restriction, but for now it seems best not to have parallel workers
Expand Down
79 changes: 0 additions & 79 deletions src/test/regress/expected/write_parallel.out

This file was deleted.

1 change: 0 additions & 1 deletion src/test/regress/parallel_schedule
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ test: rules psql_crosstab amutils

# run by itself so it can run parallel workers
test: select_parallel
test: write_parallel

# no relation related tests can be put in this group
test: publication subscription
Expand Down
1 change: 0 additions & 1 deletion src/test/regress/serial_schedule
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ test: stats_ext
test: rules
test: psql_crosstab
test: select_parallel
test: write_parallel
test: publication
test: subscription
test: amutils
Expand Down
42 changes: 0 additions & 42 deletions src/test/regress/sql/write_parallel.sql

This file was deleted.