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/11/30 18:33:00 UTC
[GitHub] trafodion pull request #1754: [TRAFODION-2408] Remove code that empties the ...
GitHub user sandhyasun opened a pull request:
https://github.com/apache/trafodion/pull/1754
[TRAFODION-2408] Remove code that empties the LOB data file when creating an empty_blob.
Remove code that renames the lob data file when an empty_blob is created.
When allocating the descriptor for a new empty_blob entry, the lob data file was being renamed. That is incorrect since the lob data file can contain data from other lob entries in the column. (There is one single lob data file for each LOB column and it can hold several blob entries in it). So remove the code that does this and simply update the descriptor files to have an entry which keeps the LOB offset and leave the LOB length as 0. This way we know the LOB is empty . If data is appended to it, the data will go into a new offset within that data file.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sandhyasun/trafodion traf_call_lib
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafodion/pull/1754.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 #1754
----
commit fb0ab72b30124418eba9473df5ac24743ffbc204
Author: Sandhya Sundaresan <sa...@...>
Date: 2018-11-30T18:26:31Z
Remove code that empties the LOB data file when creating an empty_blob.
----
---
[GitHub] trafodion pull request #1754: [TRAFODION-2408] Remove code that empties the ...
Posted by sandhyasun <gi...@git.apache.org>.
Github user sandhyasun commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1754#discussion_r238387612
--- Diff: core/sql/exp/ExpLOBaccess.cpp ---
@@ -1779,77 +1779,23 @@ Ex_Lob_Error ExLob::allocateDesc(ULng32 size, Int64 &descNum, Int64 &dataOffset,
lobDebugInfo("In ExLob::allocateDesc",0,__LINE__,lobTrace_);
Int32 openFlags = O_RDONLY ;
HDFS_Client_RetCode hdfsClientRetcode;
+ hdfsFileInfo *fInfo = NULL;
if (! useLibHdfs_) {
- if (size == 0) {
- // Delete and Create the Hdfs file by passing overwrite to TRUE
- hdfsClientRetcode = hdfsClient_->hdfsCreate(lobDataFile_.data(), TRUE, FALSE, FALSE);
- if (hdfsClientRetcode != HDFS_CLIENT_OK)
- return LOB_DATA_FILE_WRITE_ERROR;
- else {
- dataOffset = 0;
- return LOB_OPER_OK;
- }
- }
- else {
+
dataOffset = hdfsClient_->hdfsSize(hdfsClientRetcode);
--- End diff --
The dataOffset still needs to be set to the EOD of the datafile for any empty lob. For eg if we already have one lob entry int he data file and we are inserting an empty lob in the next row, then we need to retrieve dataoffset and in the descriptor we will set the size to 0 for that offset. It makes bookkeeping of the lob offsets and lengths consistent.
---
[GitHub] trafodion pull request #1754: [TRAFODION-2408] Remove code that empties the ...
Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1754#discussion_r238717456
--- Diff: core/sql/exp/ExpLOBaccess.cpp ---
@@ -1779,77 +1779,23 @@ Ex_Lob_Error ExLob::allocateDesc(ULng32 size, Int64 &descNum, Int64 &dataOffset,
lobDebugInfo("In ExLob::allocateDesc",0,__LINE__,lobTrace_);
Int32 openFlags = O_RDONLY ;
HDFS_Client_RetCode hdfsClientRetcode;
+ hdfsFileInfo *fInfo = NULL;
if (! useLibHdfs_) {
- if (size == 0) {
- // Delete and Create the Hdfs file by passing overwrite to TRUE
- hdfsClientRetcode = hdfsClient_->hdfsCreate(lobDataFile_.data(), TRUE, FALSE, FALSE);
- if (hdfsClientRetcode != HDFS_CLIENT_OK)
- return LOB_DATA_FILE_WRITE_ERROR;
- else {
- dataOffset = 0;
- return LOB_OPER_OK;
- }
- }
- else {
+
dataOffset = hdfsClient_->hdfsSize(hdfsClientRetcode);
--- End diff --
Does it make any difference if the offset is same as before or EOD when the size is set to 0 because some other lob data from a different row could be appending to this file anyway.
---
[GitHub] trafodion pull request #1754: [TRAFODION-2408] Remove code that empties the ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/trafodion/pull/1754
---
[GitHub] trafodion pull request #1754: [TRAFODION-2408] Remove code that empties the ...
Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1754#discussion_r238029142
--- Diff: core/sql/exp/ExpLOBaccess.cpp ---
@@ -1779,77 +1779,23 @@ Ex_Lob_Error ExLob::allocateDesc(ULng32 size, Int64 &descNum, Int64 &dataOffset,
lobDebugInfo("In ExLob::allocateDesc",0,__LINE__,lobTrace_);
Int32 openFlags = O_RDONLY ;
HDFS_Client_RetCode hdfsClientRetcode;
+ hdfsFileInfo *fInfo = NULL;
if (! useLibHdfs_) {
- if (size == 0) {
- // Delete and Create the Hdfs file by passing overwrite to TRUE
- hdfsClientRetcode = hdfsClient_->hdfsCreate(lobDataFile_.data(), TRUE, FALSE, FALSE);
- if (hdfsClientRetcode != HDFS_CLIENT_OK)
- return LOB_DATA_FILE_WRITE_ERROR;
- else {
- dataOffset = 0;
- return LOB_OPER_OK;
- }
- }
- else {
+
dataOffset = hdfsClient_->hdfsSize(hdfsClientRetcode);
--- End diff --
Is there a need to get hdfsSize if the size == 0 and possible set the dataOffset to -1 or some value to denote that it is an empty lob
---
[GitHub] trafodion pull request #1754: [TRAFODION-2408] Remove code that empties the ...
Posted by sandhyasun <gi...@git.apache.org>.
Github user sandhyasun commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1754#discussion_r238726891
--- Diff: core/sql/exp/ExpLOBaccess.cpp ---
@@ -1779,77 +1779,23 @@ Ex_Lob_Error ExLob::allocateDesc(ULng32 size, Int64 &descNum, Int64 &dataOffset,
lobDebugInfo("In ExLob::allocateDesc",0,__LINE__,lobTrace_);
Int32 openFlags = O_RDONLY ;
HDFS_Client_RetCode hdfsClientRetcode;
+ hdfsFileInfo *fInfo = NULL;
if (! useLibHdfs_) {
- if (size == 0) {
- // Delete and Create the Hdfs file by passing overwrite to TRUE
- hdfsClientRetcode = hdfsClient_->hdfsCreate(lobDataFile_.data(), TRUE, FALSE, FALSE);
- if (hdfsClientRetcode != HDFS_CLIENT_OK)
- return LOB_DATA_FILE_WRITE_ERROR;
- else {
- dataOffset = 0;
- return LOB_OPER_OK;
- }
- }
- else {
+
dataOffset = hdfsClient_->hdfsSize(hdfsClientRetcode);
--- End diff --
Only during GC it may be useful. It's not absolutely needed. But since we will be making empty_blob into an inline blob as soon as inline_blob is fully functional, this implementation will soon change anyway.
---