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 98da87b5295987c1d17c3a14fb398ddc64b36290
Author: David Kimura <[email protected]>
AuthorDate: Tue Sep 26 10:24:46 2023 -0700

    Add GUC optimizer_enable_orderedagg to enable/disable ordered aggregates 
(#16491)
    
    ORCA enabled ordered-set aggregate functions in commit ad40cc6. In 6X, the
    feature is disabled by default behind GUC optimizer_enable_orderedagg. In 
7X,
    GUC is enabled by default. That provides an out in case of a regression.
---
 src/backend/gpopt/config/CConfigParamMapping.cpp           |  5 +++++
 src/backend/gpopt/translate/CTranslatorScalarToDXL.cpp     |  8 ++++++++
 .../libnaucrates/include/naucrates/traceflags/traceflags.h |  3 +++
 src/backend/utils/misc/guc_gp.c                            | 10 ++++++++++
 src/include/utils/guc.h                                    |  1 +
 src/include/utils/unsync_guc_name.h                        |  1 +
 src/test/regress/expected/qp_orca_fallback.out             | 14 ++++++++++++++
 src/test/regress/expected/qp_orca_fallback_optimizer.out   | 11 +++++++++++
 src/test/regress/sql/qp_orca_fallback.sql                  |  5 +++++
 9 files changed, 58 insertions(+)

diff --git a/src/backend/gpopt/config/CConfigParamMapping.cpp 
b/src/backend/gpopt/config/CConfigParamMapping.cpp
index 873c57099a..53a8d77af9 100644
--- a/src/backend/gpopt/config/CConfigParamMapping.cpp
+++ b/src/backend/gpopt/config/CConfigParamMapping.cpp
@@ -270,6 +270,11 @@ CConfigParamMapping::SConfigMappingElem 
CConfigParamMapping::m_elements[] = {
         false,  // m_negate_param
         GPOS_WSZ_LIT(
                 "Enable Eager Agg transform for pushing aggregate below an 
innerjoin.")},
+
+       {EopttraceDisableOrderedAgg, &optimizer_enable_orderedagg,
+        true,  // m_negate_param
+        GPOS_WSZ_LIT("Disable ordered aggregate plans.")},
+
        {EopttraceExpandFullJoin, &optimizer_expand_fulljoin,
         false,  // m_negate_param
         GPOS_WSZ_LIT(
diff --git a/src/backend/gpopt/translate/CTranslatorScalarToDXL.cpp 
b/src/backend/gpopt/translate/CTranslatorScalarToDXL.cpp
index 0759d103d7..2a7018e6f1 100644
--- a/src/backend/gpopt/translate/CTranslatorScalarToDXL.cpp
+++ b/src/backend/gpopt/translate/CTranslatorScalarToDXL.cpp
@@ -1395,6 +1395,14 @@ CTranslatorScalarToDXL::TranslateAggrefToDXL(
        const Aggref *aggref = (Aggref *) expr;
        BOOL is_distinct = false;
 
+       if (aggref->aggorder != NIL && GPOS_FTRACE(EopttraceDisableOrderedAgg))
+       {
+               GPOS_RAISE(
+                       gpdxl::ExmaDXL, gpdxl::ExmiQuery2DXLUnsupportedFeature,
+                       GPOS_WSZ_LIT(
+                               "Ordered aggregates disabled. Enable by setting 
optimizer_enable_orderedagg=on"));
+       }
+
        if (aggref->aggdistinct)
        {
                is_distinct = true;
diff --git 
a/src/backend/gporca/libnaucrates/include/naucrates/traceflags/traceflags.h 
b/src/backend/gporca/libnaucrates/include/naucrates/traceflags/traceflags.h
index c45f264c6d..f37599064d 100644
--- a/src/backend/gporca/libnaucrates/include/naucrates/traceflags/traceflags.h
+++ b/src/backend/gporca/libnaucrates/include/naucrates/traceflags/traceflags.h
@@ -223,6 +223,9 @@ enum EOptTraceFlag
        // enable nested loop join alternatives
        EopttraceDisableInnerNLJ = 103046,
 
+       // Ordered Agg
+       EopttraceDisableOrderedAgg = 103047,
+
        ///////////////////////////////////////////////////////
        ///////////////////// statistics flags ////////////////
        //////////////////////////////////////////////////////
diff --git a/src/backend/utils/misc/guc_gp.c b/src/backend/utils/misc/guc_gp.c
index b50f55d102..c07704427b 100644
--- a/src/backend/utils/misc/guc_gp.c
+++ b/src/backend/utils/misc/guc_gp.c
@@ -414,6 +414,7 @@ bool                optimizer_enable_eageragg;
 bool           optimizer_enable_range_predicate_dpe;
 bool           optimizer_enable_use_distribution_in_dqa;
 bool           optimizer_enable_push_join_below_union_all;
+bool           optimizer_enable_orderedagg;
 
 /* Analyze related GUCs for Optimizer */
 bool           optimizer_analyze_root_partition;
@@ -2957,6 +2958,15 @@ struct config_bool ConfigureNamesBool_gp[] =
                false,
                NULL, NULL, NULL
        },
+       {
+               {"optimizer_enable_orderedagg", PGC_USERSET, DEVELOPER_OPTIONS,
+                       gettext_noop("Enable ordered aggregate plans."),
+                       NULL
+               },
+               &optimizer_enable_orderedagg,
+               true,
+               NULL, NULL, NULL
+       },
        {
                {"optimizer_enable_eageragg", PGC_USERSET, DEVELOPER_OPTIONS,
                        gettext_noop("Enable Eager Agg transform for pushing 
aggregate below an innerjoin."),
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index a6d3e9e3c8..797c3f8f2d 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -530,6 +530,7 @@ extern bool optimizer_enable_indexscan;
 extern bool optimizer_enable_indexonlyscan;
 extern bool optimizer_enable_tablescan;
 extern bool optimizer_enable_eageragg;
+extern bool optimizer_enable_orderedagg;
 extern bool optimizer_expand_fulljoin;
 extern bool optimizer_enable_hashagg;
 extern bool optimizer_enable_groupagg;
diff --git a/src/include/utils/unsync_guc_name.h 
b/src/include/utils/unsync_guc_name.h
index 6f8e02e6f0..4db424338a 100644
--- a/src/include/utils/unsync_guc_name.h
+++ b/src/include/utils/unsync_guc_name.h
@@ -398,6 +398,7 @@
                "optimizer_enable_dynamictablescan",
                "optimizer_enable_dynamicindexonlyscan",
                "optimizer_enable_eageragg",
+               "optimizer_enable_orderedagg",
                "optimizer_enable_gather_on_segment_for_dml",
                "optimizer_enable_groupagg",
                "optimizer_enable_hashagg",
diff --git a/src/test/regress/expected/qp_orca_fallback.out 
b/src/test/regress/expected/qp_orca_fallback.out
index 7b1cfee813..9c025590a9 100644
--- a/src/test/regress/expected/qp_orca_fallback.out
+++ b/src/test/regress/expected/qp_orca_fallback.out
@@ -276,3 +276,17 @@ explain delete from ext_part where a=1;
 
 explain update ext_part set a=1;
 ERROR:  cannot update foreign table "p2_ext"
+set optimizer_enable_orderedagg=off;
+select array_agg(a order by b)
+  from (values (1,4),(2,3),(3,1),(4,2)) v(a,b);
+ array_agg 
+-----------
+ {3,4,2,1}
+(1 row)
+
+reset optimizer_enable_orderedagg;
+-- start_ignore
+-- FIXME: gpcheckcat fails due to mismatching distribution policy if this 
table isn't dropped
+-- Keep this table around once this is fixed
+drop table ext_part;
+-- end_ignore
diff --git a/src/test/regress/expected/qp_orca_fallback_optimizer.out 
b/src/test/regress/expected/qp_orca_fallback_optimizer.out
index fabbba4cb0..a9d7ff76a1 100644
--- a/src/test/regress/expected/qp_orca_fallback_optimizer.out
+++ b/src/test/regress/expected/qp_orca_fallback_optimizer.out
@@ -332,6 +332,17 @@ explain update ext_part set a=1;
 INFO:  GPORCA failed to produce a plan, falling back to Postgres-based planner
 DETAIL:  Falling back to Postgres-based planner because GPORCA does not 
support the following feature: DML(update) on partitioned tables
 ERROR:  cannot update foreign table "p2_ext"
+set optimizer_enable_orderedagg=off;
+select array_agg(a order by b)
+  from (values (1,4),(2,3),(3,1),(4,2)) v(a,b);
+INFO:  GPORCA failed to produce a plan, falling back to Postgres-based planner
+DETAIL:  Falling back to Postgres-based planner because GPORCA does not 
support the following feature: Ordered aggregates disabled. Enable by setting 
optimizer_enable_orderedagg=on
+ array_agg 
+-----------
+ {3,4,2,1}
+(1 row)
+
+reset optimizer_enable_orderedagg;
 -- start_ignore
 -- FIXME: gpcheckcat fails due to mismatching distribution policy if this 
table isn't dropped
 -- Keep this table around once this is fixed
diff --git a/src/test/regress/sql/qp_orca_fallback.sql 
b/src/test/regress/sql/qp_orca_fallback.sql
index 50157e924b..a1c8547391 100644
--- a/src/test/regress/sql/qp_orca_fallback.sql
+++ b/src/test/regress/sql/qp_orca_fallback.sql
@@ -114,6 +114,11 @@ alter table ext_part attach partition p2_ext for values in 
(2);
 explain insert into ext_part values (1);
 explain delete from ext_part where a=1;
 explain update ext_part set a=1;
+set optimizer_enable_orderedagg=off;
+select array_agg(a order by b)
+  from (values (1,4),(2,3),(3,1),(4,2)) v(a,b);
+reset optimizer_enable_orderedagg;
+
 -- start_ignore
 -- FIXME: gpcheckcat fails due to mismatching distribution policy if this 
table isn't dropped
 -- Keep this table around once this is fixed


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to