[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[ https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16444975#comment-16444975 ] ASF GitHub Bot commented on TRAFODION-3009: --- Github user asfgit closed the pull request at: https://github.com/apache/trafodion/pull/1528 > Streamline error handling in Executor utility commands > -- > > Key: TRAFODION-3009 > URL: https://issues.apache.org/jira/browse/TRAFODION-3009 > Project: Apache Trafodion > Issue Type: Improvement > Components: -exe, sql-exe >Reporter: Selvaganesan Govindarajan >Assignee: Selvaganesan Govindarajan >Priority: Major > > Executor utility commands are the commands that issue sql statements during > its execution to complete its task. These operators are derived from > ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is > allocated in advance. This is not in accordance with the design as outlined in > [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design] > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[ https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16444619#comment-16444619 ] ASF GitHub Bot commented on TRAFODION-3009: --- GitHub user selvaganesang opened a pull request: https://github.com/apache/trafodion/pull/1528 [TRAFODION-3009] Streamline error handling in Executor utility commands ComDiagsArea is now allocated only when there are warnings or errors in all the utility commands. This requires all the executor utility commands to use a new version of ExRaiseSqlError to populate diagnostics area. You can merge this pull request into a Git repository by running: $ git pull https://github.com/selvaganesang/trafodion trafodion-3009_6 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafodion/pull/1528.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1528 commit 7eaf88d3d74b78af20d845d7f717068e3a5db66c Author: selvaganesang Date: 2018-04-19T18:47:04Z [TRAFODION-3009] Streamline error handling in Executor utility commands ComDiagsArea is now allocated only when there are warnings or errors in all the utility commands. This requires all the executor utility commands to use a new version of ExRaiseSqlError to populate diagnostics area. > Streamline error handling in Executor utility commands > -- > > Key: TRAFODION-3009 > URL: https://issues.apache.org/jira/browse/TRAFODION-3009 > Project: Apache Trafodion > Issue Type: Improvement > Components: -exe, sql-exe >Reporter: Selvaganesan Govindarajan >Assignee: Selvaganesan Govindarajan >Priority: Major > > Executor utility commands are the commands that issue sql statements during > its execution to complete its task. These operators are derived from > ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is > allocated in advance. This is not in accordance with the design as outlined in > [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design] > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[ https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16443438#comment-16443438 ] ASF GitHub Bot commented on TRAFODION-3009: --- Github user asfgit closed the pull request at: https://github.com/apache/trafodion/pull/1522 > Streamline error handling in Executor utility commands > -- > > Key: TRAFODION-3009 > URL: https://issues.apache.org/jira/browse/TRAFODION-3009 > Project: Apache Trafodion > Issue Type: Improvement > Components: -exe, sql-exe >Reporter: Selvaganesan Govindarajan >Assignee: Selvaganesan Govindarajan >Priority: Major > > Executor utility commands are the commands that issue sql statements during > its execution to complete its task. These operators are derived from > ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is > allocated in advance. This is not in accordance with the design as outlined in > [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design] > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[ https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16443427#comment-16443427 ] ASF GitHub Bot commented on TRAFODION-3009: --- Github user selvaganesang commented on a diff in the pull request: https://github.com/apache/trafodion/pull/1522#discussion_r182612555 --- Diff: core/sql/executor/ExHdfsScan.cpp --- @@ -626,14 +625,23 @@ ExWorkProcRetcode ExHdfsScanTcb::work() else extraBytesRead_ = 0; // headRoom_ is the number of extra bytes to be read (rangeTailIOSize) - // If EOF is reached while reading the range and the extraBytes read - // is less than headRoom_ then process all the data till EOF - // TODO: If the whole range fits in one buffer, it is need too to process rows till EOF for the last range alone - // No easy way to identify that last range read, but can identify that it is not the first range. - // The rows could be read more than once if there are more than 2 ranges. - // Fix optimizer not to have more than 2 ranges in that case - if (retArray_[IS_EOF] && extraBytesRead_ < headRoom_ && hdfo->getStartOffset() != 0) -extraBytesRead_ = 0; + // If the whole range fits in one buffer, it is needed to process rows till EOF for the last range alone. +/* --- End diff -- Yes. I can remove it in a later check-in > Streamline error handling in Executor utility commands > -- > > Key: TRAFODION-3009 > URL: https://issues.apache.org/jira/browse/TRAFODION-3009 > Project: Apache Trafodion > Issue Type: Improvement > Components: -exe, sql-exe >Reporter: Selvaganesan Govindarajan >Assignee: Selvaganesan Govindarajan >Priority: Major > > Executor utility commands are the commands that issue sql statements during > its execution to complete its task. These operators are derived from > ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is > allocated in advance. This is not in accordance with the design as outlined in > [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design] > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[ https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16443375#comment-16443375 ] ASF GitHub Bot commented on TRAFODION-3009: --- Github user DaveBirdsall commented on a diff in the pull request: https://github.com/apache/trafodion/pull/1522#discussion_r182604952 --- Diff: core/sql/executor/ExHdfsScan.cpp --- @@ -626,14 +625,23 @@ ExWorkProcRetcode ExHdfsScanTcb::work() else extraBytesRead_ = 0; // headRoom_ is the number of extra bytes to be read (rangeTailIOSize) - // If EOF is reached while reading the range and the extraBytes read - // is less than headRoom_ then process all the data till EOF - // TODO: If the whole range fits in one buffer, it is need too to process rows till EOF for the last range alone - // No easy way to identify that last range read, but can identify that it is not the first range. - // The rows could be read more than once if there are more than 2 ranges. - // Fix optimizer not to have more than 2 ranges in that case - if (retArray_[IS_EOF] && extraBytesRead_ < headRoom_ && hdfo->getStartOffset() != 0) -extraBytesRead_ = 0; + // If the whole range fits in one buffer, it is needed to process rows till EOF for the last range alone. +/* --- End diff -- Is the commented-out code needed anymore? Should we get rid of it? > Streamline error handling in Executor utility commands > -- > > Key: TRAFODION-3009 > URL: https://issues.apache.org/jira/browse/TRAFODION-3009 > Project: Apache Trafodion > Issue Type: Improvement > Components: -exe, sql-exe >Reporter: Selvaganesan Govindarajan >Assignee: Selvaganesan Govindarajan >Priority: Major > > Executor utility commands are the commands that issue sql statements during > its execution to complete its task. These operators are derived from > ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is > allocated in advance. This is not in accordance with the design as outlined in > [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design] > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[
https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16441232#comment-16441232
]
ASF GitHub Bot commented on TRAFODION-3009:
---
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1522#discussion_r182166447
--- Diff: core/sql/executor/ExExeUtilCli.cpp ---
@@ -1119,7 +1119,7 @@ Lng32 ExeCliInterface::executeImmediate(const char *
stmtStr,
{
// Allocate the diagnostics area if needed
// and populate the diagnostics conditions
- if (*globalDiags == NULL && retcode != 0) {
+ if (*globalDiags == NULL && retcode != 0 && retcode != 100) {
--- End diff --
Returning retcode with a value of 100 is ok and should be treated as end of
result by the callers. This additional check just avoids creating a
ComDiagsArea if the return code is just 100.
> Streamline error handling in Executor utility commands
> --
>
> Key: TRAFODION-3009
> URL: https://issues.apache.org/jira/browse/TRAFODION-3009
> Project: Apache Trafodion
> Issue Type: Improvement
> Components: -exe, sql-exe
>Reporter: Selvaganesan Govindarajan
>Assignee: Selvaganesan Govindarajan
>Priority: Major
>
> Executor utility commands are the commands that issue sql statements during
> its execution to complete its task. These operators are derived from
> ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is
> allocated in advance. This is not in accordance with the design as outlined in
> [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design]
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[
https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16441087#comment-16441087
]
ASF GitHub Bot commented on TRAFODION-3009:
---
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1522#discussion_r182136872
--- Diff: core/sql/executor/ExExeUtilCli.cpp ---
@@ -1119,7 +1119,7 @@ Lng32 ExeCliInterface::executeImmediate(const char *
stmtStr,
{
// Allocate the diagnostics area if needed
// and populate the diagnostics conditions
- if (*globalDiags == NULL && retcode != 0) {
+ if (*globalDiags == NULL && retcode != 0 && retcode != 100) {
--- End diff --
I take it in the old code we would throw away the 100 warning somewhere
else?
> Streamline error handling in Executor utility commands
> --
>
> Key: TRAFODION-3009
> URL: https://issues.apache.org/jira/browse/TRAFODION-3009
> Project: Apache Trafodion
> Issue Type: Improvement
> Components: -exe, sql-exe
>Reporter: Selvaganesan Govindarajan
>Assignee: Selvaganesan Govindarajan
>Priority: Major
>
> Executor utility commands are the commands that issue sql statements during
> its execution to complete its task. These operators are derived from
> ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is
> allocated in advance. This is not in accordance with the design as outlined in
> [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design]
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[ https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16440406#comment-16440406 ] ASF GitHub Bot commented on TRAFODION-3009: --- GitHub user selvaganesang opened a pull request: https://github.com/apache/trafodion/pull/1522 [TRAFODION-3009] Streamline error handling in Executor utility commands Changes to avoid allocation of ComDiagsArea in some more places unless it is needed to pass errors or warnings. [TRAFODION-2917] Refactor Trafodion implementation of hdfs scan for text formatted hive tables Disabling USE_LIBHDFS_SCAN by default. The new implementation of Hdfs Scan is now used to scan the text type hive files. Hdfs Scan is now stopped gracefully when the hive scan is cancelled. This avoids the random core seen with new implementation. You can merge this pull request into a Git repository by running: $ git pull https://github.com/selvaganesang/trafodion trafodion-3007_5 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafodion/pull/1522.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1522 commit 060bfc6c41277907af523d7bb62e02eb91b5bc86 Author: selvaganesang Date: 2018-04-17T05:09:04Z [TRAFODION-3009] Streamline error handling in Executor utility commands Changes to avoid allocation of ComDiagsArea in some more places unless it is needed to pass errors or warnings. [TRAFODION-2917] Refactor Trafodion implementation of hdfs scan for text formatted hive tables Disabling USE_LIBHDFS_SCAN by default. The new implementation of Hdfs Scan is now used to scan the text type hive files. Hdfs Scan is now stopped gracefully when the hive scan is cancelled. This avoids the random core seen with new implementation. > Streamline error handling in Executor utility commands > -- > > Key: TRAFODION-3009 > URL: https://issues.apache.org/jira/browse/TRAFODION-3009 > Project: Apache Trafodion > Issue Type: Improvement > Components: -exe, sql-exe >Reporter: Selvaganesan Govindarajan >Assignee: Selvaganesan Govindarajan >Priority: Major > > Executor utility commands are the commands that issue sql statements during > its execution to complete its task. These operators are derived from > ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is > allocated in advance. This is not in accordance with the design as outlined in > [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design] > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[ https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16433407#comment-16433407 ] ASF GitHub Bot commented on TRAFODION-3009: --- Github user asfgit closed the pull request at: https://github.com/apache/trafodion/pull/1513 > Streamline error handling in Executor utility commands > -- > > Key: TRAFODION-3009 > URL: https://issues.apache.org/jira/browse/TRAFODION-3009 > Project: Apache Trafodion > Issue Type: Improvement > Components: -exe, sql-exe >Reporter: Selvaganesan Govindarajan >Assignee: Selvaganesan Govindarajan >Priority: Major > > Executor utility commands are the commands that issue sql statements during > its execution to complete its task. These operators are derived from > ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is > allocated in advance. This is not in accordance with the design as outlined in > [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design] > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[ https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16433030#comment-16433030 ] ASF GitHub Bot commented on TRAFODION-3009: --- GitHub user selvaganesang opened a pull request: https://github.com/apache/trafodion/pull/1513 [TRAFODION-3009] Streamline error handling in Executor utility commands seabase/TEST002 failing with a core file. Yet another attempt to fix this issue. You can merge this pull request into a Git repository by running: $ git pull https://github.com/selvaganesang/trafodion trafodion-3009_3 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafodion/pull/1513.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1513 commit fc03afc85054bee88944f89ee2076b4c6dd64ba7 Author: selvaganesang Date: 2018-04-10T21:29:47Z [TRAFODION-3009] Streamline error handling in Executor utility commands seabase/TEST002 failing with a core file. Yet another attempt to fix this issue. > Streamline error handling in Executor utility commands > -- > > Key: TRAFODION-3009 > URL: https://issues.apache.org/jira/browse/TRAFODION-3009 > Project: Apache Trafodion > Issue Type: Improvement > Components: -exe, sql-exe >Reporter: Selvaganesan Govindarajan >Assignee: Selvaganesan Govindarajan >Priority: Major > > Executor utility commands are the commands that issue sql statements during > its execution to complete its task. These operators are derived from > ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is > allocated in advance. This is not in accordance with the design as outlined in > [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design] > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[ https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16431679#comment-16431679 ] ASF GitHub Bot commented on TRAFODION-3009: --- Github user asfgit closed the pull request at: https://github.com/apache/trafodion/pull/1511 > Streamline error handling in Executor utility commands > -- > > Key: TRAFODION-3009 > URL: https://issues.apache.org/jira/browse/TRAFODION-3009 > Project: Apache Trafodion > Issue Type: Improvement > Components: -exe, sql-exe >Reporter: Selvaganesan Govindarajan >Assignee: Selvaganesan Govindarajan >Priority: Major > > Executor utility commands are the commands that issue sql statements during > its execution to complete its task. These operators are derived from > ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is > allocated in advance. This is not in accordance with the design as outlined in > [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design] > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[ https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16431335#comment-16431335 ] ASF GitHub Bot commented on TRAFODION-3009: --- GitHub user selvaganesang opened a pull request: https://github.com/apache/trafodion/pull/1511 [TRAFODION-3009] Streamline error handling in Executor utility commands get region stats command were populating the errors in the queue entry directly, but still used handleErrors() to populate the errors again in the HANDLE_ERROR_ state. In some cases, the diagnostics area wasn't populated too. In CDH5.4, when the table is purged, the region info wasn't returned You can merge this pull request into a Git repository by running: $ git pull https://github.com/selvaganesang/trafodion trafodion-3009_2 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafodion/pull/1511.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1511 commit 4297cac937309138023d9dcc2612d6fcdc27c366 Author: selvaganesang Date: 2018-04-09T21:07:22Z [TRAFODION-3009] Streamline error handling in Executor utility commands get region stats command were populating the errors in the queue entry directly, but still used handleErrors() to populate the errors again in the HANDLE_ERROR_ state. In some cases, the diagnostics area wasn't populated too. In CDH5.4, when the table is purged, the region info wasn't returned > Streamline error handling in Executor utility commands > -- > > Key: TRAFODION-3009 > URL: https://issues.apache.org/jira/browse/TRAFODION-3009 > Project: Apache Trafodion > Issue Type: Improvement > Components: -exe, sql-exe >Reporter: Selvaganesan Govindarajan >Assignee: Selvaganesan Govindarajan >Priority: Major > > Executor utility commands are the commands that issue sql statements during > its execution to complete its task. These operators are derived from > ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is > allocated in advance. This is not in accordance with the design as outlined in > [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design] > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[ https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16428512#comment-16428512 ] ASF GitHub Bot commented on TRAFODION-3009: --- Github user asfgit closed the pull request at: https://github.com/apache/trafodion/pull/1504 > Streamline error handling in Executor utility commands > -- > > Key: TRAFODION-3009 > URL: https://issues.apache.org/jira/browse/TRAFODION-3009 > Project: Apache Trafodion > Issue Type: Improvement > Components: -exe, sql-exe >Reporter: Selvaganesan Govindarajan >Assignee: Selvaganesan Govindarajan >Priority: Major > > Executor utility commands are the commands that issue sql statements during > its execution to complete its task. These operators are derived from > ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is > allocated in advance. This is not in accordance with the design as outlined in > [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design] > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[
https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16427352#comment-16427352
]
ASF GitHub Bot commented on TRAFODION-3009:
---
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1504#discussion_r179548853
--- Diff: core/sql/sqlcomp/CmpSeabaseDDLschema.cpp ---
@@ -410,7 +410,8 @@ Int64 schemaUID = getObjectTypeandOwner(&cliInterface,
if (schemaUID < 0)
{
*CmpCommon::diags() << DgSqlCode(-CAT_SCHEMA_DOES_NOT_EXIST_ERROR)
- << DgSchemaName(catalogName + "." + schemaName);
+ << DgString0(catalogName)
--- End diff --
See my earlier comments
> Streamline error handling in Executor utility commands
> --
>
> Key: TRAFODION-3009
> URL: https://issues.apache.org/jira/browse/TRAFODION-3009
> Project: Apache Trafodion
> Issue Type: Improvement
> Components: -exe, sql-exe
>Reporter: Selvaganesan Govindarajan
>Assignee: Selvaganesan Govindarajan
>Priority: Major
>
> Executor utility commands are the commands that issue sql statements during
> its execution to complete its task. These operators are derived from
> ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is
> allocated in advance. This is not in accordance with the design as outlined in
> [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design]
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[ https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16427350#comment-16427350 ] ASF GitHub Bot commented on TRAFODION-3009: --- Github user selvaganesang commented on a diff in the pull request: https://github.com/apache/trafodion/pull/1504#discussion_r179548743 --- Diff: core/sql/bin/SqlciErrors.txt --- @@ -2,7 +2,7 @@ 1000 42000 9 BEGINNER INFRM LOGONLY A syntax error occurred. 1001 Z 9 ADVANCED CRTCL DIALOUT An internal error occurred in module $0~String0 on line $1~Int0. Details($2~String1). 1002 Z 9 BEGINNER MAJOR DBADMIN Catalog $0~CatalogName does not exist. -1003 Z 9 BEGINNER MINOR DBADMIN Schema $0~SchemaName does not exist. +1003 Z 9 BEGINNER MINOR DBADMIN Schema $0~String0.$1~String1 does not exist. --- End diff -- There are two ways to raise an error. 1) Using ExRaiseSqlError method passing in sqlcode, string and numeric parameters 2) concatenating SQLError code and the parameters via ComDiagsArea << DgSqlCode << DgStringParam0 or DgCatalogName etc. Option 1 doesn't support the concept of DgCatalogName or DgSchemaName . When an error needs to be populated in both ways, we had to change it to use String0 and String1 etc.. > Streamline error handling in Executor utility commands > -- > > Key: TRAFODION-3009 > URL: https://issues.apache.org/jira/browse/TRAFODION-3009 > Project: Apache Trafodion > Issue Type: Improvement > Components: -exe, sql-exe >Reporter: Selvaganesan Govindarajan >Assignee: Selvaganesan Govindarajan >Priority: Major > > Executor utility commands are the commands that issue sql statements during > its execution to complete its task. These operators are derived from > ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is > allocated in advance. This is not in accordance with the design as outlined in > [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design] > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[ https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16427257#comment-16427257 ] ASF GitHub Bot commented on TRAFODION-3009: --- Github user DaveBirdsall commented on a diff in the pull request: https://github.com/apache/trafodion/pull/1504#discussion_r179530143 --- Diff: core/sql/bin/SqlciErrors.txt --- @@ -2,7 +2,7 @@ 1000 42000 9 BEGINNER INFRM LOGONLY A syntax error occurred. 1001 Z 9 ADVANCED CRTCL DIALOUT An internal error occurred in module $0~String0 on line $1~Int0. Details($2~String1). 1002 Z 9 BEGINNER MAJOR DBADMIN Catalog $0~CatalogName does not exist. -1003 Z 9 BEGINNER MINOR DBADMIN Schema $0~SchemaName does not exist. +1003 Z 9 BEGINNER MINOR DBADMIN Schema $0~String0.$1~String1 does not exist. --- End diff -- I'm guessing String0 is CatalogName and String1 is SchemaName. If so, slightly better would be $0~CatalogName.$0~SchemaName > Streamline error handling in Executor utility commands > -- > > Key: TRAFODION-3009 > URL: https://issues.apache.org/jira/browse/TRAFODION-3009 > Project: Apache Trafodion > Issue Type: Improvement > Components: -exe, sql-exe >Reporter: Selvaganesan Govindarajan >Assignee: Selvaganesan Govindarajan >Priority: Major > > Executor utility commands are the commands that issue sql statements during > its execution to complete its task. These operators are derived from > ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is > allocated in advance. This is not in accordance with the design as outlined in > [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design] > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[
https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16427258#comment-16427258
]
ASF GitHub Bot commented on TRAFODION-3009:
---
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1504#discussion_r17950
--- Diff: core/sql/sqlcomp/CmpSeabaseDDLschema.cpp ---
@@ -410,7 +410,8 @@ Int64 schemaUID = getObjectTypeandOwner(&cliInterface,
if (schemaUID < 0)
{
*CmpCommon::diags() << DgSqlCode(-CAT_SCHEMA_DOES_NOT_EXIST_ERROR)
- << DgSchemaName(catalogName + "." + schemaName);
+ << DgString0(catalogName)
--- End diff --
Consider using DgCatalogName and DgSchemaName instead of DgString0 and
DgString1. If we build a manageability infrastructure that collects event data,
it is good for the data to by typed by token.
> Streamline error handling in Executor utility commands
> --
>
> Key: TRAFODION-3009
> URL: https://issues.apache.org/jira/browse/TRAFODION-3009
> Project: Apache Trafodion
> Issue Type: Improvement
> Components: -exe, sql-exe
>Reporter: Selvaganesan Govindarajan
>Assignee: Selvaganesan Govindarajan
>Priority: Major
>
> Executor utility commands are the commands that issue sql statements during
> its execution to complete its task. These operators are derived from
> ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is
> allocated in advance. This is not in accordance with the design as outlined in
> [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design]
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[
https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16426152#comment-16426152
]
ASF GitHub Bot commented on TRAFODION-3009:
---
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1504#discussion_r179279239
--- Diff: core/sql/executor/ExExeUtilGet.cpp ---
@@ -1818,8 +1813,9 @@ short ExExeUtilGetMetadataInfoTcb::work()
{
if (!CmpCommon::context()->isAuthorizationEnabled())
{
- ComDiagsArea * diags = getDiagsArea();
- *diags << DgSqlCode(-CAT_AUTHORIZATION_NOT_ENABLED);
+ ComDiagsArea * diagsArea = getDiagsArea();
+ ExRaiseSqlError(getHeap(), &diagsArea,
-CAT_AUTHORIZATION_NOT_ENABLED);
+ setDiagsArea(diagsArea);
--- End diff --
I misunderstood your comment. I thought that you wanted to make a function.
Yes. I am changing it to pass in diagsArea_ directly, so that we can do this in
a single statement. diagsArea_ already has protected mode access.
> Streamline error handling in Executor utility commands
> --
>
> Key: TRAFODION-3009
> URL: https://issues.apache.org/jira/browse/TRAFODION-3009
> Project: Apache Trafodion
> Issue Type: Improvement
> Components: -exe, sql-exe
>Reporter: Selvaganesan Govindarajan
>Assignee: Selvaganesan Govindarajan
>Priority: Major
>
> Executor utility commands are the commands that issue sql statements during
> its execution to complete its task. These operators are derived from
> ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is
> allocated in advance. This is not in accordance with the design as outlined in
> [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design]
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[
https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16424653#comment-16424653
]
ASF GitHub Bot commented on TRAFODION-3009:
---
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1504#discussion_r178971037
--- Diff: core/sql/executor/ExExeUtilGet.cpp ---
@@ -1818,8 +1813,9 @@ short ExExeUtilGetMetadataInfoTcb::work()
{
if (!CmpCommon::context()->isAuthorizationEnabled())
{
- ComDiagsArea * diags = getDiagsArea();
- *diags << DgSqlCode(-CAT_AUTHORIZATION_NOT_ENABLED);
+ ComDiagsArea * diagsArea = getDiagsArea();
+ ExRaiseSqlError(getHeap(), &diagsArea,
-CAT_AUTHORIZATION_NOT_ENABLED);
+ setDiagsArea(diagsArea);
--- End diff --
Sorry, I'm not sure I understand. What I am suggesting doesn't require you
to change any other cases, it just makes this case (and the many repetitions of
it in your commit) simpler.
> Streamline error handling in Executor utility commands
> --
>
> Key: TRAFODION-3009
> URL: https://issues.apache.org/jira/browse/TRAFODION-3009
> Project: Apache Trafodion
> Issue Type: Improvement
> Components: -exe, sql-exe
>Reporter: Selvaganesan Govindarajan
>Assignee: Selvaganesan Govindarajan
>Priority: Major
>
> Executor utility commands are the commands that issue sql statements during
> its execution to complete its task. These operators are derived from
> ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is
> allocated in advance. This is not in accordance with the design as outlined in
> [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design]
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[ https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16424649#comment-16424649 ] ASF GitHub Bot commented on TRAFODION-3009: --- Github user zellerh commented on a diff in the pull request: https://github.com/apache/trafodion/pull/1504#discussion_r178970491 --- Diff: core/sql/exp/ExpError.cpp --- @@ -135,6 +135,20 @@ ComDiagsArea *ExRaiseSqlError(CollHeap* heap, ComDiagsArea** diagsArea, intParam1, intParam2, intParam3, stringParam1, stringParam2, stringParam3); } + +ComDiagsArea *ExRaiseSqlError(CollHeap* heap, ComDiagsArea** diagsArea, --- End diff -- Thanks for the explanation. > Streamline error handling in Executor utility commands > -- > > Key: TRAFODION-3009 > URL: https://issues.apache.org/jira/browse/TRAFODION-3009 > Project: Apache Trafodion > Issue Type: Improvement > Components: -exe, sql-exe >Reporter: Selvaganesan Govindarajan >Assignee: Selvaganesan Govindarajan >Priority: Major > > Executor utility commands are the commands that issue sql statements during > its execution to complete its task. These operators are derived from > ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is > allocated in advance. This is not in accordance with the design as outlined in > [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design] > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[
https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16424536#comment-16424536
]
ASF GitHub Bot commented on TRAFODION-3009:
---
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1504#discussion_r178949774
--- Diff: core/sql/executor/ExExeUtilGet.cpp ---
@@ -1818,8 +1813,9 @@ short ExExeUtilGetMetadataInfoTcb::work()
{
if (!CmpCommon::context()->isAuthorizationEnabled())
{
- ComDiagsArea * diags = getDiagsArea();
- *diags << DgSqlCode(-CAT_AUTHORIZATION_NOT_ENABLED);
+ ComDiagsArea * diagsArea = getDiagsArea();
+ ExRaiseSqlError(getHeap(), &diagsArea,
-CAT_AUTHORIZATION_NOT_ENABLED);
+ setDiagsArea(diagsArea);
--- End diff --
I thought about it 3 statements when I made this change. However, there are
places where the diagsArea comes up from the queue entries.
> Streamline error handling in Executor utility commands
> --
>
> Key: TRAFODION-3009
> URL: https://issues.apache.org/jira/browse/TRAFODION-3009
> Project: Apache Trafodion
> Issue Type: Improvement
> Components: -exe, sql-exe
>Reporter: Selvaganesan Govindarajan
>Assignee: Selvaganesan Govindarajan
>Priority: Major
>
> Executor utility commands are the commands that issue sql statements during
> its execution to complete its task. These operators are derived from
> ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is
> allocated in advance. This is not in accordance with the design as outlined in
> [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design]
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[ https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16424530#comment-16424530 ] ASF GitHub Bot commented on TRAFODION-3009: --- Github user selvaganesang commented on a diff in the pull request: https://github.com/apache/trafodion/pull/1504#discussion_r178947965 --- Diff: core/sql/exp/ExpError.cpp --- @@ -135,6 +135,20 @@ ComDiagsArea *ExRaiseSqlError(CollHeap* heap, ComDiagsArea** diagsArea, intParam1, intParam2, intParam3, stringParam1, stringParam2, stringParam3); } + +ComDiagsArea *ExRaiseSqlError(CollHeap* heap, ComDiagsArea** diagsArea, --- End diff -- This function is modeled after other ExRaiseSqlError functions already in this file. I wasn't sure how ComDiagsArea would be passed to the callers of ExRaiseSqlError, meaning if it would exist or not. If the ComDiagsArea is passed to the caller as a parameter to itself and then it would be become recursive change to ensure that all the function in the chain passes reference pointers. The existing ExRaiseSqlError excepts the enum ExeErrorCode needs to be updated to raise sql error correctly. However, with the embedded arkcmp concepts, even the non-executor error codes needs to be passed to this function. Casting to enum ExeErrorCode when the code doesn't exist in it causes unexpected value. Hence this new function was introduced to raise sql error. Earlier, the code assumed that diagsArea is always allocated and populated it. > Streamline error handling in Executor utility commands > -- > > Key: TRAFODION-3009 > URL: https://issues.apache.org/jira/browse/TRAFODION-3009 > Project: Apache Trafodion > Issue Type: Improvement > Components: -exe, sql-exe >Reporter: Selvaganesan Govindarajan >Assignee: Selvaganesan Govindarajan >Priority: Major > > Executor utility commands are the commands that issue sql statements during > its execution to complete its task. These operators are derived from > ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is > allocated in advance. This is not in accordance with the design as outlined in > [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design] > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[
https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16424529#comment-16424529
]
ASF GitHub Bot commented on TRAFODION-3009:
---
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1504#discussion_r178944754
--- Diff: core/sql/executor/ExExeUtilCli.cpp ---
@@ -2091,27 +2084,57 @@ Lng32 ExeCliInterface::deleteContext(char*
contextHandle) // in buf contains con
Lng32 ExeCliInterface::retrieveSQLDiagnostics(ComDiagsArea *toDiags)
{
Lng32 retcode;
-
+ ex_assert(toDiags != NULL, "ComDiagsArea is null");
if (diagsArea_ != NULL)
{
diagsArea_->clear();
diagsArea_->deAllocate();
}
+ retcode = SQL_EXEC_MergeDiagnostics_Internal(*toDiags);
+ SQL_EXEC_ClearDiagnostics(NULL);
+ return retcode;
+}
- if (toDiags != NULL)
-{
- retcode = SQL_EXEC_MergeDiagnostics_Internal(*toDiags);
- SQL_EXEC_ClearDiagnostics(NULL);
-}
- else
+
+ComDiagsArea *ExeCliInterface::allocAndRetrieveSQLDiagnostics(ComDiagsArea
*&toDiags)
+{
+ Lng32 retcode;
+
+ if (diagsArea_ != NULL)
{
- diagsArea_ = ComDiagsArea::allocate(heap_);
- retcode = SQL_EXEC_MergeDiagnostics_Internal(*diagsArea_);
+ diagsArea_->clear();
+ diagsArea_->deAllocate();
}
+ if (toDiags == NULL)
+ toDiags = ComDiagsArea::allocate(heap_);
- return retcode;
+ retcode = SQL_EXEC_MergeDiagnostics_Internal(*toDiags);
+ SQL_EXEC_ClearDiagnostics(NULL);
+
+ if (retcode == 0)
+ return toDiags;
+ else
+ return NULL;
--- End diff --
I will fix it by de-allocating if it is allocated before returning NULL
> Streamline error handling in Executor utility commands
> --
>
> Key: TRAFODION-3009
> URL: https://issues.apache.org/jira/browse/TRAFODION-3009
> Project: Apache Trafodion
> Issue Type: Improvement
> Components: -exe, sql-exe
>Reporter: Selvaganesan Govindarajan
>Assignee: Selvaganesan Govindarajan
>Priority: Major
>
> Executor utility commands are the commands that issue sql statements during
> its execution to complete its task. These operators are derived from
> ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is
> allocated in advance. This is not in accordance with the design as outlined in
> [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design]
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[
https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16424514#comment-16424514
]
ASF GitHub Bot commented on TRAFODION-3009:
---
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1504#discussion_r178944088
--- Diff: core/sql/executor/ExExeUtilExplain.cpp ---
@@ -237,8 +237,7 @@ short ExExeUtilDisplayExplainTcb::work()
executeImmediate("control session 'EXPLAIN' 'ON';");
if (retcode < 0)
{
- cliInterface()->retrieveSQLDiagnostics(getDiagsArea());
-
+
setDiagsArea(cliInterface()->allocAndRetrieveSQLDiagnostics(getDiagsArea()));
--- End diff --
Yes. I could do that for ExExeUtilTcb portion of the code.
> Streamline error handling in Executor utility commands
> --
>
> Key: TRAFODION-3009
> URL: https://issues.apache.org/jira/browse/TRAFODION-3009
> Project: Apache Trafodion
> Issue Type: Improvement
> Components: -exe, sql-exe
>Reporter: Selvaganesan Govindarajan
>Assignee: Selvaganesan Govindarajan
>Priority: Major
>
> Executor utility commands are the commands that issue sql statements during
> its execution to complete its task. These operators are derived from
> ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is
> allocated in advance. This is not in accordance with the design as outlined in
> [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design]
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[ https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16424502#comment-16424502 ] ASF GitHub Bot commented on TRAFODION-3009: --- Github user zellerh commented on a diff in the pull request: https://github.com/apache/trafodion/pull/1504#discussion_r178940492 --- Diff: core/sql/executor/ExExeUtilCommon.cpp --- @@ -669,7 +669,9 @@ short ExExeUtilTcb::executeQuery(char * task, char * stringParam1 = NULL; Lng32 intParam1 = ComDiags_UnInitialized_Int; - retcode = (short)cliInterface()->retrieveSQLDiagnostics(getDiagsArea()); + setDiagsArea(cliInterface()->allocAndRetrieveSQLDiagnostics(getDiagsArea())); +if (getDiagsArea() != NULL) + retcode = 0; --- End diff -- Thanks for the explanation. > Streamline error handling in Executor utility commands > -- > > Key: TRAFODION-3009 > URL: https://issues.apache.org/jira/browse/TRAFODION-3009 > Project: Apache Trafodion > Issue Type: Improvement > Components: -exe, sql-exe >Reporter: Selvaganesan Govindarajan >Assignee: Selvaganesan Govindarajan >Priority: Major > > Executor utility commands are the commands that issue sql statements during > its execution to complete its task. These operators are derived from > ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is > allocated in advance. This is not in accordance with the design as outlined in > [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design] > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[ https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16424459#comment-16424459 ] ASF GitHub Bot commented on TRAFODION-3009: --- Github user selvaganesang commented on a diff in the pull request: https://github.com/apache/trafodion/pull/1504#discussion_r178932278 --- Diff: core/sql/executor/ExExeUtilCommon.cpp --- @@ -669,7 +669,9 @@ short ExExeUtilTcb::executeQuery(char * task, char * stringParam1 = NULL; Lng32 intParam1 = ComDiags_UnInitialized_Int; - retcode = (short)cliInterface()->retrieveSQLDiagnostics(getDiagsArea()); + setDiagsArea(cliInterface()->allocAndRetrieveSQLDiagnostics(getDiagsArea())); +if (getDiagsArea() != NULL) + retcode = 0; --- End diff -- Yes. We can rely on that assumption. It is strange that the earlier code sets the sqlcode to the return code of SQL_EXEC_MergeDiagnosticsInternal to be the sqlcode. See line 700 below. When we are able to obtain the diagnostics area, the sqlcode is set the main error code > Streamline error handling in Executor utility commands > -- > > Key: TRAFODION-3009 > URL: https://issues.apache.org/jira/browse/TRAFODION-3009 > Project: Apache Trafodion > Issue Type: Improvement > Components: -exe, sql-exe >Reporter: Selvaganesan Govindarajan >Assignee: Selvaganesan Govindarajan >Priority: Major > > Executor utility commands are the commands that issue sql statements during > its execution to complete its task. These operators are derived from > ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is > allocated in advance. This is not in accordance with the design as outlined in > [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design] > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[
https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16424438#comment-16424438
]
ASF GitHub Bot commented on TRAFODION-3009:
---
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1504#discussion_r178926825
--- Diff: core/sql/executor/ExExeUtilCli.cpp ---
@@ -2091,27 +2084,57 @@ Lng32 ExeCliInterface::deleteContext(char*
contextHandle) // in buf contains con
Lng32 ExeCliInterface::retrieveSQLDiagnostics(ComDiagsArea *toDiags)
{
Lng32 retcode;
-
+ ex_assert(toDiags != NULL, "ComDiagsArea is null");
if (diagsArea_ != NULL)
{
diagsArea_->clear();
diagsArea_->deAllocate();
}
+ retcode = SQL_EXEC_MergeDiagnostics_Internal(*toDiags);
+ SQL_EXEC_ClearDiagnostics(NULL);
+ return retcode;
+}
- if (toDiags != NULL)
-{
- retcode = SQL_EXEC_MergeDiagnostics_Internal(*toDiags);
- SQL_EXEC_ClearDiagnostics(NULL);
-}
- else
+
+ComDiagsArea *ExeCliInterface::allocAndRetrieveSQLDiagnostics(ComDiagsArea
*&toDiags)
+{
+ Lng32 retcode;
+
+ if (diagsArea_ != NULL)
{
- diagsArea_ = ComDiagsArea::allocate(heap_);
- retcode = SQL_EXEC_MergeDiagnostics_Internal(*diagsArea_);
+ diagsArea_->clear();
+ diagsArea_->deAllocate();
}
+ if (toDiags == NULL)
+ toDiags = ComDiagsArea::allocate(heap_);
- return retcode;
+ retcode = SQL_EXEC_MergeDiagnostics_Internal(*toDiags);
+ SQL_EXEC_ClearDiagnostics(NULL);
+
+ if (retcode == 0)
+ return toDiags;
+ else
+ return NULL;
}
+void ExeCliInterface::retrieveOrSaveSQLDiagnostics(Lng32 cliRetcode,
ComDiagsArea *diagsArea)
--- End diff --
This function is not used. I created this function to avoid creating
ComDiagsArea in advance for load operations. When I attempted to refactor the
retrieveSQLDiagnostics and allocAndRetrieveSQLDiagnostics, it opened up more
issues. I will remove this function
> Streamline error handling in Executor utility commands
> --
>
> Key: TRAFODION-3009
> URL: https://issues.apache.org/jira/browse/TRAFODION-3009
> Project: Apache Trafodion
> Issue Type: Improvement
> Components: -exe, sql-exe
>Reporter: Selvaganesan Govindarajan
>Assignee: Selvaganesan Govindarajan
>Priority: Major
>
> Executor utility commands are the commands that issue sql statements during
> its execution to complete its task. These operators are derived from
> ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is
> allocated in advance. This is not in accordance with the design as outlined in
> [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design]
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[
https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16424360#comment-16424360
]
ASF GitHub Bot commented on TRAFODION-3009:
---
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1504#discussion_r178900839
--- Diff: core/sql/executor/ExExeUtilExplain.cpp ---
@@ -237,8 +237,7 @@ short ExExeUtilDisplayExplainTcb::work()
executeImmediate("control session 'EXPLAIN' 'ON';");
if (retcode < 0)
{
- cliInterface()->retrieveSQLDiagnostics(getDiagsArea());
-
+
setDiagsArea(cliInterface()->allocAndRetrieveSQLDiagnostics(getDiagsArea()));
--- End diff --
From what I can tell, ExExeUtilTcb::getDiagsArea() returns a ComDiagsArea
*. The compiler then makes a reference of this temporary value and passes it to
allocAndRetrieveSQLDiagnostics(). That method may allocate a new diags area. If
we then encounter an error when merging the diags area in
allocAndRetrieveSQLDiagnostics(), it will return NULL and the allocated diags
area is leaked. See also comment above. I think it would be better to create an
ExExeUtilTcb member function like this:
```
void retrieveCliDiags() {
cliInterface_->allocAndRetrieveSQLDiagnistics(diagsArea_); }
```
Then, the line here could be replace by a call to retrieveCliDiags() and we
wouldn't have to deal with all the potential complications mentioned in this
and a previous comment.
> Streamline error handling in Executor utility commands
> --
>
> Key: TRAFODION-3009
> URL: https://issues.apache.org/jira/browse/TRAFODION-3009
> Project: Apache Trafodion
> Issue Type: Improvement
> Components: -exe, sql-exe
>Reporter: Selvaganesan Govindarajan
>Assignee: Selvaganesan Govindarajan
>Priority: Major
>
> Executor utility commands are the commands that issue sql statements during
> its execution to complete its task. These operators are derived from
> ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is
> allocated in advance. This is not in accordance with the design as outlined in
> [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design]
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[
https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16424361#comment-16424361
]
ASF GitHub Bot commented on TRAFODION-3009:
---
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1504#discussion_r178906139
--- Diff: core/sql/executor/ExExeUtilGet.cpp ---
@@ -1818,8 +1813,9 @@ short ExExeUtilGetMetadataInfoTcb::work()
{
if (!CmpCommon::context()->isAuthorizationEnabled())
{
- ComDiagsArea * diags = getDiagsArea();
- *diags << DgSqlCode(-CAT_AUTHORIZATION_NOT_ENABLED);
+ ComDiagsArea * diagsArea = getDiagsArea();
+ ExRaiseSqlError(getHeap(), &diagsArea,
-CAT_AUTHORIZATION_NOT_ENABLED);
+ setDiagsArea(diagsArea);
--- End diff --
Having to do these three statements in so many places is not ideal. Would
it make sense to make diagsArea_ a protected member, so we could pass it to
ExRaiseSqlError here, in just a single statement:
```
ExRaiseSqlError(getHeap(), &diagsArea_, ...);
```
> Streamline error handling in Executor utility commands
> --
>
> Key: TRAFODION-3009
> URL: https://issues.apache.org/jira/browse/TRAFODION-3009
> Project: Apache Trafodion
> Issue Type: Improvement
> Components: -exe, sql-exe
>Reporter: Selvaganesan Govindarajan
>Assignee: Selvaganesan Govindarajan
>Priority: Major
>
> Executor utility commands are the commands that issue sql statements during
> its execution to complete its task. These operators are derived from
> ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is
> allocated in advance. This is not in accordance with the design as outlined in
> [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design]
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[ https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16424364#comment-16424364 ] ASF GitHub Bot commented on TRAFODION-3009: --- Github user zellerh commented on a diff in the pull request: https://github.com/apache/trafodion/pull/1504#discussion_r178909282 --- Diff: core/sql/exp/ExpError.cpp --- @@ -135,6 +135,20 @@ ComDiagsArea *ExRaiseSqlError(CollHeap* heap, ComDiagsArea** diagsArea, intParam1, intParam2, intParam3, stringParam1, stringParam2, stringParam3); } + +ComDiagsArea *ExRaiseSqlError(CollHeap* heap, ComDiagsArea** diagsArea, --- End diff -- In many other places we use ComDiagsArea *&diags, so I'm wondering why we are using ** here. > Streamline error handling in Executor utility commands > -- > > Key: TRAFODION-3009 > URL: https://issues.apache.org/jira/browse/TRAFODION-3009 > Project: Apache Trafodion > Issue Type: Improvement > Components: -exe, sql-exe >Reporter: Selvaganesan Govindarajan >Assignee: Selvaganesan Govindarajan >Priority: Major > > Executor utility commands are the commands that issue sql statements during > its execution to complete its task. These operators are derived from > ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is > allocated in advance. This is not in accordance with the design as outlined in > [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design] > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[
https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16424363#comment-16424363
]
ASF GitHub Bot commented on TRAFODION-3009:
---
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1504#discussion_r178901892
--- Diff: core/sql/executor/ExExeUtilCli.cpp ---
@@ -2091,27 +2084,57 @@ Lng32 ExeCliInterface::deleteContext(char*
contextHandle) // in buf contains con
Lng32 ExeCliInterface::retrieveSQLDiagnostics(ComDiagsArea *toDiags)
{
Lng32 retcode;
-
+ ex_assert(toDiags != NULL, "ComDiagsArea is null");
if (diagsArea_ != NULL)
{
diagsArea_->clear();
diagsArea_->deAllocate();
}
+ retcode = SQL_EXEC_MergeDiagnostics_Internal(*toDiags);
+ SQL_EXEC_ClearDiagnostics(NULL);
+ return retcode;
+}
- if (toDiags != NULL)
-{
- retcode = SQL_EXEC_MergeDiagnostics_Internal(*toDiags);
- SQL_EXEC_ClearDiagnostics(NULL);
-}
- else
+
+ComDiagsArea *ExeCliInterface::allocAndRetrieveSQLDiagnostics(ComDiagsArea
*&toDiags)
+{
+ Lng32 retcode;
+
+ if (diagsArea_ != NULL)
{
- diagsArea_ = ComDiagsArea::allocate(heap_);
- retcode = SQL_EXEC_MergeDiagnostics_Internal(*diagsArea_);
+ diagsArea_->clear();
+ diagsArea_->deAllocate();
}
+ if (toDiags == NULL)
+ toDiags = ComDiagsArea::allocate(heap_);
- return retcode;
+ retcode = SQL_EXEC_MergeDiagnostics_Internal(*toDiags);
+ SQL_EXEC_ClearDiagnostics(NULL);
+
+ if (retcode == 0)
+ return toDiags;
+ else
+ return NULL;
--- End diff --
See also below for a possible leak. Instead of returning NULL, it may be
better to add a condition to the allocated diags area. That would make it
easier for the caller to check (no check needed in most cases).
> Streamline error handling in Executor utility commands
> --
>
> Key: TRAFODION-3009
> URL: https://issues.apache.org/jira/browse/TRAFODION-3009
> Project: Apache Trafodion
> Issue Type: Improvement
> Components: -exe, sql-exe
>Reporter: Selvaganesan Govindarajan
>Assignee: Selvaganesan Govindarajan
>Priority: Major
>
> Executor utility commands are the commands that issue sql statements during
> its execution to complete its task. These operators are derived from
> ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is
> allocated in advance. This is not in accordance with the design as outlined in
> [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design]
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[
https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16424362#comment-16424362
]
ASF GitHub Bot commented on TRAFODION-3009:
---
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1504#discussion_r178896649
--- Diff: core/sql/executor/ExExeUtilCli.cpp ---
@@ -2091,27 +2084,57 @@ Lng32 ExeCliInterface::deleteContext(char*
contextHandle) // in buf contains con
Lng32 ExeCliInterface::retrieveSQLDiagnostics(ComDiagsArea *toDiags)
{
Lng32 retcode;
-
+ ex_assert(toDiags != NULL, "ComDiagsArea is null");
if (diagsArea_ != NULL)
{
diagsArea_->clear();
diagsArea_->deAllocate();
}
+ retcode = SQL_EXEC_MergeDiagnostics_Internal(*toDiags);
+ SQL_EXEC_ClearDiagnostics(NULL);
+ return retcode;
+}
- if (toDiags != NULL)
-{
- retcode = SQL_EXEC_MergeDiagnostics_Internal(*toDiags);
- SQL_EXEC_ClearDiagnostics(NULL);
-}
- else
+
+ComDiagsArea *ExeCliInterface::allocAndRetrieveSQLDiagnostics(ComDiagsArea
*&toDiags)
+{
+ Lng32 retcode;
+
+ if (diagsArea_ != NULL)
{
- diagsArea_ = ComDiagsArea::allocate(heap_);
- retcode = SQL_EXEC_MergeDiagnostics_Internal(*diagsArea_);
+ diagsArea_->clear();
+ diagsArea_->deAllocate();
}
+ if (toDiags == NULL)
+ toDiags = ComDiagsArea::allocate(heap_);
- return retcode;
+ retcode = SQL_EXEC_MergeDiagnostics_Internal(*toDiags);
+ SQL_EXEC_ClearDiagnostics(NULL);
+
+ if (retcode == 0)
+ return toDiags;
+ else
+ return NULL;
}
+void ExeCliInterface::retrieveOrSaveSQLDiagnostics(Lng32 cliRetcode,
ComDiagsArea *diagsArea)
--- End diff --
A comment would help here that we are returning the diagnostics if
diagsArea is non-null and that we are saving them if NULL is passed in.
> Streamline error handling in Executor utility commands
> --
>
> Key: TRAFODION-3009
> URL: https://issues.apache.org/jira/browse/TRAFODION-3009
> Project: Apache Trafodion
> Issue Type: Improvement
> Components: -exe, sql-exe
>Reporter: Selvaganesan Govindarajan
>Assignee: Selvaganesan Govindarajan
>Priority: Major
>
> Executor utility commands are the commands that issue sql statements during
> its execution to complete its task. These operators are derived from
> ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is
> allocated in advance. This is not in accordance with the design as outlined in
> [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design]
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[ https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16424359#comment-16424359 ] ASF GitHub Bot commented on TRAFODION-3009: --- Github user zellerh commented on a diff in the pull request: https://github.com/apache/trafodion/pull/1504#discussion_r178898607 --- Diff: core/sql/executor/ExExeUtilCommon.cpp --- @@ -669,7 +669,9 @@ short ExExeUtilTcb::executeQuery(char * task, char * stringParam1 = NULL; Lng32 intParam1 = ComDiags_UnInitialized_Int; - retcode = (short)cliInterface()->retrieveSQLDiagnostics(getDiagsArea()); + setDiagsArea(cliInterface()->allocAndRetrieveSQLDiagnostics(getDiagsArea())); +if (getDiagsArea() != NULL) + retcode = 0; --- End diff -- Should we rely on the assumption that retcode is non-zero when we reach here? Maybe better to set it explicitly to some defined value, something indicating that an error occurred but that we found no diagnostics information? > Streamline error handling in Executor utility commands > -- > > Key: TRAFODION-3009 > URL: https://issues.apache.org/jira/browse/TRAFODION-3009 > Project: Apache Trafodion > Issue Type: Improvement > Components: -exe, sql-exe >Reporter: Selvaganesan Govindarajan >Assignee: Selvaganesan Govindarajan >Priority: Major > > Executor utility commands are the commands that issue sql statements during > its execution to complete its task. These operators are derived from > ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is > allocated in advance. This is not in accordance with the design as outlined in > [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design] > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TRAFODION-3009) Streamline error handling in Executor utility commands
[ https://issues.apache.org/jira/browse/TRAFODION-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16423343#comment-16423343 ] ASF GitHub Bot commented on TRAFODION-3009: --- GitHub user selvaganesang opened a pull request: https://github.com/apache/trafodion/pull/1504 [TRAFODION-3009] Streamline error handling in Executor utility commands ComDiagsArea is now allocated only when there are warnings or error in all the utility commands except load. In case of load, the ComDiagsArea is allocated in advance to report error rows count. This requires all the executor utility commands to use a new version of ExRaiseSqlError to populate diagnostics area. [TRAFODION-3017] Simplify the hive client access in Trafodion Hive Client functions are now moved to a new file HiveClient_JNI.h and HiveClient_JNI.cpp. Most of the HiveClient functions are static functions allowing to use HiveClient in Trafodion with ease. You can merge this pull request into a Git repository by running: $ git pull https://github.com/selvaganesang/trafodion trafodion-3009_1 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafodion/pull/1504.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1504 commit 1f44166c843fd1a6e8b325a2fcbe0cf8158e099b Author: selvaganesang Date: 2018-03-29T00:13:15Z [TRAFODION-3009] Streamline error handling in Executor utility commands ComDiagsArea is now allocated only when there are warnings or error in all the utility commands except load. In case of load, the ComDiagsArea is allocated in advance to report error rows count. This requires all the executor utility commands to use a new version of ExRaiseSqlError to populate diagnostics area. [TRAFODION-3017] Simplify the hive client access in Trafodion Hive Client functions are now moved to a new file HiveClient_JNI.h and HiveClient_JNI.cpp. Most of the HiveClient functions are static functions allowing to use HiveClient in Trafodion with ease. > Streamline error handling in Executor utility commands > -- > > Key: TRAFODION-3009 > URL: https://issues.apache.org/jira/browse/TRAFODION-3009 > Project: Apache Trafodion > Issue Type: Improvement > Components: -exe, sql-exe >Reporter: Selvaganesan Govindarajan >Assignee: Selvaganesan Govindarajan >Priority: Major > > Executor utility commands are the commands that issue sql statements during > its execution to complete its task. These operators are derived from > ExExeUtilTcb operator in Trafodion. These commands assume ComDiagsArea is > allocated in advance. This is not in accordance with the design as outlined in > [https://cwiki.apache.org/confluence/display/TRAFODION/SQL+Diagnostics+Architecture+and+Design] > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
