You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by mashengchen <gi...@git.apache.org> on 2018/03/07 01:23:24 UTC

[GitHub] trafodion pull request #1464: TRAFODION-2981 mxosrvr crashes during explain ...

GitHub user mashengchen opened a pull request:

    https://github.com/apache/trafodion/pull/1464

    TRAFODION-2981 mxosrvr crashes during explain using EXPLAIN_QID

    SELECT * FROM TABLE(explain(null, 'EXPLAIN_QID=MXID11000007019212384153894533416000000002206U3333308T150010100_923_SQL_CUR_2'));
    this is related to TRAFODION-1755


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/mashengchen/trafodion TRAFODION-2981

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/trafodion/pull/1464.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 #1464
    
----
commit 6b8fc454df57076d54c3e566eec8bcd3ea806a6d
Author: aven <sh...@...>
Date:   2018-03-06T09:59:04Z

    TRAFODION-2981 mxosrvr crashes during explain using EXPLAIN_QID

----


---

[GitHub] trafodion pull request #1464: TRAFODION-2981 mxosrvr crashes during explain ...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/trafodion/pull/1464


---

[GitHub] trafodion pull request #1464: TRAFODION-2981 mxosrvr crashes during explain ...

Posted by sandhyasun <gi...@git.apache.org>.
Github user sandhyasun commented on a diff in the pull request:

    https://github.com/apache/trafodion/pull/1464#discussion_r173272707
  
    --- Diff: core/sql/executor/ExExplain.cpp ---
    @@ -1724,9 +1724,9 @@ short ExExplainTcb::getExplainFromRepos(char * qid, Lng32 qidLen)
       if (vi->get(0, ptr, len))
         goto label_error2;
       
    -  explainFragLen_ = str_decoded_len(len); // remove trailing null terminator
    +  explainFragLen_ = str_decoded_len(len - 1); // remove trailing null terminator
    --- End diff --
    
    The JIRA isn't showing  trace fileor debugging info. Can you explain what the issue was with passingin 'len' into str_decoded_len ? Seems like the explainFragLen_ would have had sufficient length to allocate the explain fragment ? Some explanation in the PR or JIRA will help. 


---

[GitHub] trafodion pull request #1464: TRAFODION-2981 mxosrvr crashes during explain ...

Posted by mashengchen <gi...@git.apache.org>.
Github user mashengchen commented on a diff in the pull request:

    https://github.com/apache/trafodion/pull/1464#discussion_r173354026
  
    --- Diff: core/sql/executor/ExExplain.cpp ---
    @@ -1724,9 +1724,9 @@ short ExExplainTcb::getExplainFromRepos(char * qid, Lng32 qidLen)
       if (vi->get(0, ptr, len))
         goto label_error2;
       
    -  explainFragLen_ = str_decoded_len(len); // remove trailing null terminator
    +  explainFragLen_ = str_decoded_len(len - 1); // remove trailing null terminator
    --- End diff --
    
    for the '-1' , i think it should remove trailinng null terminator as the comment.
    but last time in anoop 's commit, the '-1' was removed, but the method (getExplainFromRepos) is for EXPLAIN_QID,  not for EXPLAIN_STMT (see TRAFODION-1755), so i guess he changed the place there is no need to change.


---

[GitHub] trafodion pull request #1464: TRAFODION-2981 mxosrvr crashes during explain ...

Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:

    https://github.com/apache/trafodion/pull/1464#discussion_r174177517
  
    --- Diff: core/sql/executor/ExExplain.cpp ---
    @@ -1724,9 +1724,9 @@ short ExExplainTcb::getExplainFromRepos(char * qid, Lng32 qidLen)
       if (vi->get(0, ptr, len))
         goto label_error2;
       
    -  explainFragLen_ = str_decoded_len(len); // remove trailing null terminator
    +  explainFragLen_ = str_decoded_len(len - 1); // remove trailing null terminator
    --- End diff --
    
    @sandhyasun, are you content with this change now? Can I merge it? 


---