[ 
https://issues.apache.org/jira/browse/TRAFODION-2635?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16041828#comment-16041828
 ] 

ASF GitHub Bot commented on TRAFODION-2635:
-------------------------------------------

GitHub user DaveBirdsall opened a pull request:

    https://github.com/apache/incubator-trafodion/pull/1114

    [TRAFODION-2635] Fix two bugs re: HBASE CELL access on Traf objects

    This set of changes fixes two bugs.
    
    1.  In ValueIdList::computeEncodedKey (optimizer/ValueDesc.cpp), if we used 
HBASE.\”_CELL_\” access for a salted Trafodion table, and we had no begin/end 
key (e.g. select count(*) from hbase.\”_CELL_\”.\”TRAFODION.SCH.XYZ\”), the 
code would dereference a null pointer.
    2.  In NATableDB::get, if we were creating an NATable object on the NATable 
cache for an HBASE.\”_CELL_\” access to a Trafodion table, the HBase-related 
descriptors were allocated on the statement heap instead of the context heap, 
resulting in a dangling pointer (with unpredictable failure modes) when the 
object was accessed that way in a second statement.
    
    The change to optimizer/ValueDesc.cpp fixes the first bug.
    
    The remaining changes fix the second bug.
    
    Why so many changes for the second bug?
    
    Partly, it is because routines in generator/Generator.cpp that wrote these 
descriptors into compiled plans were reused to obtain these descriptors for 
compile time logic. Unfortunately, the memory models are different: For 
compiled plans we use ComSpace for heap allocation (as we need the allocations 
to be contiguous in memory). For compiler access, we use an NAHeap. Before this 
set of changes, this was hidden in the GENHEAP macro in sqlcat/TrafDDLdesc.h; 
the code would use a ComSpace object if the space parameter to several methods 
was non-null, and use the statement heap if the space parameter was null.
    
    The need to (sometimes) use the context heap in NATableDB::get meant that 
this GENHEAP scheme could no longer work. Since ComSpace and NAHeap inherit 
from NAMemory, one possibility was to change all relevant descriptor methods to 
use an NAMemory * parameter instead of ComSpace *. This set of changes attempts 
this possibility. There is a nuance: The NAMemory class hierarchy does not use 
virtual methods. So, when there is a need for polymorphism, the calling 
routines must explicitly cast the NAMemory pointer to the proper class. 
Fortunately, there was only one place where this was necessary. I’m not 
completely satisfied with this solution as it seems a dangerous coding 
practice. No simple alternative seemed to be available however.
    
    After making this change, I encountered numerous test failures due to 
uninitialized variables in the various descriptors. Perhaps objects on the 
Statement heap are guaranteed (or vastly more likely) to be zeroed out while 
those on the Context heap are not. As a result, I added initializers for many 
of the descriptor members in sqlcat/TrafDDLdesc.h. I am not confident that I 
got all of the ones needed (I did initialize all pointer members so those are 
safe).
    
    Please do not merge this until a full regression suite run has passed, 
which I’m doing on a workstation.
    


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

    $ git pull https://github.com/DaveBirdsall/incubator-trafodion Trafodion2635

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

    https://github.com/apache/incubator-trafodion/pull/1114.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 #1114
    
----
commit 8d9c1d173d77adfa0fc274dac6b9ce89dc1aae51
Author: Dave Birdsall <dbirds...@apache.org>
Date:   2017-06-07T22:53:38Z

    [TRAFODION-2635] Fix two bugs concerning HBASE CELL access on Traf objects

----


> Core on select count(*) using hbase cell access on a salted Trafodion table
> ---------------------------------------------------------------------------
>
>                 Key: TRAFODION-2635
>                 URL: https://issues.apache.org/jira/browse/TRAFODION-2635
>             Project: Apache Trafodion
>          Issue Type: Bug
>          Components: sql-cmp
>    Affects Versions: 2.2-incubating
>         Environment: All
>            Reporter: David Wayne Birdsall
>            Assignee: David Wayne Birdsall
>
> The following sqlci script demonstrates the problem:
> set schema sch;
> create table t1 
> ( a int not null, b int, primary key (a) )
> salt using 4 partitions;
> prepare s1 from select count(*) from hbase."_CELL_"."TRAFODION.SCH.T1";
> When doing the prepare, sqlci cores. The stack trace is as follows:
> (gdb) bt
> #0  0x00007f2c6036a625 in raise () from /lib64/libc.so.6
> #1  0x00007f2c6036bd8d in abort () from /lib64/libc.so.6
> #2  0x00007f2c62145165 in ?? ()
>    from /usr/lib/jvm/java-1.7.0-openjdk.x86_64/jre/lib/amd64/server/libjvm.so
> #3  0x00007f2c622b985f in ?? ()
>    from /usr/lib/jvm/java-1.7.0-openjdk.x86_64/jre/lib/amd64/server/libjvm.so
> #4  0x00007f2c62149d02 in JVM_handle_linux_signal ()
>    from /usr/lib/jvm/java-1.7.0-openjdk.x86_64/jre/lib/amd64/server/libjvm.so
> #5  <signal handler called>
> #6  0x00000000004028ba in folly::fbstring_core<char>::category (this=0x10)
>     at ../export/FBString.h:1001
> #7  0x0000000000402890 in folly::fbstring_core<char>::size (this=0x10)
>     at ../export/FBString.h:803
> #8  0x000000000040285c in folly::basic_fbstring<char, std::char_traits<char>, 
> std::allocator<char>, folly::fbstring_core<char> >::size (this=0x10)
>     at ../export/FBString.h:1388
> #9  0x00007f2c5fd92318 in folly::basic_fbstring<char, std::char_traits<char>, 
> std::allocator<char>, folly::fbstring_core<char> >::find (this=0x10, 
>     needle=0x7f2c5898d71e "_ISO88591", pos=0, nsize=9)
>     at ../export/FBString.h:1960
> #10 0x00007f2c5fd8f899 in NAString::index (this=0x0, 
>     pattern=0x7f2c5898d71e "_ISO88591", plen=9, startIndex=0, 
>     cmp=NAString::exact) at ../export/NAStringDef.cpp:316
> #11 0x00007f2c629e89ac in NAString::index (this=0x0, 
>     s=0x7f2c5898d71e "_ISO88591", i=0, cmp=NAString::exact)
>     at ../export/NAStringDef.h:581
> #12 0x00007f2c588174e8 in ValueIdList::computeEncodedKey 
> (this=0x7f2c3c5886c0, 
>     tDesc=0x7f2c3e4dffd8, isMaxKey=0, encodedKeyBuffer=@0x7ffd8cf4cc80, 
>     keyBufLen=@0x7ffd8cf4cc7c) at ../optimizer/ValueDesc.cpp:6802
> #13 0x00007f2c5861df90 in 
> RangePartitioningFunction::computeNumOfActivePartitions (this=0x7f2c3e4e3300, 
> skey=0x7f2c3c5885b8, tDesc=0x7f2c3e4dffd8)
>     at ../optimizer/PartFunc.cpp:5110
> #14 0x00007f2c586a7d16 in FileScan::FileScan (this=0x7f2c3c587a10, 
>     tableName=..., tableDescPtr=0x7f2c3e4dffd8, indexDescPtr=0x7f2c3e4e17e0, 
>     isReverseScan=0, baseCardinality=@0x7ffd8cf4cf3c, accessOpts=..., 
>     groupAttributesPtr=0x7f2c3e4bd508, selectionPredicates=..., 
> disjuncts=..., 
>     generatedCCPreds=..., otype=REL_HBASE_ACCESS)
>     at ../optimizer/RelExpr.cpp:9671
> #15 0x00007f2c586a9fe2 in HbaseAccess::HbaseAccess (this=0x7f2c3c587a10, 
>     corrName=..., tableDesc=0x7f2c3e4dffd8, idx=0x7f2c3e4e17e0, 
>     isReverseScan=0, baseCardinality=@0x7ffd8cf4cf3c, accessOptions=..., 
>     groupAttributesPtr=0x7f2c3e4bd508, selectionPredicates=..., 
> disjuncts=..., 
>     generatedCCPreds=..., otype=REL_HBASE_ACCESS, oHeap=0x7f2c3e4d4b90)
>     at ../optimizer/RelExpr.cpp:10139
> #16 0x00007f2c5837b897 in createAndInsertHbaseScan (idesc=0x7f2c3e4e17e0, bef=
>     0x7f2c3e4bcd70, memory=@0x7ffd8cf4d460, disjunctsPtr=0x7f2c3c586ef8, 
>     generatedCCPreds=..., oc=SAME_ORDER, ixMdamFlag=MDAM_OFF)
>     at ../optimizer/ImplRule.cpp:1083
> #17 0x00007f2c5837ba99 in createAndInsertScan (idesc=0x7f2c3e4e17e0, 
>     bef=0x7f2c3e4bcd70, memory=@0x7ffd8cf4d460, disjunctsPtr=0x7f2c3c586ef8, 
>     generatedCCPreds=..., oc=SAME_ORDER, ixMdamFlag=MDAM_OFF, isHbase=1)
>     at ../optimizer/ImplRule.cpp:1122
> #18 0x00007f2c5837c0d0 in generateScanSubstitutes (before=0x7f2c3e4bcd70, 
>     context=0x7f2c3c5877e8, memory=@0x7ffd8cf4d460, isHbase=1)
>     at ../optimizer/ImplRule.cpp:1257
> #19 0x00007f2c5837d677 in HbaseScanRule::nextSubstitute (this=0x7f2c4d888d68, 
>     before=0x7f2c3e4bcd70, context=0x7f2c3c5877e8, memory=@0x7ffd8cf4d460)
>     at ../optimizer/ImplRule.cpp:1662
> #20 0x00007f2c587c2251 in ApplyRuleTask::perform (this=0x7f2c3c586920, 
>     taskId=11) at ../optimizer/tasks.cpp:1188
> #21 0x00007f2c5857d06d in QueryOptimizerDriver::optimizeAPassHelper (
>     this=0x7ffd8cf519b0, context=0x7f2c3c583c08) at ../optimizer/opt.cpp:7084
> #22 0x00007f2c5857cc00 in QueryOptimizerDriver::optimizeAPass (
>     this=0x7ffd8cf519b0, context=0x7f2c3c583c08) at ../optimizer/opt.cpp:7017
> #23 0x00007f2c5857c824 in QueryOptimizerDriver::doPass2PerhapsPass1 (
>     this=0x7ffd8cf519b0, relExpr=0x7f2c3e4bd868, context=0x7f2c3c583c08, 
>     original=0x7f2c3c57c530) at ../optimizer/opt.cpp:6944
> #24 0x00007f2c5857b80c in RelExpr::optimize2 (this=0x7f2c3e4bd868)
>     at ../optimizer/opt.cpp:6640
> #25 0x00007f2c585a5b6c in RelExpr::optimizeNode (this=0x7f2c3e4bd868)
>     at ../optimizer/OptLogRelExpr.cpp:76
> #26 0x00007f2c5a09cf27 in CmpMain::compile (this=0x7ffd8cf55370, 
>     input_str=0x7f2c3e4c3148 "select count(*) from 
> hbase.\"_CELL_\".\"TRAFODION.SCH.T1\";", charset=15, 
> queryExpr=@0x7ffd8cf55258, gen_code=0x7f2c3e4d6f10, 
>     gen_code_len=0x7f2c3e4d6f08, heap=0x7f2c630e26d0, phase=CmpMain::END, 
>     fragmentDir=0x7ffd8cf554c0, op=3004, useQueryCache=CmpMain::NORMAL, 
>     cacheable=0x7ffd8cf55244, begTime=0x7ffd8cf55260, shouldLog=0)
>     at ../sqlcomp/CmpMain.cpp:2319
> #27 0x00007f2c5a09ad05 in CmpMain::sqlcomp (this=0x7ffd8cf55370, 
>     input_str=0x7f2c3e4c3148 "select count(*) from 
> hbase.\"_CELL_\".\"TRAFODION.SCH.T1\";", charset=15, 
> queryExpr=@0x7ffd8cf55258, gen_code=0x7f2c3e4d6f10, 
>     gen_code_len=0x7f2c3e4d6f08, heap=0x7f2c630e26d0, phase=CmpMain::END, 
>     fragmentDir=0x7ffd8cf554c0, op=3004, useQueryCache=CmpMain::NORMAL, 
>     cacheable=0x7ffd8cf55244, begTime=0x7ffd8cf55260, shouldLog=0)
>     at ../sqlcomp/CmpMain.cpp:1701
> #28 0x00007f2c5a0985f5 in CmpMain::sqlcomp (this=0x7ffd8cf55370, input=..., 
>     gen_code=0x7f2c3e4d6f10, gen_code_len=0x7f2c3e4d6f08, 
> heap=0x7f2c630e26d0, 
>     phase=CmpMain::END, fragmentDir=0x7ffd8cf554c0, op=3004, 
>     useQueryCache=CmpMain::NORMAL) at ../sqlcomp/CmpMain.cpp:817
> #29 0x00007f2c5f5fbae0 in CmpStatement::process (this=0x7f2c3e4c2fb8, 
>     sqltext=...) at ../arkcmp/CmpStatement.cpp:507
> #30 0x00007f2c5f5eb0b1 in CmpContext::compileDirect (this=0x7f2c4d852090, 
> ---Type <return> to continue, or q <return> to quit---
>     data=0x7f2c630ef5e8 "p", data_len=168, outHeap=0x7f2c631510b8, 
> charset=15, 
>     op=CmpMessageObj::SQLTEXT_COMPILE, gen_code=@0x7ffd8cf55a00, 
>     gen_code_len=@0x7ffd8cf55a0c, parserFlags=0, parentQid=0x0, 
>     parentQidLen=0, diagsArea=0x7f2c630ef698) at ../arkcmp/CmpContext.cpp:906
> #31 0x00007f2c600a6435 in CliStatement::prepare2 (this=0x7f2c630e1f30, 
>     source=0x7f2c631242c8 "select count(*) from 
> hbase.\"_CELL_\".\"TRAFODION.SCH.T1\";", diagsArea=..., passed_gen_code=0x0, 
> passed_gen_code_len=0, 
>     charset=15, unpackTdbs=1, cliFlags=144) at ../cli/Statement.cpp:1752
> #32 0x00007f2c600a54e4 in CliStatement::prepare (this=0x7f2c630e1f30, 
>     source=0x7f2c631242c8 "select count(*) from 
> hbase.\"_CELL_\".\"TRAFODION.SCH.T1\";", diagsArea=..., passed_gen_code=0x0, 
> passed_gen_code_len=0, 
>     charset=15, unpackTdbs=1, cliFlags=144) at ../cli/Statement.cpp:1405
> #33 0x00007f2c60025960 in SQLCLI_Prepare2 (cliGlobals=0x1939b70, 
>     statement_id=0x41ca5d0, sql_source=0x5491ff0, gencode_ptr=0x0, 
>     gencode_len=0, ret_gencode_len=0x0, query_cost_info=0x7ffd8cf57090, 
>     query_comp_stats_info=0x7ffd8cf55e00, uniqueStmtId=0x7ffd8cf56f00 "    ", 
>     uniqueStmtIdLen=0x7ffd8cf57150, flags=128) at ../cli/Cli.cpp:5840
> #34 0x00007f2c600c535a in SQL_EXEC_Prepare2 (statement_id=0x41ca5d0, 
>     sql_source=0x5491ff0, gencode_ptr=0x0, gencode_len=0, 
> ret_gencode_len=0x0, 
>     query_cost_info=0x7ffd8cf57090, comp_stats_info=0x7ffd8cf55e00, 
>     uniqueStmtId=0x7ffd8cf56f00 "    ", uniqueStmtIdLen=0x7ffd8cf57150, 
>     flags=128) at ../cli/CliExtern.cpp:5132
> #35 0x00007f2c629eb806 in SqlCmd::do_prepare (sqlci_env=0x1922240, 
>     prep_stmt=0x486b8e0, 
>     sqlStmt=0x486bc00 "select count(*) from 
> hbase.\"_CELL_\".\"TRAFODION.SCH.T1\";", resetLastExecStmt=1, rsIndex=0, 
> prepcode=0x0, statisticsType=0x0)
>     at ../sqlci/SqlCmd.cpp:998
> #36 0x00007f2c629f19e8 in Prepare::process (this=0x44648d0, 
>     sqlci_env=0x1922240) at ../sqlci/SqlCmd.cpp:3066
> #37 0x00007f2c629d1e44 in Obey::process (this=0x5467b20, sqlci_env=0x1922240)
>     at ../sqlci/Obey.cpp:267
> #38 0x00007f2c629db008 in SqlciEnv::executeCommands (this=0x1922240, 
>     input_stmt=@0x7ffd8cf57448) at ../sqlci/SqlciEnv.cpp:841
> #39 0x00007f2c629da749 in SqlciEnv::run (this=0x1922240)
>     at ../sqlci/SqlciEnv.cpp:650
> #40 0x00000000004026d8 in main (argc=1, argv=0x7ffd8cf57608)
>     at ../bin/SqlciMain.cpp:328
> (gdb) 
> The problem is that ValueIdList::computeEncodedKey attempts to dereference a 
> null pointer.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Reply via email to