You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by sandhyasun <gi...@git.apache.org> on 2018/01/31 18:38:29 UTC

[GitHub] trafodion pull request #1428: [TRAFODION-2874] New syntax to retrieve the LO...

GitHub user sandhyasun opened a pull request:

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

    [TRAFODION-2874] New syntax to retrieve the LOB HDFS filename, offset for a LOBhandle

     for both external and internal LOBs . Also added syntax to return starting offset of a particular LOB handle in the LOB Hdfs data file.

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

    $ git pull https://github.com/sandhyasun/trafodion traf_lob_global_fix

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

    https://github.com/apache/trafodion/pull/1428.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 #1428
    
----
commit a96968e401666a99f36b1729597dab23dd87b74b
Author: Sandhya Sundaresan <sa...@...>
Date:   2018-01-31T18:35:48Z

    New syntax to retrieve the LOB HDFS filename for both external and internal LOBs . Also added syntax to return starting offset of a particular LOB handle in the LOB Hdfs data file.

----


---

[GitHub] trafodion pull request #1428: [TRAFODION-2874] New syntax to retrieve the LO...

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

    https://github.com/apache/trafodion/pull/1428#discussion_r166720182
  
    --- Diff: core/sql/exp/ExpLOBaccess.cpp ---
    @@ -870,6 +870,67 @@ Ex_Lob_Error ExLob::getLength(char *handleIn, Int32 handleInLen,Int64 &outLobLen
           }
       return err;
     }
    +Ex_Lob_Error ExLob::getOffset(char *handleIn, Int32 handleInLen,Int64 &outLobOffset,LobsSubOper so, Int64 transId)
    +{
    +  char logBuf[4096];
    +  Int32 cliErr = 0;
    +  Ex_Lob_Error err=LOB_OPER_OK; 
    +  char *blackBox = new(getLobGlobalHeap()) char[MAX_LOB_FILE_NAME_LEN+6];
    +  Int32 blackBoxLen = 0;
    +  Int64 dummy = 0;
    +  Int32 dummy2 = 0;
    +  if (so != Lob_External_File)
    +    {
    +      
    +      cliErr = SQL_EXEC_LOBcliInterface(handleIn, handleInLen,NULL,NULL,NULL,NULL,LOB_CLI_SELECT_LOBOFFSET,LOB_CLI_ExecImmed,&outLobOffset,0, 0, 0,0,transId,lobTrace_);
    +    
    +      if (cliErr < 0 ) {
    +        str_sprintf(logBuf,"CLI SELECT_LOBOFFSET returned error %d",cliErr);
    +        lobDebugInfo(logBuf, 0,__LINE__,lobTrace_);
    +  
    +        return LOB_DESC_READ_ERROR;
    +      }
    +    }
    + 
    +  return err;
    +}
    +
    +Ex_Lob_Error ExLob::getFileName(char *handleIn, Int32 handleInLen, char *outFileName, Int32 &outFileLen , LobsSubOper so, Int64 transId)
    +{
    +  char logBuf[4096];
    +  Int32 cliErr = 0;
    +  Ex_Lob_Error err=LOB_OPER_OK; 
    +  Int64 dummy = 0;
    +  Int32 dummy2 = 0;
    +  if (so != Lob_External_File)
    +    {
    +      //Derive the filename from the LOB handle and return
    +      str_cpy_all(outFileName, (char *)lobDataFile_.data(),lobDataFile_.length());
    --- End diff --
    
    Internally we allocate enough to hold the name. But when the final result is copied out, it's copied out to a caller  provided address (64 bit address). So we are assuming the caller (mxosrvr)  has allocated enough space. 


---

[GitHub] trafodion pull request #1428: [TRAFODION-2874] New syntax to retrieve the LO...

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

    https://github.com/apache/trafodion/pull/1428#discussion_r192260422
  
    --- Diff: core/sql/exp/ExpLOBaccess.cpp ---
    @@ -870,6 +870,67 @@ Ex_Lob_Error ExLob::getLength(char *handleIn, Int32 handleInLen,Int64 &outLobLen
           }
       return err;
     }
    +Ex_Lob_Error ExLob::getOffset(char *handleIn, Int32 handleInLen,Int64 &outLobOffset,LobsSubOper so, Int64 transId)
    +{
    +  char logBuf[4096];
    +  Int32 cliErr = 0;
    +  Ex_Lob_Error err=LOB_OPER_OK; 
    +  char *blackBox = new(getLobGlobalHeap()) char[MAX_LOB_FILE_NAME_LEN+6];
    +  Int32 blackBoxLen = 0;
    +  Int64 dummy = 0;
    +  Int32 dummy2 = 0;
    +  if (so != Lob_External_File)
    +    {
    +      
    +      cliErr = SQL_EXEC_LOBcliInterface(handleIn, handleInLen,NULL,NULL,NULL,NULL,LOB_CLI_SELECT_LOBOFFSET,LOB_CLI_ExecImmed,&outLobOffset,0, 0, 0,0,transId,lobTrace_);
    +    
    +      if (cliErr < 0 ) {
    +        str_sprintf(logBuf,"CLI SELECT_LOBOFFSET returned error %d",cliErr);
    +        lobDebugInfo(logBuf, 0,__LINE__,lobTrace_);
    +  
    +        return LOB_DESC_READ_ERROR;
    +      }
    +    }
    + 
    +  return err;
    +}
    +
    +Ex_Lob_Error ExLob::getFileName(char *handleIn, Int32 handleInLen, char *outFileName, Int32 &outFileLen , LobsSubOper so, Int64 transId)
    +{
    +  char logBuf[4096];
    +  Int32 cliErr = 0;
    +  Ex_Lob_Error err=LOB_OPER_OK; 
    +  Int64 dummy = 0;
    +  Int32 dummy2 = 0;
    +  if (so != Lob_External_File)
    +    {
    +      //Derive the filename from the LOB handle and return
    +      str_cpy_all(outFileName, (char *)lobDataFile_.data(),lobDataFile_.length());
    --- End diff --
    
    On further thought, maybe there needs to be an extra check in the sscp. in ::processLobRequest where we should check if another process on another node has already propgated/set the same lock into the local shared segment and if so error out and ensure that the current  SetLobkLock call fails .  This way only one of them succeeds. There can be cases where both processes repeatedly fail but with AQR and erroring out seems like a safer thing to do. Will set @selvaganesang  comment on this issue. 


---

[GitHub] trafodion pull request #1428: [TRAFODION-2874] New syntax to retrieve the LO...

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

    https://github.com/apache/trafodion/pull/1428#discussion_r166421248
  
    --- Diff: core/sql/exp/ExpLOBaccess.cpp ---
    @@ -870,6 +870,67 @@ Ex_Lob_Error ExLob::getLength(char *handleIn, Int32 handleInLen,Int64 &outLobLen
           }
       return err;
     }
    +Ex_Lob_Error ExLob::getOffset(char *handleIn, Int32 handleInLen,Int64 &outLobOffset,LobsSubOper so, Int64 transId)
    +{
    +  char logBuf[4096];
    +  Int32 cliErr = 0;
    +  Ex_Lob_Error err=LOB_OPER_OK; 
    +  char *blackBox = new(getLobGlobalHeap()) char[MAX_LOB_FILE_NAME_LEN+6];
    +  Int32 blackBoxLen = 0;
    +  Int64 dummy = 0;
    +  Int32 dummy2 = 0;
    +  if (so != Lob_External_File)
    +    {
    +      
    +      cliErr = SQL_EXEC_LOBcliInterface(handleIn, handleInLen,NULL,NULL,NULL,NULL,LOB_CLI_SELECT_LOBOFFSET,LOB_CLI_ExecImmed,&outLobOffset,0, 0, 0,0,transId,lobTrace_);
    +    
    +      if (cliErr < 0 ) {
    +        str_sprintf(logBuf,"CLI SELECT_LOBOFFSET returned error %d",cliErr);
    +        lobDebugInfo(logBuf, 0,__LINE__,lobTrace_);
    +  
    +        return LOB_DESC_READ_ERROR;
    +      }
    +    }
    + 
    +  return err;
    +}
    +
    +Ex_Lob_Error ExLob::getFileName(char *handleIn, Int32 handleInLen, char *outFileName, Int32 &outFileLen , LobsSubOper so, Int64 transId)
    +{
    +  char logBuf[4096];
    +  Int32 cliErr = 0;
    +  Ex_Lob_Error err=LOB_OPER_OK; 
    +  Int64 dummy = 0;
    +  Int32 dummy2 = 0;
    +  if (so != Lob_External_File)
    +    {
    +      //Derive the filename from the LOB handle and return
    +      str_cpy_all(outFileName, (char *)lobDataFile_.data(),lobDataFile_.length());
    --- End diff --
    
    Do we know that the outFileName buffer is long enough?


---

[GitHub] trafodion pull request #1428: [TRAFODION-2874] New syntax to retrieve the LO...

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

    https://github.com/apache/trafodion/pull/1428#discussion_r166713210
  
    --- Diff: core/sql/cli/Cli.cpp ---
    @@ -9160,11 +9160,45 @@ Lng32 SQLCLI_LOBcliInterface
     
     	Int64 outlen = 0;Lng32 len = 0;
     	cliRC = cliInterface->executeImmediate(query,(char *)dataLen, &len, FALSE);
    -	    if (inoutDescPartnKey)
    -	      *inoutDescPartnKey = descPartnKey;
    +        if (inoutDescPartnKey)
    +          *inoutDescPartnKey = descPartnKey;
     
    -	    if (inoutDescSyskey)
    -	      *inoutDescSyskey = inDescSyskey;
    +        if (inoutDescSyskey)
    +          *inoutDescSyskey = inDescSyskey;
    +	    
    +	Lng32 saveCliErr = cliRC;
    +
    +	
    +	if (cliRC < 0)
    +	  {
    +	    cliInterface->retrieveSQLDiagnostics(myDiags);
    +	    
    +	    goto error_return;
    +	  }
    +
    +	cliRC = saveCliErr;
    +      }
    +      break;
    +     case LOB_CLI_SELECT_LOBOFFSET:
    +      {
    +	
    +	//Retrive offset of the first chunk
    +	str_sprintf(query, "select  c.dataOffset from table(ghost table %s) h, table(ghost table %s) c where h.descPartnKey = c.descPartnKey and h.syskey = c.descSyskey and h.descPartnKey = %ld and h.syskey = %ld and c.chunkNum = 1 for read committed access",
    +		    lobDescHandleName, lobDescChunksName, 
    +		    descPartnKey, inDescSyskey);
    +
    +        lobDebugInfo(query,0,__LINE__,lobTrace);
    +	// set parserflags to allow ghost table
    +	currContext.setSqlParserFlags(0x1);
    --- End diff --
    
    Good catch Dave ! It's missing here and from the code I cloned from the block above. Will fix both. 


---

[GitHub] trafodion pull request #1428: [TRAFODION-2874] New syntax to retrieve the LO...

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

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


---

[GitHub] trafodion pull request #1428: [TRAFODION-2874] New syntax to retrieve the LO...

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

    https://github.com/apache/trafodion/pull/1428#discussion_r166420852
  
    --- Diff: core/sql/exp/ExpLOBaccess.cpp ---
    @@ -870,6 +870,67 @@ Ex_Lob_Error ExLob::getLength(char *handleIn, Int32 handleInLen,Int64 &outLobLen
           }
       return err;
     }
    +Ex_Lob_Error ExLob::getOffset(char *handleIn, Int32 handleInLen,Int64 &outLobOffset,LobsSubOper so, Int64 transId)
    +{
    +  char logBuf[4096];
    +  Int32 cliErr = 0;
    +  Ex_Lob_Error err=LOB_OPER_OK; 
    +  char *blackBox = new(getLobGlobalHeap()) char[MAX_LOB_FILE_NAME_LEN+6];
    --- End diff --
    
    It doesn't look like this is used anywhere. Maybe we can delete this?


---

[GitHub] trafodion pull request #1428: [TRAFODION-2874] New syntax to retrieve the LO...

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

    https://github.com/apache/trafodion/pull/1428#discussion_r166419614
  
    --- Diff: core/sql/cli/Cli.cpp ---
    @@ -9160,11 +9160,45 @@ Lng32 SQLCLI_LOBcliInterface
     
     	Int64 outlen = 0;Lng32 len = 0;
     	cliRC = cliInterface->executeImmediate(query,(char *)dataLen, &len, FALSE);
    -	    if (inoutDescPartnKey)
    -	      *inoutDescPartnKey = descPartnKey;
    +        if (inoutDescPartnKey)
    +          *inoutDescPartnKey = descPartnKey;
     
    -	    if (inoutDescSyskey)
    -	      *inoutDescSyskey = inDescSyskey;
    +        if (inoutDescSyskey)
    +          *inoutDescSyskey = inDescSyskey;
    +	    
    +	Lng32 saveCliErr = cliRC;
    +
    +	
    +	if (cliRC < 0)
    +	  {
    +	    cliInterface->retrieveSQLDiagnostics(myDiags);
    +	    
    +	    goto error_return;
    +	  }
    +
    +	cliRC = saveCliErr;
    +      }
    +      break;
    +     case LOB_CLI_SELECT_LOBOFFSET:
    +      {
    +	
    +	//Retrive offset of the first chunk
    +	str_sprintf(query, "select  c.dataOffset from table(ghost table %s) h, table(ghost table %s) c where h.descPartnKey = c.descPartnKey and h.syskey = c.descSyskey and h.descPartnKey = %ld and h.syskey = %ld and c.chunkNum = 1 for read committed access",
    +		    lobDescHandleName, lobDescChunksName, 
    +		    descPartnKey, inDescSyskey);
    +
    +        lobDebugInfo(query,0,__LINE__,lobTrace);
    +	// set parserflags to allow ghost table
    +	currContext.setSqlParserFlags(0x1);
    --- End diff --
    
    Is there code somewhere to revert the parser flags setting?


---