This is an automated email from the ASF dual-hosted git repository. maxyang pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit 0ffc4af83d9fc1167743ed7a191a13a4017f1eb2 Author: David Kimura <[email protected]> AuthorDate: Mon Aug 1 11:24:35 2022 -0700 ORCA Update reset stat to handle cyclic memo path (#13879) Commit 2d49b616fe updated memo group reset to include reset of a group's duplicate. Previously, group reset would recursively traverse only the children. However, by also traversing duplicates it became possible to form cyclic reset paths in the memo (e.g. a group's child is a duplicate of parent group). This can lead to a infinite reset loop. Admittedly, this patch should only be a temporary solution. Intent of the function FResetStats() is to only reset if logical operators were added to any group reachable from reset group. In order to do that we must first search the children before resetting ourselves. Ultimately, we need a way to properly detect cycles. Co-authored-by: Jingyu Wang <[email protected]> --- src/backend/gporca/libgpopt/src/search/CGroup.cpp | 14 ++++++++++++-- src/test/regress/expected/qp_with_clause.out | 2 -- src/test/regress/expected/qp_with_clause_optimizer.out | 2 -- src/test/regress/sql/qp_with_clause.sql | 2 -- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/backend/gporca/libgpopt/src/search/CGroup.cpp b/src/backend/gporca/libgpopt/src/search/CGroup.cpp index 35877d8434..9b709a0b20 100644 --- a/src/backend/gporca/libgpopt/src/search/CGroup.cpp +++ b/src/backend/gporca/libgpopt/src/search/CGroup.cpp @@ -1451,7 +1451,8 @@ CGroup::PstatsRecursiveDerive(CMemoryPool *pmpLocal, CMemoryPool *pmpGlobal, } IStatistics *stats = nullptr; - // if this is a duplicate group, return stats from the duplicate + // if a duplicate group is found, the stats is derived of the duplicate + // group and returned, no stats will be derived of the group itself if (FDuplicateGroup()) { // call stat derivation on the duplicate group @@ -1905,6 +1906,9 @@ CGroup::FResetStats() return true; } + // if a duplicate group is found, the duplicate group stats will be + // reset, whereas the group's own stats (most likely NULL) won't be + // reset if (FDuplicateGroup()) { return PgroupDuplicate()->FResetStats(); @@ -1932,7 +1936,13 @@ CGroup::FResetStats() GPOS_CHECK_ABORT; CGroup *pgroupChild = (*pgexprCurrent)[ul]; - if (!pgroupChild->FScalar() && pgroupChild->FResetStats()) + if (!pgroupChild->FScalar() && + // FIXME: Scenario exists where this group is child's duplicate + // group. When the case arises, the child's children + // won’t be reset. Otherwise we can hit infinite loop + // that resets child->this->child. + (this == pgroupChild->PgroupDuplicate() || + pgroupChild->FResetStats())) { fResetStats = true; // we cannot break here since we must visit all child groups diff --git a/src/test/regress/expected/qp_with_clause.out b/src/test/regress/expected/qp_with_clause.out index 88d1c8d345..1c1ed6d20d 100644 --- a/src/test/regress/expected/qp_with_clause.out +++ b/src/test/regress/expected/qp_with_clause.out @@ -615,7 +615,6 @@ where e1.code = e2.code order by e2.COUNTRY,e1.language LIMIT 20; GIB | Gibraltar | Gibraltar | English | t | 88.9 | GIB | Gibraltar | Gibraltar | English | t | 88.9 (20 rows) -SET optimizer_trace_fallback=off; -- query 2 using multiple CTEs with same names as tables. with country as (select country.code,country.name COUNTRY, city.name CAPITAL, language, isofficial, percentage @@ -657,7 +656,6 @@ order by COUNTRY,percentage1 LIMIT 20;-- queries using same name for CTEs and th GIB | Gibraltar | Gibraltar | English | t | 88.9 | Gibraltar (20 rows) -SET optimizer_trace_fallback=on; -- query1 with c1 as (select country.code,country.name COUNTRY, city.name CAPITAL, language, isofficial, percentage diff --git a/src/test/regress/expected/qp_with_clause_optimizer.out b/src/test/regress/expected/qp_with_clause_optimizer.out index b6bf86f931..766890f49d 100644 --- a/src/test/regress/expected/qp_with_clause_optimizer.out +++ b/src/test/regress/expected/qp_with_clause_optimizer.out @@ -615,7 +615,6 @@ where e1.code = e2.code order by e2.COUNTRY,e1.language LIMIT 20; GIB | Gibraltar | Gibraltar | English | t | 88.9 | GIB | Gibraltar | Gibraltar | English | t | 88.9 (20 rows) -SET optimizer_trace_fallback=off; -- query 2 using multiple CTEs with same names as tables. with country as (select country.code,country.name COUNTRY, city.name CAPITAL, language, isofficial, percentage @@ -657,7 +656,6 @@ order by COUNTRY,percentage1 LIMIT 20;-- queries using same name for CTEs and th GIB | Gibraltar | Gibraltar | English | t | 88.9 | Gibraltar (20 rows) -SET optimizer_trace_fallback=on; -- query1 with c1 as (select country.code,country.name COUNTRY, city.name CAPITAL, language, isofficial, percentage diff --git a/src/test/regress/sql/qp_with_clause.sql b/src/test/regress/sql/qp_with_clause.sql index b31b7353a6..4613eb6253 100644 --- a/src/test/regress/sql/qp_with_clause.sql +++ b/src/test/regress/sql/qp_with_clause.sql @@ -5607,7 +5607,6 @@ select * from (select * from country where percentage > 50) e2 where e1.code = e2.code order by e2.COUNTRY,e1.language LIMIT 20; -SET optimizer_trace_fallback=off; -- query 2 using multiple CTEs with same names as tables. with country as (select country.code,country.name COUNTRY, city.name CAPITAL, language, isofficial, percentage @@ -5625,7 +5624,6 @@ where e1.code = e2.code order by e2.COUNTRY,e1.language select code1,country1,capital1,language1,isofficial1,percentage1,country.COUNTRY from country,countrylanguage where country.code = countrylanguage.code1 and country.percentage = countrylanguage.percentage1 order by COUNTRY,percentage1 LIMIT 20;-- queries using same name for CTEs and the subquery aliases in the main query -SET optimizer_trace_fallback=on; -- query1 with c1 as --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
