[clang-tidy] Add readability braces around statements (#13730)

Enable config "readability-braces-around-statements" [1]. Also, commit
d045742 had config syntax error that unintentionally disabled tidy checks.
Auto fix up the files after fixing that mistake.

[1] https://releases.llvm.org/11.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/readability-braces-around-statements.html

Author: dgkimura

.clang-tidy

@@ -9,7 +9,8 @@ Checks: |
   modernize-use-nullptr,
   modernize-use-default-member-init,
   modernize-use-bool-literals,
-  modernize-use-using
+  modernize-use-using,
+  readability-braces-around-statements,
   readability-make-member-function-const,
   readability-non-const-parameter,
   readability-static-accessed-through-instance,

src/backend/gpopt/CGPOptimizer.cpp

@@ -133,15 +133,19 @@ CGPOptimizer::GPOPTOptimizedPlan(
 				errmsg(
 					"GPORCA failed to produce a plan, falling back to planner");
 				if (serialized_error_msg)
+				{
 					errdetail("%s", serialized_error_msg);
+				}
 				errfinish(ex.Filename(), ex.Line(), nullptr);
 			}
 		}
 
 		*had_unexpected_failure = gpopt_context.m_is_unexpected_failure;
 
 		if (serialized_error_msg)
+		{
 			pfree(serialized_error_msg);
+		}
 	}
 	GPOS_CATCH_END;
 	return plStmt;

src/backend/gpopt/gpdbwrappers.cpp

@@ -473,7 +473,9 @@ gpdb::TypeCollation(Oid type)
 		if (OidIsValid(typcollation))
 		{
 			if (type == NAMEOID)
+			{
 				return typcollation;  // As of v12, this is C_COLLATION_OID
+			}
 			return DEFAULT_COLLATION_OID;
 		}
 		return collation;
@@ -1661,7 +1663,9 @@ gpdb::GpdbEreportImpl(int xerrcode, int severitylevel, const char *xerrmsg,
 			errcode(xerrcode);
 			errmsg("%s", xerrmsg);
 			if (xerrhint)
+			{
 				errhint("%s", xerrhint);
+			}
 			errfinish(filename, lineno, funcname);
 		}
 	}
@@ -2633,7 +2637,9 @@ gpdb::MDCacheNeedsReset(void)
 			mdcache_invalidation_counter_registered = true;
 		}
 		if (last_mdcache_invalidation_counter == mdcache_invalidation_counter)
+		{
 			return false;
+		}
 		else
 		{
 			last_mdcache_invalidation_counter = mdcache_invalidation_counter;

src/backend/gpopt/translate/CContextDXLToPlStmt.cpp

@@ -524,8 +524,10 @@ CContextDXLToPlStmt::GetStaticPruneResult(ULONG scanId)
 	// GPDB_12_MERGE_FIXME: we haven't seen the scan id yet, this scan id is likely for dynamic pruning.
 	// When we can, remove this check
 	if ((scanId - 1) >= m_static_prune_results.size())
+	{
 		GPOS_RAISE(gpdxl::ExmaDXL, gpdxl::ExmiDXL2PlStmtConversion,
 				   GPOS_WSZ_LIT("dynamic pruning"));
+	}
 	return m_static_prune_results[scanId - 1];
 }
 void

src/backend/gpopt/translate/CQueryMutators.cpp

@@ -836,11 +836,15 @@ CQueryMutators::MakeVarInDerivedTable(Node *node,
 	const ULONG attno = gpdb::ListLength(context->m_lower_table_tlist) + 1;
 	TargetEntry *tle = nullptr;
 	if (IsA(node, Aggref) || IsA(node, GroupingFunc))
+	{
 		tle = GetTargetEntryForAggExpr(context->m_mp, context->m_mda, node,
 									   attno);
+	}
 	else if (IsA(node, Var))
+	{
 		tle = gpdb::MakeTargetEntry((Expr *) node, (AttrNumber) attno, nullptr,
 									false);
+	}
 
 	context->m_lower_table_tlist =
 		gpdb::LAppend(context->m_lower_table_tlist, tle);

src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp

@@ -2871,19 +2871,33 @@ CTranslatorDXLToPlStmt::TranslateDXLWindow(
 					win_frame_leading_dxlnode->GetOperator())
 					->ParseDXLFrameBoundary();
 			if (lead_boundary_type == EdxlfbUnboundedPreceding)
+			{
 				window->frameOptions |= FRAMEOPTION_END_UNBOUNDED_PRECEDING;
+			}
 			if (lead_boundary_type == EdxlfbBoundedPreceding)
+			{
 				window->frameOptions |= FRAMEOPTION_END_OFFSET_PRECEDING;
+			}
 			if (lead_boundary_type == EdxlfbCurrentRow)
+			{
 				window->frameOptions |= FRAMEOPTION_END_CURRENT_ROW;
+			}
 			if (lead_boundary_type == EdxlfbBoundedFollowing)
+			{
 				window->frameOptions |= FRAMEOPTION_END_OFFSET_FOLLOWING;
+			}
 			if (lead_boundary_type == EdxlfbUnboundedFollowing)
+			{
 				window->frameOptions |= FRAMEOPTION_END_UNBOUNDED_FOLLOWING;
+			}
 			if (lead_boundary_type == EdxlfbDelayedBoundedPreceding)
+			{
 				window->frameOptions |= FRAMEOPTION_END_OFFSET_PRECEDING;
+			}
 			if (lead_boundary_type == EdxlfbDelayedBoundedFollowing)
+			{
 				window->frameOptions |= FRAMEOPTION_END_OFFSET_FOLLOWING;
+			}
 			if (0 != win_frame_leading_dxlnode->Arity())
 			{
 				window->endOffset =
@@ -2899,19 +2913,33 @@ CTranslatorDXLToPlStmt::TranslateDXLWindow(
 					win_frame_trailing_dxlnode->GetOperator())
 					->ParseDXLFrameBoundary();
 			if (trail_boundary_type == EdxlfbUnboundedPreceding)
+			{
 				window->frameOptions |= FRAMEOPTION_START_UNBOUNDED_PRECEDING;
+			}
 			if (trail_boundary_type == EdxlfbBoundedPreceding)
+			{
 				window->frameOptions |= FRAMEOPTION_START_OFFSET_PRECEDING;
+			}
 			if (trail_boundary_type == EdxlfbCurrentRow)
+			{
 				window->frameOptions |= FRAMEOPTION_START_CURRENT_ROW;
+			}
 			if (trail_boundary_type == EdxlfbBoundedFollowing)
+			{
 				window->frameOptions |= FRAMEOPTION_START_OFFSET_FOLLOWING;
+			}
 			if (trail_boundary_type == EdxlfbUnboundedFollowing)
+			{
 				window->frameOptions |= FRAMEOPTION_START_UNBOUNDED_FOLLOWING;
+			}
 			if (trail_boundary_type == EdxlfbDelayedBoundedPreceding)
+			{
 				window->frameOptions |= FRAMEOPTION_START_OFFSET_PRECEDING;
+			}
 			if (trail_boundary_type == EdxlfbDelayedBoundedFollowing)
+			{
 				window->frameOptions |= FRAMEOPTION_START_OFFSET_FOLLOWING;
+			}
 			if (0 != win_frame_trailing_dxlnode->Arity())
 			{
 				window->startOffset =
@@ -2923,7 +2951,9 @@ CTranslatorDXLToPlStmt::TranslateDXLWindow(
 			child_contexts->Release();
 		}
 		else
+		{
 			window->frameOptions = FRAMEOPTION_DEFAULTS;
+		}
 	}
 
 	SetParamIds(plan);
@@ -3128,7 +3158,9 @@ ContainsSetReturningFuncOrOp(const CDXLNode *project_list_dxlnode,
 		{
 			case EdxlopScalarFuncExpr:
 				if (CDXLScalarFuncExpr::Cast(op)->ReturnsSet())
+				{
 					return true;
+				}
 				break;
 			case EdxlopScalarOpExpr:
 			{
@@ -3137,7 +3169,9 @@ ContainsSetReturningFuncOrOp(const CDXLNode *project_list_dxlnode,
 				const IMDFunction *md_func =
 					md_accessor->RetrieveFunc(md_sclar_op->FuncMdId());
 				if (md_func->ReturnsSet())
+				{
 					return true;
+				}
 				break;
 			}
 			default:
@@ -3161,16 +3195,24 @@ SanityCheckProjectSetTargetList(List *targetlist)
 			(IsA(expr, OpExpr) && ((OpExpr *) expr)->opretset))
 		{
 			if (IsA(expr, FuncExpr))
+			{
 				args = ((FuncExpr *) expr)->args;
+			}
 			else
+			{
 				args = ((OpExpr *) expr)->args;
+			}
 			if (gpdb::ExpressionReturnsSet((Node *) args))
+			{
 				return false;
+			}
 			continue;
 		}
 
 		if (gpdb::ExpressionReturnsSet((Node *) expr))
+		{
 			return false;
+		}
 	}
 	return true;
 }
@@ -3185,9 +3227,11 @@ CTranslatorDXLToPlStmt::TranslateDXLProjectSet(
 	// GPDB_12_MERGE_FIXME: had we generated a DXLProjectSet in ORCA we wouldn't
 	// have needed to be defensive here...
 	if ((*result_dxlnode)[EdxlresultIndexFilter]->Arity() > 0)
+	{
 		GPOS_RAISE(
 			gpdxl::ExmaDXL, gpdxl::ExmiQuery2DXLUnsupportedFeature,
 			GPOS_WSZ_LIT("Unsupported one-time filter in ProjectSet node"));
+	}
 
 	// create project set (nee result) plan node
 	ProjectSet *project_set = MakeNode(ProjectSet);
@@ -3241,9 +3285,11 @@ CTranslatorDXLToPlStmt::TranslateDXLProjectSet(
 	// double check the targetlist is kosher
 	// we are only doing this because ORCA didn't do it...
 	if (!SanityCheckProjectSetTargetList(plan->targetlist))
+	{
 		GPOS_RAISE(
 			gpdxl::ExmaDXL, gpdxl::ExmiQuery2DXLUnsupportedFeature,
 			GPOS_WSZ_LIT("Unexpected target list entries in ProjectSet node"));
+	}
 
 	return (Plan *) project_set;
 }
@@ -3267,8 +3313,10 @@ CTranslatorDXLToPlStmt::TranslateDXLResult(
 	// actual result node
 	if (ContainsSetReturningFuncOrOp((*result_dxlnode)[EdxlresultIndexProjList],
 									 m_md_accessor))
+	{
 		return TranslateDXLProjectSet(result_dxlnode, output_context,
 									  ctxt_translation_prev_siblings);
+	}
 
 	// create result plan node
 	Result *result = MakeNode(Result);
@@ -3331,9 +3379,11 @@ CTranslatorDXLToPlStmt::TranslateDXLResult(
 	// double check the targetlist is kosher
 	// we are only doing this because ORCA didn't do it...
 	if (!SanityCheckProjectSetTargetList(plan->targetlist))
+	{
 		GPOS_RAISE(
 			gpdxl::ExmaDXL, gpdxl::ExmiQuery2DXLUnsupportedFeature,
 			GPOS_WSZ_LIT("Unexpected target list entries in ProjectSet node"));
+	}
 
 	return (Plan *) result;
 }
@@ -3974,9 +4024,11 @@ CTranslatorDXLToPlStmt::TranslateDXLDml(
 	// partition Oid in the child's target list, but we don't use it for
 	// anything in GPDB.
 	if (m_cmd_type == CMD_UPDATE)
+	{
 		(void) AddJunkTargetEntryForColId(&dml_target_list, &child_context,
 										  phy_dml_dxlop->ActionColId(),
 										  "DMLAction");
+	}
 
 	if (m_cmd_type == CMD_UPDATE || m_cmd_type == CMD_DELETE)
 	{
@@ -3987,8 +4039,10 @@ CTranslatorDXLToPlStmt::TranslateDXLDml(
 								   "gp_segment_id");
 	}
 	if (m_cmd_type == CMD_UPDATE && phy_dml_dxlop->IsOidsPreserved())
+	{
 		AddJunkTargetEntryForColId(&dml_target_list, &child_context,
 								   phy_dml_dxlop->GetTupleOid(), "oid");
+	}
 
 	// Add a Result node on top of the child plan, to coerce the target
 	// list to match the exact physical layout of the target table,
@@ -4020,7 +4074,9 @@ CTranslatorDXLToPlStmt::TranslateDXLDml(
 
 	// ORCA plans all updates as split updates
 	if (m_cmd_type == CMD_UPDATE)
+	{
 		dml->isSplitUpdates = ListMake1Int((int) true);
+	}
 
 	plan->targetlist = NIL;
 	plan->plan_node_id = m_dxl_to_plstmt_context->GetNextPlanId();

src/backend/gpopt/translate/CTranslatorDXLToScalar.cpp

@@ -730,8 +730,10 @@ CTranslatorDXLToScalar::TranslateDXLScalarWindowRefToScalar(
 	window_func->winagg = dxlop->IsSimpleAgg();
 
 	if (dxlop->GetDxlWinStage() != EdxlwinstageImmediate)
+	{
 		GPOS_RAISE(gpdxl::ExmaDXL, gpdxl::ExmiQuery2DXLError,
 				   GPOS_WSZ_LIT("Unsupported window function stage"));
+	}
 
 	// translate the arguments of the window function
 	window_func->args = TranslateScalarChildren(window_func->args,

src/backend/gpopt/translate/CTranslatorQueryToDXL.cpp

@@ -155,9 +155,13 @@ CTranslatorQueryToDXL::CTranslatorQueryToDXL(
 	// If this is a subquery, make a copy of the parent's mapping, otherwise
 	// initialize a new, empty, mapping.
 	if (var_colid_mapping)
+	{
 		m_var_to_colid_map = var_colid_mapping->CopyMapColId(m_mp);
+	}
 	else
+	{
 		m_var_to_colid_map = GPOS_NEW(m_mp) CMappingVarColId(m_mp);
+	}
 
 	m_query_level_to_cte_map = GPOS_NEW(m_mp) HMUlCTEListEntry(m_mp);
 	m_dxl_cte_producers = GPOS_NEW(m_mp) CDXLNodeArray(m_mp);
@@ -266,7 +270,9 @@ CTranslatorQueryToDXL::~CTranslatorQueryToDXL()
 	CRefCount::SafeRelease(m_dxl_query_output_cols);
 
 	if (m_query_level == 0)
+	{
 		GPOS_DELETE(m_context);
+	}
 }
 
 //---------------------------------------------------------------------------
@@ -521,13 +527,17 @@ CTranslatorQueryToDXL::TranslateSelectQueryToDXL()
 
 	// RETURNING is not supported yet.
 	if (m_query->returningList)
+	{
 		GPOS_RAISE(gpdxl::ExmaDXL, gpdxl::ExmiQuery2DXLUnsupportedFeature,
 				   GPOS_WSZ_LIT("RETURNING clause"));
+	}
 
 	// ON CONFLICT is not supported yet.
 	if (m_query->onConflict)
+	{
 		GPOS_RAISE(gpdxl::ExmaDXL, gpdxl::ExmiQuery2DXLUnsupportedFeature,
 				   GPOS_WSZ_LIT("ON CONFLICT clause"));
+	}
 
 	CDXLNode *child_dxlnode = nullptr;
 	IntToUlongMap *sort_group_attno_to_colid_mapping =
@@ -3321,9 +3331,13 @@ CTranslatorQueryToDXL::NoteDistributionPolicyOpclasses(const RangeTblEntry *rte)
 					gpdb::GetDefaultDistributionOpclassForType(typeoid);
 
 				if (opclasses[i] == default_opclass)
+				{
 					contains_default_hashops = true;
+				}
 				else
+				{
 					contains_nondefault_hashops = true;
+				}
 			}
 		}
 
@@ -3340,21 +3354,25 @@ CTranslatorQueryToDXL::NoteDistributionPolicyOpclasses(const RangeTblEntry *rte)
 		{
 			if (m_context->m_distribution_hashops !=
 				DistrHashOpsNotDeterminedYet)
+			{
 				GPOS_RAISE(
 					gpdxl::ExmaMD, gpdxl::ExmiMDObjUnsupported,
 					GPOS_WSZ_LIT(
 						"Query contains relations with a mix of default and legacy hash opclasses"));
+			}
 			m_context->m_distribution_hashops = DistrUseDefaultHashOps;
 		}
 		if (contains_legacy_hashops &&
 			m_context->m_distribution_hashops != DistrUseLegacyHashOps)
 		{
 			if (m_context->m_distribution_hashops !=
 				DistrHashOpsNotDeterminedYet)
+			{
 				GPOS_RAISE(
 					gpdxl::ExmaMD, gpdxl::ExmiMDObjUnsupported,
 					GPOS_WSZ_LIT(
 						"Query contains relations with a mix of default and legacy hash opclasses"));
+			}
 			m_context->m_distribution_hashops = DistrUseLegacyHashOps;
 		}
 	}
@@ -3912,7 +3930,9 @@ CTranslatorQueryToDXL::TranslateJoinExprInFromToDXL(JoinExpr *join_expr)
 		Node *join_alias_node = (Node *) lfirst(lc_node);
 		// rte->joinaliasvars may contain NULL ptrs which indicates dropped columns
 		if (!join_alias_node)
+		{
 			continue;
+		}
 		GPOS_ASSERT(IsA(join_alias_node, Var) ||
 					IsA(join_alias_node, CoalesceExpr));
 		Value *value = (Value *) lfirst(lc_col_name);