Merge branch 'main' into explain_results_format

.github/workflows/build_and_test.yml

@@ -471,14 +471,30 @@ jobs:
       run: |-
         detected_changes=$(git diff origin/main... --name-only --diff-filter=AM | (grep 'src/test/regress/sql/.*\.sql\|src/test/regress/spec/.*\.spec\|src/test/regress/citus_tests/test/test_.*\.py' || true))
         tests=${detected_changes}
-        if [ -z "$tests" ]; then
-            echo "No test found."
+
+        # split the tests to be skipped --today we only skip upgrade tests
+        skipped_tests=""
+        not_skipped_tests=""
+        for test in $tests; do
+            if [[ $test =~ ^src/test/regress/sql/upgrade_ ]]; then
+                skipped_tests="$skipped_tests $test"
+            else
+                not_skipped_tests="$not_skipped_tests $test"
+            fi
+        done
+
+        if [ ! -z "$skipped_tests" ]; then
+            echo "Skipped tests " $skipped_tests
+        fi
+
+        if [ -z "$not_skipped_tests" ]; then
+            echo "Not detected any tests that flaky test detection should run"
         else
-          echo "Detected tests " $tests
+            echo "Detected tests " $not_skipped_tests
         fi
 
         echo 'tests<<EOF' >> $GITHUB_OUTPUT
-        echo "$tests" >> "$GITHUB_OUTPUT"
+        echo "$not_skipped_tests" >> "$GITHUB_OUTPUT"
         echo 'EOF' >> $GITHUB_OUTPUT
   test-flakyness:
     if: ${{ needs.test-flakyness-pre.outputs.tests != ''}}

src/backend/distributed/executor/adaptive_executor.c

@@ -401,7 +401,7 @@ typedef struct WorkerPool
 
 	/*
 	 * Placement executions destined for worker node, but not assigned to any
-	 * connection and not ready to start.
+	 * connection and ready to start.
 	 */
 	dlist_head readyTaskQueue;
 	int readyTaskCount;
@@ -492,8 +492,6 @@ typedef struct WorkerSession
 } WorkerSession;
 
 
-struct TaskPlacementExecution;
-
 /* GUC, determining whether Citus opens 1 connection per task */
 bool ForceMaxQueryParallelization = false;
 int MaxAdaptiveExecutorPoolSize = 16;
@@ -585,7 +583,7 @@ typedef enum TaskPlacementExecutionState
 } TaskPlacementExecutionState;
 
 /*
- * TaskPlacementExecution represents the an execution of a command
+ * TaskPlacementExecution represents the execution of a command
  * on a shard placement.
  */
 typedef struct TaskPlacementExecution
@@ -1908,7 +1906,7 @@ RunDistributedExecution(DistributedExecution *execution)
 
 		/*
 		 * Iterate until all the tasks are finished. Once all the tasks
-		 * are finished, ensure that that all the connection initializations
+		 * are finished, ensure that all the connection initializations
 		 * are also finished. Otherwise, those connections are terminated
 		 * abruptly before they are established (or failed). Instead, we let
 		 * the ConnectionStateMachine() to properly handle them.
@@ -3118,7 +3116,7 @@ ConnectionStateMachine(WorkerSession *session)
 				 *
 				 * We can only retry connection when the remote transaction has
 				 * not started over the connection. Otherwise, we'd have to deal
-				 * with restoring the transaction state, which iis beyond our
+				 * with restoring the transaction state, which is beyond our
 				 * purpose at this time.
 				 */
 				RemoteTransaction *transaction = &connection->remoteTransaction;

src/backend/distributed/executor/multi_executor.c

@@ -168,7 +168,7 @@ CitusExecutorRun(QueryDesc *queryDesc,
 	executorBoundParams = queryDesc->params;
 
 	/*
-	 * We do some potentially time consuming operations our self now before we hand of
+	 * We do some potentially time consuming operations ourself now before we hand off
 	 * control to postgres' executor. To make sure that time spent is accurately measured
 	 * we remove the totaltime instrumentation from the queryDesc. Instead we will start
 	 * and stop the instrumentation of the total time and put it back on the queryDesc

src/backend/distributed/planner/fast_path_router_planner.c

@@ -252,7 +252,7 @@ FastPathRouterQuery(Query *query, Node **distributionKeyValue)
 
 	/*
 	 * Distribution column must be used in a simple equality match check and it must be
-	 * place at top level conjustion operator. In simple words, we should have
+	 * place at top level conjunction operator. In simple words, we should have
 	 *	    WHERE dist_key = VALUE [AND  ....];
 	 *
 	 *	We're also not allowing any other appearances of the distribution key in the quals.

src/backend/distributed/planner/multi_logical_planner.c

@@ -715,8 +715,8 @@ MultiNodeTree(Query *queryTree)
 
 
 /*
- * ContainsReadIntermediateResultFunction determines whether an expresion tree contains
- * a call to the read_intermediate_result function.
+ * ContainsReadIntermediateResultFunction determines whether an expression tree
+ * contains a call to the read_intermediate_result function.
  */
 bool
 ContainsReadIntermediateResultFunction(Node *node)
@@ -726,7 +726,7 @@ ContainsReadIntermediateResultFunction(Node *node)
 
 
 /*
- * ContainsReadIntermediateResultArrayFunction determines whether an expresion
+ * ContainsReadIntermediateResultArrayFunction determines whether an expression
  * tree contains a call to the read_intermediate_results(result_ids, format)
  * function.
  */

src/backend/distributed/planner/multi_router_planner.c

@@ -434,7 +434,7 @@ ExtractSelectRangeTableEntry(Query *query)
  * for the given modification query.
  *
  * The function errors out if the input query is not a
- * modify query (e.g., INSERT, UPDATE or DELETE). So, this
+ * modify query (e.g., INSERT, UPDATE, DELETE or MERGE). So, this
  * function is not expected to be called on SELECT queries.
  */
 Oid
@@ -2271,13 +2271,13 @@ SelectsFromDistributedTable(List *rangeTableList, Query *query)
 
 
 /*
- * RouterQuery runs router pruning logic for SELECT, UPDATE, DELETE, and MERGE queries.
- * If there are shards present and query is routable, all RTEs have been updated
- * to point to the relevant shards in the originalQuery. Also, placementList is
- * filled with the list of worker nodes that has all the required shard placements
- * for the query execution. anchorShardId is set to the first pruned shardId of
- * the given query. Finally, relationShardList is filled with the list of
- * relation-to-shard mappings for the query.
+ * PlanRouterQuery runs router pruning logic for SELECT, UPDATE, DELETE, and
+ * MERGE queries. If there are shards present and query is routable, all RTEs
+ * have been updated to point to the relevant shards in the originalQuery. Also,
+ * placementList is filled with the list of worker nodes that has all the
+ * required shard placements for the query execution. anchorShardId is set to
+ * the first pruned shardId of the given query. Finally, relationShardList is
+ * filled with the list of relation-to-shard mappings for the query.
  *
  * If the given query is not routable, it fills planningError with the related
  * DeferredErrorMessage. The caller can check this error message to see if query
@@ -2510,7 +2510,7 @@ AllShardsColocated(List *relationShardList)
 			if (currentTableType == RANGE_DISTRIBUTED ||
 				currentTableType == APPEND_DISTRIBUTED)
 			{
-				/* we do not have further strict colocation chceks */
+				/* we do not have further strict colocation checks */
 				continue;
 			}
 		}
@@ -2932,7 +2932,7 @@ TargetShardIntervalsForRestrictInfo(RelationRestrictionContext *restrictionConte
 	}
 
 	/*
-	 * Different resrictions might have different partition columns.
+	 * Different restrictions might have different partition columns.
 	 * We report partition column value if there is only one.
 	 */
 	if (multiplePartitionValuesExist)

src/include/distributed/distributed_planner.h

@@ -104,7 +104,7 @@ typedef struct FastPathRestrictionContext
 	 * Set to true when distKey = Param; in the queryTree
 	 */
 	bool distributionKeyHasParam;
-}FastPathRestrictionContext;
+} FastPathRestrictionContext;
 
 typedef struct PlannerRestrictionContext
 {

src/include/distributed/multi_physical_planner.h

@@ -238,8 +238,8 @@ typedef struct Task
 	TaskQuery taskQuery;
 
 	/*
-	 * A task can have multiple queries, in which case queryCount will be > 1. If
-	 * a task has more one query, then taskQuery->queryType == TASK_QUERY_TEXT_LIST.
+	 * A task can have multiple queries, in which case queryCount will be > 1, and
+	 * taskQuery->queryType == TASK_QUERY_TEXT_LIST.
 	 */
 	int queryCount;
 
@@ -290,7 +290,7 @@ typedef struct Task
 
 	/*
 	 * When we evaluate functions and parameters in the query string then
-	 * we should no longer send the list of parameters long with the
+	 * we should no longer send the list of parameters along with the
 	 * query.
 	 */
 	bool parametersInQueryStringResolved;

src/test/regress/before_citus_upgrade_coord_schedule

@@ -1,5 +1,5 @@
 # this schedule is to be run on only coordinators
 
-test: upgrade_basic_before
+test: upgrade_basic_before upgrade_basic_before_non_mixed
 test: upgrade_pg_dist_cleanup_before
 test: upgrade_post_11_before

src/test/regress/before_pg_upgrade_schedule

@@ -1,5 +1,5 @@
 # The basic tests runs analyze which depends on shard numbers
-test: multi_test_helpers multi_test_helpers_superuser
+test: multi_test_helpers multi_test_helpers_superuser upgrade_basic_before_non_mixed
 test: multi_test_catalog_views
 test: upgrade_basic_before
 test: upgrade_ref2ref_before

src/test/regress/bin/normalize.sed

@@ -215,7 +215,7 @@ s/^(ERROR:  The index name \(test_index_creation1_p2020_09_26)_([0-9])+_(tenant_
 s/^(DEBUG:  the index name on the shards of the partition is too long, switching to sequential and local execution mode to prevent self deadlocks: test_index_creation1_p2020_09_26)_([0-9])+_(tenant_id_timeperiod_idx)/\1_xxxxxx_\3/g
 
 # normalize errors for not being able to connect to a non-existing host
-s/could not translate host name "foobar" to address: .*$/could not translate host name "foobar" to address: <system specific error>/g
+s/could not translate host name "([A-Za-z0-9\.\-]+)" to address: .*$/could not translate host name "\1" to address: <system specific error>/g
 
 # ignore PL/pgSQL line numbers that differ on Mac builds
 s/(CONTEXT:  PL\/pgSQL function .* line )([0-9]+)/\1XX/g

src/test/regress/citus_tests/common.py

@@ -92,7 +92,7 @@ PG_MAJOR_VERSION = get_pg_major_version()
 OLDEST_SUPPORTED_CITUS_VERSION_MATRIX = {
     14: "10.2.0",
     15: "11.1.5",
-    16: "12.1devel",
+    16: "12.1.1",
 }
 
 OLDEST_SUPPORTED_CITUS_VERSION = OLDEST_SUPPORTED_CITUS_VERSION_MATRIX[PG_MAJOR_VERSION]

src/test/regress/expected/isolation_update_node.out

@@ -250,7 +250,7 @@ count
 step s1-commit-prepared:
     COMMIT prepared 'label';
 
-s2: WARNING:  connection to the remote node non-existent:57637 failed with the following error: could not translate host name "non-existent" to address: Name or service not known
+s2: WARNING:  connection to the remote node non-existent:57637 failed with the following error: could not translate host name "non-existent" to address: <system specific error>
 step s2-execute-prepared:
     EXECUTE foo;