Merge pull request #14 from Algunenano/REL_10_CARTO_revert_parallel

Revert #10

Author: Algunenano

CHANGELOG.carto.md

@@ -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

doc/src/sgml/parallel.sgml

@@ -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>
 
@@ -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

src/backend/access/heap/heapam.c

@@ -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)
 	{

src/backend/commands/createas.c

@@ -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

src/backend/commands/explain.c

@@ -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))

src/backend/executor/execMain.c

@@ -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;

src/backend/optimizer/plan/planner.c

@@ -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

src/test/regress/expected/write_parallel.out

@@ -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

src/test/regress/parallel_schedule

@@ -134,7 +134,6 @@ test: stats_ext
 test: rules
 test: psql_crosstab
 test: select_parallel
-test: write_parallel
 test: publication
 test: subscription
 test: amutils