Repository: incubator-hawq Updated Branches: refs/heads/master 2c361d95e -> 19f3fa81b
HAWQ-1093. Bump Orca version and enable Orca related exception propagation In COptTasks::PplstmtOptimize, we print exception message from context using elog::ERROR. Because orca / gpos enhanced to rethrow exception, we were not executing this code. In this patch, we catch the exception and print the message as warning. After printing, we throw the exception and abort the plan generation in CGPOptimizer::PplstmtOptimize. We can't use elog::ERROR because it will do sigjmp to PostgresMain and hence result in memory leak. Also since there is no easy way to carry the message in CException, as an initial implementation we decided to print as warning. E.g. query and console message: ``` select * from wet_pos1; WARNING: External scan error: It is not possible to read from a WRITABLE external table. Create the table as READABLE instead. ERROR: Unrecoverable warning. Aborting GPORCA plan generation. (CGPOptimizer.cpp:66) ``` Project: http://git-wip-us.apache.org/repos/asf/incubator-hawq/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-hawq/commit/19f3fa81 Tree: http://git-wip-us.apache.org/repos/asf/incubator-hawq/tree/19f3fa81 Diff: http://git-wip-us.apache.org/repos/asf/incubator-hawq/diff/19f3fa81 Branch: refs/heads/master Commit: 19f3fa81b1e4ca548d3534eeae2e114c66e4b717 Parents: 2c361d9 Author: Haisheng Yuan and Omer Arap <[email protected]> Authored: Thu Sep 8 17:47:24 2016 -0700 Committer: Haisheng Yuan <[email protected]> Committed: Sun Oct 9 21:45:22 2016 -0500 ---------------------------------------------------------------------- depends/thirdparty/gporca.commit | 2 +- depends/thirdparty/gpos.commit | 2 +- src/backend/gpopt/CGPOptimizer.cpp | 20 ++++++- src/backend/gpopt/gpdbwrappers.cpp | 7 ++- .../gpopt/translate/CTranslatorQueryToDXL.cpp | 2 - src/backend/gpopt/utils/COptTasks.cpp | 62 ++++++++++++++------ src/include/gpopt/utils/COptTasks.h | 4 ++ src/test/regress/expected/errors.out | 4 ++ src/test/regress/expected/horology.out | 5 +- src/test/regress/sql/errors.sql | 5 +- src/test/regress/sql/horology.sql | 6 +- 11 files changed, 91 insertions(+), 28 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/19f3fa81/depends/thirdparty/gporca.commit ---------------------------------------------------------------------- diff --git a/depends/thirdparty/gporca.commit b/depends/thirdparty/gporca.commit index cf4eaaa..b7aef6e 100644 --- a/depends/thirdparty/gporca.commit +++ b/depends/thirdparty/gporca.commit @@ -1 +1 @@ -https://github.com/greenplum-db/gporca.git master c5e40f283703b5fa4c2eb40f367ab7c1b1ab4d0d +https://github.com/greenplum-db/gporca.git master cdd832b193a1fb21c8fa43bc5161ea381aa2b621 http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/19f3fa81/depends/thirdparty/gpos.commit ---------------------------------------------------------------------- diff --git a/depends/thirdparty/gpos.commit b/depends/thirdparty/gpos.commit index 5455581..04c74ca 100644 --- a/depends/thirdparty/gpos.commit +++ b/depends/thirdparty/gpos.commit @@ -1 +1 @@ -https://github.com/greenplum-db/gpos.git master 6af760fb96f5bd48783e8644e7d63c39132b8c08 +https://github.com/greenplum-db/gpos.git master 7f1bd6eec40fcaaa032edeac88dd54660f2f0943 http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/19f3fa81/src/backend/gpopt/CGPOptimizer.cpp ---------------------------------------------------------------------- diff --git a/src/backend/gpopt/CGPOptimizer.cpp b/src/backend/gpopt/CGPOptimizer.cpp index db3cab8..bcace95 100644 --- a/src/backend/gpopt/CGPOptimizer.cpp +++ b/src/backend/gpopt/CGPOptimizer.cpp @@ -37,6 +37,8 @@ #include "gpopt/init.h" #include "gpos/_api.h" +#include "naucrates/exception.h" + //--------------------------------------------------------------------------- // @function: // CGPOptimizer::TouchLibraryInitializers @@ -69,7 +71,23 @@ CGPOptimizer::PplstmtOptimize bool *pfUnexpectedFailure // output : set to true if optimizer unexpectedly failed to produce plan ) { - return COptTasks::PplstmtOptimize(pquery, pfUnexpectedFailure); + GPOS_TRY + { + return COptTasks::PplstmtOptimize(pquery, pfUnexpectedFailure); + } + GPOS_CATCH_EX(ex) + { + if (GPOS_MATCH_EX(ex, gpdxl::ExmaDXL, gpdxl::ExmiWarningAsError)) + { + elog(ERROR, "PQO unable to generate plan, please see the above message for details."); + } + if (GPOS_MATCH_EX(ex, gpdxl::ExmaGPDB, gpdxl::ExmiGPDBError)) + { + elog(ERROR, "GPDB exception. Aborting PQO plan generation."); + } + } + GPOS_CATCH_END; + return NULL; } http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/19f3fa81/src/backend/gpopt/gpdbwrappers.cpp ---------------------------------------------------------------------- diff --git a/src/backend/gpopt/gpdbwrappers.cpp b/src/backend/gpopt/gpdbwrappers.cpp index 7d72823..1d6a071 100644 --- a/src/backend/gpopt/gpdbwrappers.cpp +++ b/src/backend/gpopt/gpdbwrappers.cpp @@ -237,8 +237,13 @@ #define GP_WRAP_END \ } \ + else \ + { \ + EmitErrorReport(); \ + FlushErrorState(); \ + GPOS_RAISE(gpdxl::ExmaGPDB, gpdxl::ExmiGPDBError); \ + } \ } \ - GPOS_RAISE(gpdxl::ExmaGPDB, gpdxl::ExmiGPDBError) using namespace gpos; http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/19f3fa81/src/backend/gpopt/translate/CTranslatorQueryToDXL.cpp ---------------------------------------------------------------------- diff --git a/src/backend/gpopt/translate/CTranslatorQueryToDXL.cpp b/src/backend/gpopt/translate/CTranslatorQueryToDXL.cpp index e03a542..2985378 100644 --- a/src/backend/gpopt/translate/CTranslatorQueryToDXL.cpp +++ b/src/backend/gpopt/translate/CTranslatorQueryToDXL.cpp @@ -447,8 +447,6 @@ CTranslatorQueryToDXL::CheckSupportedCmdType GPOS_RAISE(gpdxl::ExmaDXL, gpdxl::ExmiQuery2DXLUnsupportedFeature, mapelem.m_wsz); } } - - GPOS_ASSERT(!"Unrecognized command type"); } //--------------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/19f3fa81/src/backend/gpopt/utils/COptTasks.cpp ---------------------------------------------------------------------- diff --git a/src/backend/gpopt/utils/COptTasks.cpp b/src/backend/gpopt/utils/COptTasks.cpp index ca20e4c..aa00410 100644 --- a/src/backend/gpopt/utils/COptTasks.cpp +++ b/src/backend/gpopt/utils/COptTasks.cpp @@ -50,6 +50,7 @@ #include "utils/guc.h" #include "gpos/base.h" +#include "gpos/error/CException.h" #undef setstate #include "gpos/_api.h" @@ -178,6 +179,37 @@ COptTasks::SOptContext::SOptContext() m_szErrorMsg(NULL) {} +//--------------------------------------------------------------------------- +// @function: +// COptTasks::SOptContext::HandleError +// +// @doc: +// If there is an error print as warning and throw GPOS_EXCEPTION to abort +// plan generation. Calling elog::ERROR will result in longjump and hence +// a memory leak. +//--------------------------------------------------------------------------- +void +COptTasks::SOptContext::HandleError + ( + BOOL *pfUnexpectedFailure + ) +{ + BOOL bhasError = false; + if (NULL != m_szErrorMsg) + { + bhasError = true; + elog(WARNING, "%s", m_szErrorMsg); + } + *pfUnexpectedFailure = m_fUnexpectedFailure; + + // clean up context + Free(epinQuery, epinPlStmt); + if (bhasError) + { + GPOS_RAISE(gpdxl::ExmaDXL, gpdxl::ExmiWarningAsError); + } +} + //--------------------------------------------------------------------------- // @function: @@ -726,9 +758,11 @@ COptTasks::PdrgPssLoad } GPOS_CATCH_EX(ex) { - GPOS_RESET_EX; - + if (GPOS_MATCH_EX(ex, gpdxl::ExmaGPDB, gpdxl::ExmiGPDBError)) { + GPOS_RETHROW(ex); + } elog(DEBUG2, "\n[OPT]: Using default search strategy"); + GPOS_RESET_EX; } GPOS_CATCH_END; @@ -1528,13 +1562,6 @@ COptTasks::PvEvalExprFromDXLTask { CMDCache::Shutdown(); } - // Catch GPDB exceptions - if (GPOS_MATCH_EX(ex, gpdxl::ExmaGPDB, gpdxl::ExmiGPDBError)) - { - elog(NOTICE, "Found non const expression. Please check log for more information."); - GPOS_RESET_EX; - return NULL; - } if (FErrorOut(ex)) { IErrorContext *perrctxt = CTask::PtskSelf()->Perrctxt(); @@ -1616,17 +1643,18 @@ COptTasks::PplstmtOptimize SOptContext octx; octx.m_pquery = pquery; octx.m_fGeneratePlStmt= true; - Execute(&PvOptimizeTask, &octx); - - if (NULL != octx.m_szErrorMsg) + GPOS_TRY { - elog(ERROR, octx.m_szErrorMsg); + Execute(&PvOptimizeTask, &octx); } - *pfUnexpectedFailure = octx.m_fUnexpectedFailure; - - // clean up context - octx.Free(octx.epinQuery, octx.epinPlStmt); + GPOS_CATCH_EX(ex) + { + octx.HandleError(pfUnexpectedFailure); + GPOS_RETHROW(ex); + } + GPOS_CATCH_END; + octx.HandleError(pfUnexpectedFailure); return octx.m_pplstmt; } http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/19f3fa81/src/include/gpopt/utils/COptTasks.h ---------------------------------------------------------------------- diff --git a/src/include/gpopt/utils/COptTasks.h b/src/include/gpopt/utils/COptTasks.h index 580ab1d..fe24475 100644 --- a/src/include/gpopt/utils/COptTasks.h +++ b/src/include/gpopt/utils/COptTasks.h @@ -114,6 +114,10 @@ class COptTasks // ctor SOptContext(); + // If there is an error print as warning and throw exception to abort + // plan generation + void HandleError(BOOL *pfUnexpectedFailure); + // free all members except input and output pointers void Free(EPin epinInput, EPin epinOutput); http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/19f3fa81/src/test/regress/expected/errors.out ---------------------------------------------------------------------- diff --git a/src/test/regress/expected/errors.out b/src/test/regress/expected/errors.out index 2e53cdc..6f2f5e9 100755 --- a/src/test/regress/expected/errors.out +++ b/src/test/regress/expected/errors.out @@ -483,9 +483,13 @@ create function infinite_recurse() returns int as -- # mpp-2756 -- m/(ERROR|WARNING|CONTEXT|NOTICE):.*stack depth limit exceeded\s+at\s+character/ -- s/\s+at\s+character.*// +-- m/ERROR:.*GPDB exception. Aborting PQO.*/ +-- s/ERROR:.*GPDB exception. Aborting PQO.*// -- end_matchsubs +-- start_ignore select infinite_recurse(); ERROR: stack depth limit exceeded +-- end_ignore select 1; -- test that this works ?column? ---------- http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/19f3fa81/src/test/regress/expected/horology.out ---------------------------------------------------------------------- diff --git a/src/test/regress/expected/horology.out b/src/test/regress/expected/horology.out index 4b79ed8..9786e5a 100755 --- a/src/test/regress/expected/horology.out +++ b/src/test/regress/expected/horology.out @@ -10,8 +10,9 @@ INSERT INTO ABSTIME_HOROLOGY_TBL (f1) VALUES ('Jan 14, 1973 03:14:21'), (abstime 'epoch'), (abstime 'infinity'), (abstime '-infinity'), -(abstime 'May 10, 1947 23:59:12'), -('Jun 10, 1843'); +(abstime 'May 10, 1947 23:59:12'); +-- orca will fail for this +INSERT INTO ABSTIME_HOROLOGY_TBL (f1) VALUES('Jun 10, 1843'); CREATE TABLE INTERVAL_HOROLOGY_TBL (f1 interval); NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'f1' 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. http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/19f3fa81/src/test/regress/sql/errors.sql ---------------------------------------------------------------------- diff --git a/src/test/regress/sql/errors.sql b/src/test/regress/sql/errors.sql index 5138407..9416057 100644 --- a/src/test/regress/sql/errors.sql +++ b/src/test/regress/sql/errors.sql @@ -400,7 +400,10 @@ create function infinite_recurse() returns int as -- # mpp-2756 -- m/(ERROR|WARNING|CONTEXT|NOTICE):.*stack depth limit exceeded\s+at\s+character/ -- s/\s+at\s+character.*// +-- m/ERROR:.*GPDB exception. Aborting PQO.*/ +-- s/ERROR:.*GPDB exception. Aborting PQO.*// -- end_matchsubs +-- start_ignore select infinite_recurse(); - +-- end_ignore select 1; -- test that this works http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/19f3fa81/src/test/regress/sql/horology.sql ---------------------------------------------------------------------- diff --git a/src/test/regress/sql/horology.sql b/src/test/regress/sql/horology.sql index 65eee0a..8f54e71 100644 --- a/src/test/regress/sql/horology.sql +++ b/src/test/regress/sql/horology.sql @@ -8,8 +8,10 @@ INSERT INTO ABSTIME_HOROLOGY_TBL (f1) VALUES ('Jan 14, 1973 03:14:21'), (abstime 'epoch'), (abstime 'infinity'), (abstime '-infinity'), -(abstime 'May 10, 1947 23:59:12'), -('Jun 10, 1843'); +(abstime 'May 10, 1947 23:59:12'); + +-- orca will fail for this +INSERT INTO ABSTIME_HOROLOGY_TBL (f1) VALUES('Jun 10, 1843'); CREATE TABLE INTERVAL_HOROLOGY_TBL (f1 interval); INSERT INTO INTERVAL_HOROLOGY_TBL (f1) VALUES ('@ 1 minute'),
