Repository: incubator-hawq Updated Branches: refs/heads/master 9c7920cbf -> 08a1c6c2a
HAWQ-934. Populate canSetTag of PlannedStmt from Query object HAWQ generated an error if a single query resulted in multiple query plans because of rule transformation and the plans were produced by PQO. This is because of an incorrect directive in the plan to lock the same resource more than once. This issue has been fixed. Project: http://git-wip-us.apache.org/repos/asf/incubator-hawq/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-hawq/commit/a17647be Tree: http://git-wip-us.apache.org/repos/asf/incubator-hawq/tree/a17647be Diff: http://git-wip-us.apache.org/repos/asf/incubator-hawq/diff/a17647be Branch: refs/heads/master Commit: a17647be029a42c85e659cfc7fe253708a036bd7 Parents: 9c7920c Author: Haisheng Yuan <[email protected]> Authored: Fri Aug 5 15:37:33 2016 -0500 Committer: Ming LI <[email protected]> Committed: Fri Aug 26 15:02:18 2016 +0800 ---------------------------------------------------------------------- .../gpopt/translate/CTranslatorDXLToPlStmt.cpp | 5 +- src/backend/gpopt/utils/COptTasks.cpp | 11 +++-- .../gpopt/translate/CTranslatorDXLToPlStmt.h | 2 +- src/include/gpopt/utils/COptTasks.h | 2 +- src/test/regress/expected/gp_optimizer.out | 52 ++++++++++++++++++++ src/test/regress/sql/gp_optimizer.sql | 40 +++++++++++++++ 6 files changed, 104 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/a17647be/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp ---------------------------------------------------------------------- diff --git a/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp b/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp index fe68cbd..dabfeb9 100644 --- a/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp +++ b/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp @@ -227,7 +227,8 @@ CTranslatorDXLToPlStmt::InitTranslators() PlannedStmt * CTranslatorDXLToPlStmt::PplstmtFromDXL ( - const CDXLNode *pdxln + const CDXLNode *pdxln, + bool canSetTag ) { GPOS_ASSERT(NULL != pdxln); @@ -264,7 +265,7 @@ CTranslatorDXLToPlStmt::PplstmtFromDXL // store partitioned table indexes in planned stmt pplstmt->queryPartOids = m_pctxdxltoplstmt->PlPartitionedTables(); - pplstmt->canSetTag = true; + pplstmt->canSetTag = canSetTag; pplstmt->relationOids = plOids; pplstmt->numSelectorsPerScanId = m_pctxdxltoplstmt->PlNumPartitionSelectors(); http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/a17647be/src/backend/gpopt/utils/COptTasks.cpp ---------------------------------------------------------------------- diff --git a/src/backend/gpopt/utils/COptTasks.cpp b/src/backend/gpopt/utils/COptTasks.cpp index 5be6431..ca20e4c 100644 --- a/src/backend/gpopt/utils/COptTasks.cpp +++ b/src/backend/gpopt/utils/COptTasks.cpp @@ -661,7 +661,8 @@ COptTasks::Pplstmt ( IMemoryPool *pmp, CMDAccessor *pmda, - const CDXLNode *pdxln + const CDXLNode *pdxln, + bool canSetTag ) { @@ -687,7 +688,7 @@ COptTasks::Pplstmt // translate DXL -> PlannedStmt CTranslatorDXLToPlStmt trdxltoplstmt(pmp, pmda, &ctxdxltoplstmt, gpdb::UlSegmentCountGP()); - return trdxltoplstmt.PplstmtFromDXL(pdxln); + return trdxltoplstmt.PplstmtFromDXL(pdxln, canSetTag); } @@ -1043,7 +1044,9 @@ COptTasks::PvOptimizeTask // translate DXL->PlStmt only when needed if (poctx->m_fGeneratePlStmt) { - poctx->m_pplstmt = (PlannedStmt *) gpdb::PvCopyObject(Pplstmt(pmp, &mda, pdxlnPlan)); + // always use poctx->m_pquery->canSetTag as the ptrquerytodxl->Pquery() is a mutated Query object + // that may not have the correct canSetTag + poctx->m_pplstmt = (PlannedStmt *) gpdb::PvCopyObject(Pplstmt(pmp, &mda, pdxlnPlan, poctx->m_pquery->canSetTag)); } CStatisticsConfig *pstatsconf = pocconf->Pstatsconf(); @@ -1292,7 +1295,7 @@ COptTasks::PvPlstmtFromDXLTask // translate DXL -> PlannedStmt CTranslatorDXLToPlStmt trdxltoplstmt(pmp, amda.Pmda(), &ctxdxlplstmt, gpdb::UlSegmentCountGP()); - PlannedStmt *pplstmt = trdxltoplstmt.PplstmtFromDXL(pdxlnOriginal); + PlannedStmt *pplstmt = trdxltoplstmt.PplstmtFromDXL(pdxlnOriginal, poctx->m_pquery->canSetTag); if (optimizer_print_plan) { elog(NOTICE, "Plstmt: %s", gpdb::SzNodeToString(pplstmt)); http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/a17647be/src/include/gpopt/translate/CTranslatorDXLToPlStmt.h ---------------------------------------------------------------------- diff --git a/src/include/gpopt/translate/CTranslatorDXLToPlStmt.h b/src/include/gpopt/translate/CTranslatorDXLToPlStmt.h index acc84bf..a0577dc 100644 --- a/src/include/gpopt/translate/CTranslatorDXLToPlStmt.h +++ b/src/include/gpopt/translate/CTranslatorDXLToPlStmt.h @@ -275,7 +275,7 @@ namespace gpdxl ); // main translation routine for DXL tree -> PlannedStmt - PlannedStmt *PplstmtFromDXL(const CDXLNode *pdxln); + PlannedStmt *PplstmtFromDXL(const CDXLNode *pdxln, bool canSetTag); // translate the join types from its DXL representation to the GPDB one static JoinType JtFromEdxljt(EdxlJoinType edxljt); http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/a17647be/src/include/gpopt/utils/COptTasks.h ---------------------------------------------------------------------- diff --git a/src/include/gpopt/utils/COptTasks.h b/src/include/gpopt/utils/COptTasks.h index 8916814..580ab1d 100644 --- a/src/include/gpopt/utils/COptTasks.h +++ b/src/include/gpopt/utils/COptTasks.h @@ -221,7 +221,7 @@ class COptTasks // translate a DXL tree into a planned statement static - PlannedStmt *Pplstmt(IMemoryPool *pmp, CMDAccessor *pmda, const CDXLNode *pdxln); + PlannedStmt *Pplstmt(IMemoryPool *pmp, CMDAccessor *pmda, const CDXLNode *pdxln, bool canSetTag); // load search strategy from given path static http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/a17647be/src/test/regress/expected/gp_optimizer.out ---------------------------------------------------------------------- diff --git a/src/test/regress/expected/gp_optimizer.out b/src/test/regress/expected/gp_optimizer.out index 23b1e5a..fd3a686 100644 --- a/src/test/regress/expected/gp_optimizer.out +++ b/src/test/regress/expected/gp_optimizer.out @@ -8678,6 +8678,58 @@ SELECT cc, sum(nn) over() FROM v1_mpp_20470; drop view v1_mpp_20470; drop table t_mpp_20470; + +-- Checking if ORCA correctly populates canSetTag in PlannedStmt for multiple statements because of rules +drop table if exists can_set_tag_target; +NOTICE: table "can_set_tag_target" does not exist, skipping +create table can_set_tag_target +( + x int, + y int, + z char +); +NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'x' as the Greenplum Database data distribution key for this table. +HINT: The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew. +drop table if exists can_set_tag_audit; +NOTICE: table "can_set_tag_audit" does not exist, skipping +create table can_set_tag_audit +( + t timestamp without time zone, + x int, + y int, + z char +); +NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 't' as the Greenplum Database data distribution key for this table. +HINT: The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew. +create rule can_set_tag_audit_update AS + ON UPDATE TO can_set_tag_target DO INSERT INTO can_set_tag_audit (t, x, y, z) + VALUES (now(), old.x, old.y, old.z); +insert into can_set_tag_target select i, i + 1, i + 2 from generate_series(1,2) as i; +create role unpriv; +NOTICE: resource queue required -- using default resource queue "pg_default" +grant all on can_set_tag_target to unpriv; +grant all on can_set_tag_audit to unpriv; +set role unpriv; +show optimizer; + optimizer +----------- + on +(1 row) + +update can_set_tag_target set y = y + 1; +select count(1) from can_set_tag_audit; + count +------- + 2 +(1 row) + +reset role; +revoke all on can_set_tag_target from unpriv; +revoke all on can_set_tag_audit from unpriv; +drop role unpriv; +drop table can_set_tag_target; +drop table can_set_tag_audit; + -- clean up drop schema orca cascade; NOTICE: drop cascades to table orca.bm_dyn_test_onepart_1_prt_part5 http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/a17647be/src/test/regress/sql/gp_optimizer.sql ---------------------------------------------------------------------- diff --git a/src/test/regress/sql/gp_optimizer.sql b/src/test/regress/sql/gp_optimizer.sql index 9d02ae5..c1ab7a1 100644 --- a/src/test/regress/sql/gp_optimizer.sql +++ b/src/test/regress/sql/gp_optimizer.sql @@ -776,5 +776,45 @@ SELECT cc, sum(nn) over() FROM v1_mpp_20470; drop view v1_mpp_20470; drop table t_mpp_20470; + +-- Checking if ORCA correctly populates canSetTag in PlannedStmt for multiple statements because of rules +drop table if exists can_set_tag_target; +create table can_set_tag_target +( + x int, + y int, + z char +); + +drop table if exists can_set_tag_audit; +create table can_set_tag_audit +( + t timestamp without time zone, + x int, + y int, + z char +); + +create rule can_set_tag_audit_update AS + ON UPDATE TO can_set_tag_target DO INSERT INTO can_set_tag_audit (t, x, y, z) + VALUES (now(), old.x, old.y, old.z); + +insert into can_set_tag_target select i, i + 1, i + 2 from generate_series(1,2) as i; + +create role unpriv; +grant all on can_set_tag_target to unpriv; +grant all on can_set_tag_audit to unpriv; +set role unpriv; +show optimizer; +update can_set_tag_target set y = y + 1; +select count(1) from can_set_tag_audit; +reset role; + +revoke all on can_set_tag_target from unpriv; +revoke all on can_set_tag_audit from unpriv; +drop role unpriv; +drop table can_set_tag_target; +drop table can_set_tag_audit; + -- clean up drop schema orca cascade;
