Github user traflm commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/642#discussion_r73778733
  
    --- Diff: core/sql/common/CharType.cpp ---
    @@ -1217,7 +1217,7 @@ void CharType::minMaxRepresentableValue(void* bufPtr,
       if (stringLiteral)
         {
           NABoolean isNull = FALSE;
    -      NABoolean res = createSQLLiteral((const char *) bufPtr, 
*stringLiteral, isNull, h);
    +      NABoolean res = createSQLLiteral((const char *) bufPtr  - 
getSQLnullHdrSize(), *stringLiteral, isNull, h);
    --- End diff --
    
    The calling to type->minRepresentableValue should pass the buf address that 
take null indicator buffer into account, good examples are in:
    -EncodedValue::minMaxValue()
    -ConstValue::ConstValue()
    
    As you can see in above two functions, the buffer ptr is set to the postion:
    buffer_start + null_header_len
    
    a buffer should like
    [nullHeader][VCHARLEN]Data  
    
    inside minRepresentableValue , it will treat the VCHARLEN, so it seems the 
agreement is the nullHeader is already bypassed.
    
    So the starting position of bufPtr is
    [nullHeader][VCHARLEN]Data  
                       ^
    
    However, in the createSQLLiteral() , it will again bypass the nullHeader:
      const char *valPtr = buf + getSQLnullHdrSize();
    
    So I did this change. As you correctly pointed out, this is dangerous... 
    A better way is to change createSQLLiteral, but there are some other places 
call   createSQLLiteral() which has different assumption of the starting 
position of the buffer ptr, ConstValue::getConstStr() for example, so I choose 
the simplest way, I am more nervous to change many other code which I don't 
understand where will be invoked. But yes, it is not good. 
    A better change is to fix in createSQLLiteral semantics, the input buffer 
ptr will be the position after nullHeader, but that will involve more changes 
which I feel not confident, but I will try.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to