Repository: trafodion Updated Branches: refs/heads/master 44db81913 -> e6e3e3c1d
[TRAFODION-3158] Make EncodedValue error handling more robust Project: http://git-wip-us.apache.org/repos/asf/trafodion/repo Commit: http://git-wip-us.apache.org/repos/asf/trafodion/commit/64e5ed11 Tree: http://git-wip-us.apache.org/repos/asf/trafodion/tree/64e5ed11 Diff: http://git-wip-us.apache.org/repos/asf/trafodion/diff/64e5ed11 Branch: refs/heads/master Commit: 64e5ed11a2d1deae8d748d7d749abb5007ae0c41 Parents: b09048a Author: Dave Birdsall <dbirds...@apache.org> Authored: Mon Jul 23 17:24:57 2018 +0000 Committer: Dave Birdsall <dbirds...@apache.org> Committed: Mon Jul 23 17:24:57 2018 +0000 ---------------------------------------------------------------------- core/sql/optimizer/EncodedValue.cpp | 85 +++++++++++++++++++++++++++++--- core/sql/optimizer/EncodedValue.h | 11 +++-- 2 files changed, 85 insertions(+), 11 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/trafodion/blob/64e5ed11/core/sql/optimizer/EncodedValue.cpp ---------------------------------------------------------------------- diff --git a/core/sql/optimizer/EncodedValue.cpp b/core/sql/optimizer/EncodedValue.cpp index f05297a..f8c6b7b 100644 --- a/core/sql/optimizer/EncodedValue.cpp +++ b/core/sql/optimizer/EncodedValue.cpp @@ -648,8 +648,62 @@ EncodedValue::EncodedValue (ItemExpr *expr, void EncodedValue::constructorFunction (const NAWchar * theValue, const NAColumnArray &columns, + NABoolean okToReportErrors, ConstValue* cvPtrs[]) { + // Some notes about error reporting for this function: + // + // The error reporting for this function is a little strange + // and should be re-engineered when we can imagine a better + // design for it. + // + // This function is called from two very different contexts. + // + // One is from the constructors of global objects, to provide + // convenient encoded constants. Being global objects, this + // call is made as a result of global constructor calls before + // the C++ main for the process is invoked. As such, we cannot + // depend on other global objects being constructed. So, for + // example, we cannot depend on CmpCommon::diags() being + // initialized, as C++ makes no guarantees about the order in + // which global objects are created. Too, it does no good to + // throw a C++ exception as there would be nothing to catch it + // and process it. So, if an error happens in this code path, + // we'll simply assert. + // + // The other is in the course of histogram processing. Histograms + // have been read in, and now we want to encode the boundary + // values in the histogram. These might be stale or corrupted + // so that condition has to be detected. When detected, this + // routine raises a warning in CmpCommon::diags(), and then + // throws a C++ exception. + // + // This warning processing has to be done carefully. The way + // it works is that lower level routines report errors into + // CmpCommon::diags(). This routine checks for such errors and + // if it sees any, it throws them away, replacing them with + // a warning in CmpCommon::diags(). It then throws a C++ + // exception which is typically caught by HSHistogrmCursor::fetch + // (ustat/hs_read.cpp). We use default histograms in that case. + // + // Why do we throw away the errors? We do this because of the + // way the Normalizer handles CmpCommon::diags(). During + // synthesise logical properties processing, histograms may + // be read and processed. (Note: They can be read and processed + // from other phases as well, such as table analysis.) The + // Normalizer checks for errors in CmpCommon::diags(), and if + // found, retries compilation. On the retry, CmpCommon::diags() + // will be cleared, and the histograms code will simply use + // a default histogram. So, if there is any error in + // CmpCommon::diags(), we will lose the histogram warnings + // generated in this method. + // + // Note that if there is already an error in CmpCommon::diags() + // when this method is called, we'll lose the histogram warnings + // anyway. Sigh. + + Lng32 mark = okToReportErrors ? CmpCommon::diags()->mark() : -1; + // Find the first non-blank char. const NAWchar *item = theValue; while (*item == L' ') @@ -657,10 +711,17 @@ EncodedValue::constructorFunction (const NAWchar * theValue, if ( *item != L'(' ) // must be '(' { - *CmpCommon::diags() << DgSqlCode(CATALOG_HISTOGRM_HISTINTS_TABLES_CONTAIN_BAD_VALUE) + if (okToReportErrors) + { + *CmpCommon::diags() << DgSqlCode(CATALOG_HISTOGRM_HISTINTS_TABLES_CONTAIN_BAD_VALUE) << DgWString0(theValue) << DgString1(columns[0]->getFullColRefNameAsAnsiString()); - CmpInternalException("Bad Interval Boundary", __FILE__ , __LINE__).throwException(); + CmpInternalException("Bad Interval Boundary", __FILE__ , __LINE__).throwException(); + } + else + { + CMPASSERT(FALSE); // developer needs to fix the bug + } } item++; @@ -766,11 +827,19 @@ EncodedValue::constructorFunction (const NAWchar * theValue, if ( next == NULL ) // should never happen! { - *CmpCommon::diags() + if (okToReportErrors) + { + CmpCommon::diags()->rewind(mark,TRUE); // get rid of any diags we may have added + *CmpCommon::diags() << DgSqlCode(CATALOG_HISTOGRM_HISTINTS_TABLES_CONTAIN_BAD_VALUE) << DgWString0(theValue) << DgString1(columns[i]->getFullColRefNameAsAnsiString()); - CmpInternalException("Bad Interval Boundary", __FILE__ , __LINE__).throwException(); + CmpInternalException("Bad Interval Boundary", __FILE__ , __LINE__).throwException(); + } + else + { + CMPASSERT(FALSE); // developer needs to fix the bug + } } Lng32 len = BOUNDARY_LEN; @@ -803,6 +872,10 @@ EncodedValue::constructorFunction (const NAWchar * theValue, break; } + // Parser assumes CmpCommon::diags() is initialized and available + // so okToReportErrors better be true in this code path. + CMPASSERT(okToReportErrors); + // invoke parser to parse the char string and generate a ConstValue Parser parser(CmpCommon::context()); @@ -863,9 +936,7 @@ EncodedValue::constructorFunction (const NAWchar * theValue, (entries == 1 && constVal->getType()->getTypeQualifier() != colType->getTypeQualifier())) { - if (CmpCommon::diags()->getNumber(DgSqlCode::ERROR_)) - CmpCommon::diags()->deleteError(0); - + CmpCommon::diags()->rewind(mark,TRUE); // get rid of any diags parser may have added *CmpCommon::diags() << DgSqlCode(CATALOG_HISTOGRM_HISTINTS_TABLES_CONTAIN_BAD_VALUE) << DgWString0(theValue) http://git-wip-us.apache.org/repos/asf/trafodion/blob/64e5ed11/core/sql/optimizer/EncodedValue.h ---------------------------------------------------------------------- diff --git a/core/sql/optimizer/EncodedValue.h b/core/sql/optimizer/EncodedValue.h index 961adeb..4bc5e84 100644 --- a/core/sql/optimizer/EncodedValue.h +++ b/core/sql/optimizer/EncodedValue.h @@ -165,24 +165,27 @@ public: // construct a multi-attribute value given a string representation // of the multi-attribute value, and a description of the columns + + // Note: This constructor is used to construct global objects, so + // it cannot report errors. EncodedValue (const NAWchar *theValue) : valueList_(NULL), heap_(HISTHEAP) { const NAColumnArray empty ((CollHeap*) 0 /* NULL CollHeap* */) ; - constructorFunction (theValue, empty) ; + constructorFunction (theValue, empty, FALSE) ; } EncodedValue (const wchar_t *theValue, const NAColumnArray &columns, ConstValue* cvPtrs[] = NULL) : valueList_(NULL), heap_(HISTHEAP) - { constructorFunction (theValue, columns, cvPtrs) ; } + { constructorFunction (theValue, columns, TRUE, cvPtrs) ; } EncodedValue (const NAWchar *theValue, const NAColumn * column) : valueList_(NULL), heap_(HISTHEAP) { NAColumnArray columns ; columns.insertAt (0,(NAColumn*)(column)) ; - constructorFunction (theValue, columns) ; + constructorFunction (theValue, columns, TRUE) ; } EncodedValue (const EncodedValue & other, NAMemory * h = 0); @@ -192,7 +195,7 @@ private: // thing, I've created a non-ctor function that both of them // call. Sooner or later we can stop using the ctor that takes // a NAColumnArray as its parameter ... - void constructorFunction (const wchar_t * theValue, const NAColumnArray & columns, ConstValue** cvPtr = NULL) ; + void constructorFunction (const wchar_t * theValue, const NAColumnArray & columns, NABoolean okToReportErrors, ConstValue** cvPtr = NULL) ; public: // construct a single-attribute value from a constant or list of constants
