You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hawq.apache.org by ml...@apache.org on 2016/10/10 03:33:09 UTC
incubator-hawq git commit: HAWQ-1093. Bump Orca version and enable
Orca related exception propagation
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 <hy...@pivotal.io>
Authored: Thu Sep 8 17:47:24 2016 -0700
Committer: Haisheng Yuan <hy...@pivotal.io>
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'),