[GitHub] [incubator-doris] wangbo commented on a change in pull request #1944: Segment v2 use string's real length(#1943)

2019-10-12 Thread GitBox
wangbo commented on a change in pull request #1944: Segment v2 use string's 
real length(#1943)
URL: https://github.com/apache/incubator-doris/pull/1944#discussion_r334230348
 
 

 ##
 File path: be/src/olap/field.h
 ##
 @@ -389,6 +397,7 @@ class VarcharField: public Field {
 return  _length - OLAP_STRING_MAX_BYTES;
 }
 
+//todo(wangbo) using _length replace (_length - OLAP_STRING_MAX_BYTES) 
when segment v2 release
 
 Review comment:
   ;allocate_memory is used by max/min value and row_cursor used to write value
   I checked latter case, varchar which allocated by Field.allocate_memory is 
written by direct copy,so the length here doesn't matter


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org



[GitHub] [incubator-doris] wangbo commented on a change in pull request #1944: Segment v2 use string's real length(#1943)

2019-10-12 Thread GitBox
wangbo commented on a change in pull request #1944: Segment v2 use string's 
real length(#1943)
URL: https://github.com/apache/incubator-doris/pull/1944#discussion_r334229094
 
 

 ##
 File path: be/src/olap/types.h
 ##
 @@ -601,22 +594,11 @@ struct FieldTypeTraits : public 
FieldTypeTraitssize = value_len;
 return OLAP_SUCCESS;
 }
-static void set_to_max(void* buf) {
 
 Review comment:
   ;We can unify char/varchar's set_to_max


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org



[GitHub] [incubator-doris] wangbo commented on a change in pull request #1944: Segment v2 use string's real length(#1943)

2019-10-12 Thread GitBox
wangbo commented on a change in pull request #1944: Segment v2 use string's 
real length(#1943)
URL: https://github.com/apache/incubator-doris/pull/1944#discussion_r334228945
 
 

 ##
 File path: be/src/olap/field.h
 ##
 @@ -55,9 +55,9 @@ class Field {
 inline size_t field_size() const { return size() + 1; }
 inline size_t index_size() const { return _index_size; }
 
-inline void set_to_max(char* buf) const { return 
_type_info->set_to_max(buf); }
+virtual inline void set_to_max(char* buf) const { return 
_type_info->set_to_max(buf); }
 inline void set_to_min(char* buf) const { return 
_type_info->set_to_min(buf); }
-inline char* allocate_value_from_arena(Arena* arena) const { return 
_type_info->allocate_value_from_arena(arena); }
+virtual inline char* allocate_value_from_arena(Arena* arena) const { 
return _type_info->allocate_value_from_arena(arena); }
 
 Review comment:
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org



[GitHub] [incubator-doris] wangbo commented on a change in pull request #1944: Segment v2 use string's real length(#1943)

2019-10-12 Thread GitBox
wangbo commented on a change in pull request #1944: Segment v2 use string's 
real length(#1943)
URL: https://github.com/apache/incubator-doris/pull/1944#discussion_r334228828
 
 

 ##
 File path: be/src/olap/field.h
 ##
 @@ -55,9 +55,9 @@ class Field {
 inline size_t field_size() const { return size() + 1; }
 inline size_t index_size() const { return _index_size; }
 
-inline void set_to_max(char* buf) const { return 
_type_info->set_to_max(buf); }
+virtual inline void set_to_max(char* buf) const { return 
_type_info->set_to_max(buf); }
 inline void set_to_min(char* buf) const { return 
_type_info->set_to_min(buf); }
-inline char* allocate_value_from_arena(Arena* arena) const { return 
_type_info->allocate_value_from_arena(arena); }
+virtual inline char* allocate_value_from_arena(Arena* arena) const { 
return _type_info->allocate_value_from_arena(arena); }
 
 Review comment:
   But Field has no info about sizeof(CppType),only TypeInfo knows it


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org



[GitHub] [incubator-doris] wangbo commented on a change in pull request #1944: Segment v2 use string's real length(#1943)

2019-10-11 Thread GitBox
wangbo commented on a change in pull request #1944: Segment v2 use string's 
real length(#1943)
URL: https://github.com/apache/incubator-doris/pull/1944#discussion_r334218215
 
 

 ##
 File path: be/src/olap/field.h
 ##
 @@ -55,9 +55,11 @@ class Field {
 inline size_t field_size() const { return size() + 1; }
 inline size_t index_size() const { return _index_size; }
 
-inline void set_to_max(char* buf) const { return 
_type_info->set_to_max(buf); }
+virtual inline void set_to_max(char* buf) const { return 
_type_info->set_to_max(buf); }
+//todo delete this method when segment v2 release
+inline void set_to_max_v1(char* buf) const { return 
_type_info->set_to_max(buf); }
 
 Review comment:
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org



[GitHub] [incubator-doris] wangbo commented on a change in pull request #1944: Segment v2 use string's real length(#1943)

2019-10-11 Thread GitBox
wangbo commented on a change in pull request #1944: Segment v2 use string's 
real length(#1943)
URL: https://github.com/apache/incubator-doris/pull/1944#discussion_r334217064
 
 

 ##
 File path: be/src/olap/field.h
 ##
 @@ -55,9 +55,11 @@ class Field {
 inline size_t field_size() const { return size() + 1; }
 inline size_t index_size() const { return _index_size; }
 
-inline void set_to_max(char* buf) const { return 
_type_info->set_to_max(buf); }
+virtual inline void set_to_max(char* buf) const { return 
_type_info->set_to_max(buf); }
+//todo delete this method when segment v2 release
+inline void set_to_max_v1(char* buf) const { return 
_type_info->set_to_max(buf); }
 
 Review comment:
   > why the length of memset is gt than the char array's real length?
   
   For Varchar field,use set_to_max instead of set_to_max_v1 means using 
Field._length to set slice.size. For segment v1, Field._length=slice.size + 
sizeof(StringLengthType)(see WrapperField.create), Field._length > slice.size
   when calling set_to_max in segment v1 code,Field._length will be used to 
memset slice.size,so memset's length will gt than slice.size.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org



[GitHub] [incubator-doris] wangbo commented on a change in pull request #1944: Segment v2 use string's real length(#1943)

2019-10-11 Thread GitBox
wangbo commented on a change in pull request #1944: Segment v2 use string's 
real length(#1943)
URL: https://github.com/apache/incubator-doris/pull/1944#discussion_r333915743
 
 

 ##
 File path: be/src/olap/field.h
 ##
 @@ -55,9 +55,11 @@ class Field {
 inline size_t field_size() const { return size() + 1; }
 inline size_t index_size() const { return _index_size; }
 
-inline void set_to_max(char* buf) const { return 
_type_info->set_to_max(buf); }
+virtual inline void set_to_max(char* buf) const { return 
_type_info->set_to_max(buf); }
+//todo delete this method when segment v2 release
+inline void set_to_max_v1(char* buf) const { return 
_type_info->set_to_max(buf); }
 
 Review comment:
   when memset a char array to 0xff,if the length of memset is gt than the char 
array's real length,this may cause subsequently tcmalloc core dump,So I think 
it's better to memset using char array's real length. core dumped as below:
   
   >#0  SLL_Next (t=0x985ca2758b7b4b00) at src/linked_list.h:45
   >#1  SLL_TryPop (rv=, list=0x403ece0) at 
src/linked_list.h:69
   >#2  TryPop (rv=, this=0x403ece0) at 
src/thread_cache.h:220
   >#3  Allocate (oom_handler=0x25e8f00 , cl=5, size=64, this=) at src/thread_cache.h:379
   >#4  malloc_fast_path (size=size@entry=64) at 
src/tcmalloc.cc:1855
   >#5  tc_new (size=size@entry=64) at src/tcmalloc.cc:1976
   >#6  0x0119ba04 in doris::RowBlock::RowBlock (this=0x4894820, 
schema=)
   >at /home/wangbo/incubator-doris/be/src/olap/row_block.cpp:44
   >#7  0x00cbc5ad in doris::ColumnDataWriter::init (this=0x4894780) at 
/home/wangbo/incubator-doris/be/src/olap/rowset/column_data_writer.cpp:82
   >#8  0x00ca5d1c in doris::AlphaRowsetWriter::_init 
(this=this@entry=0x48c8500)
   >at 
/home/wangbo/incubator-doris/be/src/olap/rowset/alpha_rowset_writer.cpp:275
   >#9  0x00ca71b2 in doris::AlphaRowsetWriter::init (this=0x48c8500, 
rowset_writer_context=...)
   >at 
/home/wangbo/incubator-doris/be/src/olap/rowset/alpha_rowset_writer.cpp:73
   >#10 0x00c94df3 in doris::RowsetFactory::create_rowset_writer 
(context=..., output=output@entry=0x7fff27799208)
   >at /home/wangbo/incubator-doris/be/src/olap/rowset/rowset_factory.cpp:50
   >#11 0x00bc711a in doris::TabletManager::_create_inital_rowset 
(this=this@entry=0x4a2a180, tablet=..., request=...)
   >at /home/wangbo/incubator-doris/be/src/olap/tablet_manager.cpp:1250
   >#12 0x00bca139 in doris::TabletManager::_internal_create_tablet 
(this=this@entry=0x4a2a180, alter_type=alter_type@entry=doris::SCHEMA_CHANGE, 
   >request=..., 
is_schema_change_tablet=is_schema_change_tablet@entry=false, ref_tablet=..., 
data_dirs=...)
   >at /home/wangbo/incubator-doris/be/src/olap/tablet_manager.cpp:371
   >#13 0x00bcb592 in doris::TabletManager::create_tablet 
(this=this@entry=0x4a2a180, request=..., stores=...)
   >at /home/wangbo/incubator-doris/be/src/olap/tablet_manager.cpp:318
   >#14 0x00b7fc32 in doris::StorageEngine::create_tablet 
(this=, request=...)
   >at /home/wangbo/incubator-doris/be/src/olap/storage_engine.cpp:760
   >#15 0x00b4f97c in doris::TestDeleteConditionHandler::SetUp 
(this=0x4af2540)
   >at /home/wangbo/incubator-doris/be/test/olap/delete_handler_test.cpp:173
   >#16 0x02534737 in void 
testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) ()
   >#17 0x0252f29c in void 
testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) ()
   >#18 0x025145f7 in testing::Test::Run() ()
   >#19 0x02514ece in testing::TestInfo::Run() ()
   >#20 0x02515525 in testing::TestCase::Run() ()
   >#21 0x0251c099 in testing::internal::UnitTestImpl::RunAllTests() ()
   >#22 0x025356b3 in bool 
testing::internal::HandleSehExceptionsInMethodIfSupported(testing::internal::UnitTestImpl*, bool 
(testing::internal::UnitTestImpl::*)(), char const*) ()
   >#23 0x0252ffa2 in bool 
testing::internal::HandleExceptionsInMethodIfSupported(testing::internal::UnitTestImpl*, bool 
(testing::internal::UnitTestImpl::*)(), char const*) ()
   >#24 0x0251ad70 in testing::UnitTest::Run() ()
   >#25 0x009fb5f6 in RUN_ALL_TESTS () at 
/home/wangbo/incubator-doris/thirdparty/installed/include/gtest/gtest.h:2233
   >#26 main (argc=, argv=0x7fff27799e78) at 
/home/wangbo/incubator-doris/be/test/olap/delete_handler_test.cpp:951


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [incubator-doris] wangbo commented on a change in pull request #1944: Segment v2 use string's real length(#1943)

2019-10-11 Thread GitBox
wangbo commented on a change in pull request #1944: Segment v2 use string's 
real length(#1943)
URL: https://github.com/apache/incubator-doris/pull/1944#discussion_r333915743
 
 

 ##
 File path: be/src/olap/field.h
 ##
 @@ -55,9 +55,11 @@ class Field {
 inline size_t field_size() const { return size() + 1; }
 inline size_t index_size() const { return _index_size; }
 
-inline void set_to_max(char* buf) const { return 
_type_info->set_to_max(buf); }
+virtual inline void set_to_max(char* buf) const { return 
_type_info->set_to_max(buf); }
+//todo delete this method when segment v2 release
+inline void set_to_max_v1(char* buf) const { return 
_type_info->set_to_max(buf); }
 
 Review comment:
   when memset a char array to 0xff,if the length of memset is gt than the char 
array's real length,this may cause tcmalloc core dump,So I think it's better to 
memset using char array's real length. core dumped as below:
   
   >#0  SLL_Next (t=0x985ca2758b7b4b00) at src/linked_list.h:45
   >#1  SLL_TryPop (rv=, list=0x403ece0) at 
src/linked_list.h:69
   >#2  TryPop (rv=, this=0x403ece0) at 
src/thread_cache.h:220
   >#3  Allocate (oom_handler=0x25e8f00 , cl=5, size=64, this=) at src/thread_cache.h:379
   >#4  malloc_fast_path (size=size@entry=64) at 
src/tcmalloc.cc:1855
   >#5  tc_new (size=size@entry=64) at src/tcmalloc.cc:1976
   >#6  0x0119ba04 in doris::RowBlock::RowBlock (this=0x4894820, 
schema=)
   >at /home/wangbo/incubator-doris/be/src/olap/row_block.cpp:44
   >#7  0x00cbc5ad in doris::ColumnDataWriter::init (this=0x4894780) at 
/home/wangbo/incubator-doris/be/src/olap/rowset/column_data_writer.cpp:82
   >#8  0x00ca5d1c in doris::AlphaRowsetWriter::_init 
(this=this@entry=0x48c8500)
   >at 
/home/wangbo/incubator-doris/be/src/olap/rowset/alpha_rowset_writer.cpp:275
   >#9  0x00ca71b2 in doris::AlphaRowsetWriter::init (this=0x48c8500, 
rowset_writer_context=...)
   >at 
/home/wangbo/incubator-doris/be/src/olap/rowset/alpha_rowset_writer.cpp:73
   >#10 0x00c94df3 in doris::RowsetFactory::create_rowset_writer 
(context=..., output=output@entry=0x7fff27799208)
   >at /home/wangbo/incubator-doris/be/src/olap/rowset/rowset_factory.cpp:50
   >#11 0x00bc711a in doris::TabletManager::_create_inital_rowset 
(this=this@entry=0x4a2a180, tablet=..., request=...)
   >at /home/wangbo/incubator-doris/be/src/olap/tablet_manager.cpp:1250
   >#12 0x00bca139 in doris::TabletManager::_internal_create_tablet 
(this=this@entry=0x4a2a180, alter_type=alter_type@entry=doris::SCHEMA_CHANGE, 
   >request=..., 
is_schema_change_tablet=is_schema_change_tablet@entry=false, ref_tablet=..., 
data_dirs=...)
   >at /home/wangbo/incubator-doris/be/src/olap/tablet_manager.cpp:371
   >#13 0x00bcb592 in doris::TabletManager::create_tablet 
(this=this@entry=0x4a2a180, request=..., stores=...)
   >at /home/wangbo/incubator-doris/be/src/olap/tablet_manager.cpp:318
   >#14 0x00b7fc32 in doris::StorageEngine::create_tablet 
(this=, request=...)
   >at /home/wangbo/incubator-doris/be/src/olap/storage_engine.cpp:760
   >#15 0x00b4f97c in doris::TestDeleteConditionHandler::SetUp 
(this=0x4af2540)
   >at /home/wangbo/incubator-doris/be/test/olap/delete_handler_test.cpp:173
   >#16 0x02534737 in void 
testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) ()
   >#17 0x0252f29c in void 
testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) ()
   >#18 0x025145f7 in testing::Test::Run() ()
   >#19 0x02514ece in testing::TestInfo::Run() ()
   >#20 0x02515525 in testing::TestCase::Run() ()
   >#21 0x0251c099 in testing::internal::UnitTestImpl::RunAllTests() ()
   >#22 0x025356b3 in bool 
testing::internal::HandleSehExceptionsInMethodIfSupported(testing::internal::UnitTestImpl*, bool 
(testing::internal::UnitTestImpl::*)(), char const*) ()
   >#23 0x0252ffa2 in bool 
testing::internal::HandleExceptionsInMethodIfSupported(testing::internal::UnitTestImpl*, bool 
(testing::internal::UnitTestImpl::*)(), char const*) ()
   >#24 0x0251ad70 in testing::UnitTest::Run() ()
   >#25 0x009fb5f6 in RUN_ALL_TESTS () at 
/home/wangbo/incubator-doris/thirdparty/installed/include/gtest/gtest.h:2233
   >#26 main (argc=, argv=0x7fff27799e78) at 
/home/wangbo/incubator-doris/be/test/olap/delete_handler_test.cpp:951


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [incubator-doris] wangbo commented on a change in pull request #1944: Segment v2 use string's real length(#1943)

2019-10-11 Thread GitBox
wangbo commented on a change in pull request #1944: Segment v2 use string's 
real length(#1943)
URL: https://github.com/apache/incubator-doris/pull/1944#discussion_r333807895
 
 

 ##
 File path: be/src/olap/field.h
 ##
 @@ -55,9 +55,11 @@ class Field {
 inline size_t field_size() const { return size() + 1; }
 inline size_t index_size() const { return _index_size; }
 
-inline void set_to_max(char* buf) const { return 
_type_info->set_to_max(buf); }
+virtual inline void set_to_max(char* buf) const { return 
_type_info->set_to_max(buf); }
+//todo delete this method when segment v2 release
+inline void set_to_max_v1(char* buf) const { return 
_type_info->set_to_max(buf); }
 
 Review comment:
   implement it the same with set_to_max means:
when calling varchar field.set_to_max,using field._length to memset slice's 
data.
but for segment v1, field._length is regarded as 
slice.size+sizeof(StringLengthType)
   So when calling segment v1 varchar field.set_max,the length of memset is 
beyond slice's actually size,this may cause subsequently tcmalloc core 
dumped.see error stack below:
   >#0  SLL_Next (t=0x) at src/linked_list.h:45
   >#1  SLL_TryPop (rv=, list=0x436dce0) at 
src/linked_list.h:69
   >#2  TryPop (rv=, this=0x436dce0) at 
src/thread_cache.h:220
   >#3  Allocate (oom_handler=0x25e5700 , cl=5, size=64, this=) at src/thread_cache.h:379
   >#4  malloc_fast_path (size=64) at 
src/tcmalloc.cc:1855
   >#5  tc_new (size=64) at src/tcmalloc.cc:1976
   >#6  0x00c21672 in doris::WrapperField::WrapperField 
(this=0x4e07d40, rep=, variable_len=, 
   >is_string_type=) at 
/home/wangbo/incubator-doris/be/src/olap/wrapper_field.cpp:84
   >#7  0x00c21b9e in doris::WrapperField::create (column=..., 
len=, len@entry=0)
   >at /home/wangbo/incubator-doris/be/src/olap/wrapper_field.cpp:51
   >#8  0x00cb88e1 in doris::ColumnDataWriter::init (this=0x4bc4780) at 
/home/wangbo/incubator-doris/be/src/olap/rowset/column_data_writer.cpp:72
   >#9  0x00ca273c in doris::AlphaRowsetWriter::_init 
(this=this@entry=0x4bf83c0)
   >at 
/home/wangbo/incubator-doris/be/src/olap/rowset/alpha_rowset_writer.cpp:275
   >#10 0x00ca3694 in doris::AlphaRowsetWriter::init (this=0x4bf83c0, 
rowset_writer_context=...)
   >at 
/home/wangbo/incubator-doris/be/src/olap/rowset/alpha_rowset_writer.cpp:73
   >#11 0x00c91b03 in doris::RowsetFactory::create_rowset_writer 
(context=..., output=output@entry=0x7fffcdf75318)
   >at /home/wangbo/incubator-doris/be/src/olap/rowset/rowset_factory.cpp:50
   >#12 0x00bc9b3f in doris::TabletManager::_create_inital_rowset 
(this=this@entry=0x4d5a180, tablet=..., request=...)
   >at /home/wangbo/incubator-doris/be/src/olap/tablet_manager.cpp:1250
   >#13 0x00bcc919 in doris::TabletManager::_internal_create_tablet 
(this=this@entry=0x4d5a180, alter_type=alter_type@entry=doris::SCHEMA_CHANGE, 
   >request=..., 
is_schema_change_tablet=is_schema_change_tablet@entry=false, ref_tablet=..., 
data_dirs=...)
   >at /home/wangbo/incubator-doris/be/src/olap/tablet_manager.cpp:371
   >#14 0x00bcdd72 in doris::TabletManager::create_tablet 
(this=this@entry=0x4d5a180, request=..., stores=...)
   >at /home/wangbo/incubator-doris/be/src/olap/tablet_manager.cpp:318
   >#15 0x00b82642 in doris::StorageEngine::create_tablet 
(this=, request=...)
   >at /home/wangbo/incubator-doris/be/src/olap/storage_engine.cpp:760
   >#16 0x00b4fac3 in doris::TestDeleteConditionHandler::SetUp 
(this=0x4e08460)
   >at /home/wangbo/incubator-doris/be/test/olap/delete_handler_test.cpp:173
   >#17 0x02530f37 in void 
testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) ()
   >#18 0x0252ba9c in void 
testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) ()
   >#19 0x02510df7 in testing::Test::Run() ()
   >#20 0x025116ce in testing::TestInfo::Run() ()
   >#21 0x02511d25 in testing::TestCase::Run() ()
   >#22 0x02518899 in testing::internal::UnitTestImpl::RunAllTests() ()
   >#23 0x02531eb3 in bool 
testing::internal::HandleSehExceptionsInMethodIfSupported(testing::internal::UnitTestImpl*, bool 
(testing::internal::UnitTestImpl::*)(), char const*) ()
   >#24 0x0252c7a2 in bool 
testing::internal::HandleExceptionsInMethodIfSupported(testing::internal::UnitTestImpl*, bool 
(testing::internal::UnitTestImpl::*)(), char const*) ()
   >#25 0x02517570 in testing::UnitTest::Run() ()
   >#26 0x009fb316 in RUN_ALL_TESTS () at 
/home/wangbo/incubator-doris/thirdparty/installed/include/gtest/gtest.h:2233
   >#27 main (argc=, argv=0x7fffcdf75f88) at 
/home/wangbo/incubator-doris/be/test/olap/delete_handler_test.cpp:951
   
   it seems that memset memory to 0xff may effect tcmalloc 

[GitHub] [incubator-doris] wangbo commented on a change in pull request #1944: Segment v2 use string's real length(#1943)

2019-10-10 Thread GitBox
wangbo commented on a change in pull request #1944: Segment v2 use string's 
real length(#1943)
URL: https://github.com/apache/incubator-doris/pull/1944#discussion_r333808141
 
 

 ##
 File path: be/src/olap/rowset/segment_v2/column_writer.cpp
 ##
 @@ -72,13 +72,13 @@ class NullBitmapBuilder {
 };
 
 ColumnWriter::ColumnWriter(const ColumnWriterOptions& opts,
-   const TypeInfo* typeinfo,
+   Field* field,
 
 Review comment:
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org



[GitHub] [incubator-doris] wangbo commented on a change in pull request #1944: Segment v2 use string's real length(#1943)

2019-10-10 Thread GitBox
wangbo commented on a change in pull request #1944: Segment v2 use string's 
real length(#1943)
URL: https://github.com/apache/incubator-doris/pull/1944#discussion_r333808109
 
 

 ##
 File path: be/src/olap/rowset/segment_v2/segment_writer.cpp
 ##
 @@ -55,17 +55,17 @@ Status SegmentWriter::init(uint32_t write_mbytes_per_sec) {
 bool is_nullable = column.is_nullable();
 column_meta->set_is_nullable(is_nullable);
 
-// TODO(zc): we can add type_info into TabletColumn?
-const TypeInfo* type_info = get_type_info(column.type());
-DCHECK(type_info != nullptr);
-
 ColumnWriterOptions opts;
 opts.compression_type = segment_v2::CompressionTypePB::LZ4F;
 // now we create zone map for key columns
 if (column.is_key()) {
 opts.need_zone_map = true;
 }
-std::unique_ptr writer(new ColumnWriter(opts, type_info, 
is_nullable, _output_file.get()));
+
+Field* field = FieldFactory::create(column);
+DCHECK(field != nullptr);
+
+std::unique_ptr writer(new ColumnWriter(opts, field, 
is_nullable, _output_file.get()));
 
 Review comment:
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org



[GitHub] [incubator-doris] wangbo commented on a change in pull request #1944: Segment v2 use string's real length(#1943)

2019-10-10 Thread GitBox
wangbo commented on a change in pull request #1944: Segment v2 use string's 
real length(#1943)
URL: https://github.com/apache/incubator-doris/pull/1944#discussion_r333808062
 
 

 ##
 File path: be/src/olap/rowset/segment_v2/segment_writer.cpp
 ##
 @@ -55,17 +55,17 @@ Status SegmentWriter::init(uint32_t write_mbytes_per_sec) {
 bool is_nullable = column.is_nullable();
 column_meta->set_is_nullable(is_nullable);
 
-// TODO(zc): we can add type_info into TabletColumn?
-const TypeInfo* type_info = get_type_info(column.type());
-DCHECK(type_info != nullptr);
-
 ColumnWriterOptions opts;
 opts.compression_type = segment_v2::CompressionTypePB::LZ4F;
 // now we create zone map for key columns
 if (column.is_key()) {
 opts.need_zone_map = true;
 }
-std::unique_ptr writer(new ColumnWriter(opts, type_info, 
is_nullable, _output_file.get()));
+
+Field* field = FieldFactory::create(column);
 
 Review comment:
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org



[GitHub] [incubator-doris] wangbo commented on a change in pull request #1944: Segment v2 use string's real length(#1943)

2019-10-10 Thread GitBox
wangbo commented on a change in pull request #1944: Segment v2 use string's 
real length(#1943)
URL: https://github.com/apache/incubator-doris/pull/1944#discussion_r333807936
 
 

 ##
 File path: be/src/olap/field.h
 ##
 @@ -55,9 +55,11 @@ class Field {
 inline size_t field_size() const { return size() + 1; }
 inline size_t index_size() const { return _index_size; }
 
-inline void set_to_max(char* buf) const { return 
_type_info->set_to_max(buf); }
+virtual inline void set_to_max(char* buf) const { return 
_type_info->set_to_max(buf); }
+//todo delete this method when segment v2 release
 
 Review comment:
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org



[GitHub] [incubator-doris] wangbo commented on a change in pull request #1944: Segment v2 use string's real length(#1943)

2019-10-10 Thread GitBox
wangbo commented on a change in pull request #1944: Segment v2 use string's 
real length(#1943)
URL: https://github.com/apache/incubator-doris/pull/1944#discussion_r333807981
 
 

 ##
 File path: be/src/olap/field.h
 ##
 @@ -400,6 +411,16 @@ class VarcharField: public Field {
 VarcharField* clone() const override {
 return new VarcharField(*this);
 }
+
+char* allocate_value_from_arena(Arena* arena) const {
 
 Review comment:
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org



[GitHub] [incubator-doris] wangbo commented on a change in pull request #1944: Segment v2 use string's real length(#1943)

2019-10-10 Thread GitBox
wangbo commented on a change in pull request #1944: Segment v2 use string's 
real length(#1943)
URL: https://github.com/apache/incubator-doris/pull/1944#discussion_r333807995
 
 

 ##
 File path: be/src/olap/field.h
 ##
 @@ -400,6 +411,16 @@ class VarcharField: public Field {
 VarcharField* clone() const override {
 return new VarcharField(*this);
 }
+
+char* allocate_value_from_arena(Arena* arena) const {
+return Field::allocate_string_value_from_arena(arena);
+}
+
+void set_to_max(char* ch) {
 
 Review comment:
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org



[GitHub] [incubator-doris] wangbo commented on a change in pull request #1944: Segment v2 use string's real length(#1943)

2019-10-10 Thread GitBox
wangbo commented on a change in pull request #1944: Segment v2 use string's 
real length(#1943)
URL: https://github.com/apache/incubator-doris/pull/1944#discussion_r333807895
 
 

 ##
 File path: be/src/olap/field.h
 ##
 @@ -55,9 +55,11 @@ class Field {
 inline size_t field_size() const { return size() + 1; }
 inline size_t index_size() const { return _index_size; }
 
-inline void set_to_max(char* buf) const { return 
_type_info->set_to_max(buf); }
+virtual inline void set_to_max(char* buf) const { return 
_type_info->set_to_max(buf); }
+//todo delete this method when segment v2 release
+inline void set_to_max_v1(char* buf) const { return 
_type_info->set_to_max(buf); }
 
 Review comment:
   implement it the same with set_to_max means:
when calling varchar field.set_to_max,using field._length to memset slice's 
data.
but for segment v1, field._length is regarded as 
slice.size+sizeof(StringLengthType)
   So when calling segment v1 varchar field.set_max,the length of memset is 
beyond slice's actually size,this may cause subsequently tcmalloc core 
dumped.see error stack below:
   >#0  SLL_Next (t=0x) at src/linked_list.h:45
   >#1  SLL_TryPop (rv=, list=0x436dce0) at 
src/linked_list.h:69
   >#2  TryPop (rv=, this=0x436dce0) at 
src/thread_cache.h:220
   >#3  Allocate (oom_handler=0x25e5700 , cl=5, size=64, this=) at src/thread_cache.h:379
   >#4  malloc_fast_path (size=64) at 
src/tcmalloc.cc:1855
   >#5  tc_new (size=64) at src/tcmalloc.cc:1976
   >#6  0x00c21672 in doris::WrapperField::WrapperField 
(this=0x4e07d40, rep=, variable_len=, 
   >is_string_type=) at 
/home/wangbo/incubator-doris/be/src/olap/wrapper_field.cpp:84
   >#7  0x00c21b9e in doris::WrapperField::create (column=..., 
len=, len@entry=0)
   >at /home/wangbo/incubator-doris/be/src/olap/wrapper_field.cpp:51
   >#8  0x00cb88e1 in doris::ColumnDataWriter::init (this=0x4bc4780) at 
/home/wangbo/incubator-doris/be/src/olap/rowset/column_data_writer.cpp:72
   >#9  0x00ca273c in doris::AlphaRowsetWriter::_init 
(this=this@entry=0x4bf83c0)
   >at 
/home/wangbo/incubator-doris/be/src/olap/rowset/alpha_rowset_writer.cpp:275
   >#10 0x00ca3694 in doris::AlphaRowsetWriter::init (this=0x4bf83c0, 
rowset_writer_context=...)
   >at 
/home/wangbo/incubator-doris/be/src/olap/rowset/alpha_rowset_writer.cpp:73
   >#11 0x00c91b03 in doris::RowsetFactory::create_rowset_writer 
(context=..., output=output@entry=0x7fffcdf75318)
   >at /home/wangbo/incubator-doris/be/src/olap/rowset/rowset_factory.cpp:50
   >#12 0x00bc9b3f in doris::TabletManager::_create_inital_rowset 
(this=this@entry=0x4d5a180, tablet=..., request=...)
   >at /home/wangbo/incubator-doris/be/src/olap/tablet_manager.cpp:1250
   >#13 0x00bcc919 in doris::TabletManager::_internal_create_tablet 
(this=this@entry=0x4d5a180, alter_type=alter_type@entry=doris::SCHEMA_CHANGE, 
   >request=..., 
is_schema_change_tablet=is_schema_change_tablet@entry=false, ref_tablet=..., 
data_dirs=...)
   >at /home/wangbo/incubator-doris/be/src/olap/tablet_manager.cpp:371
   >#14 0x00bcdd72 in doris::TabletManager::create_tablet 
(this=this@entry=0x4d5a180, request=..., stores=...)
   >at /home/wangbo/incubator-doris/be/src/olap/tablet_manager.cpp:318
   >#15 0x00b82642 in doris::StorageEngine::create_tablet 
(this=, request=...)
   >at /home/wangbo/incubator-doris/be/src/olap/storage_engine.cpp:760
   >#16 0x00b4fac3 in doris::TestDeleteConditionHandler::SetUp 
(this=0x4e08460)
   >at /home/wangbo/incubator-doris/be/test/olap/delete_handler_test.cpp:173
   >#17 0x02530f37 in void 
testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) ()
   >#18 0x0252ba9c in void 
testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) ()
   >#19 0x02510df7 in testing::Test::Run() ()
   >#20 0x025116ce in testing::TestInfo::Run() ()
   >#21 0x02511d25 in testing::TestCase::Run() ()
   >#22 0x02518899 in testing::internal::UnitTestImpl::RunAllTests() ()
   >#23 0x02531eb3 in bool 
testing::internal::HandleSehExceptionsInMethodIfSupported(testing::internal::UnitTestImpl*, bool 
(testing::internal::UnitTestImpl::*)(), char const*) ()
   >#24 0x0252c7a2 in bool 
testing::internal::HandleExceptionsInMethodIfSupported(testing::internal::UnitTestImpl*, bool 
(testing::internal::UnitTestImpl::*)(), char const*) ()
   >#25 0x02517570 in testing::UnitTest::Run() ()
   >#26 0x009fb316 in RUN_ALL_TESTS () at 
/home/wangbo/incubator-doris/thirdparty/installed/include/gtest/gtest.h:2233
   >#27 main (argc=, argv=0x7fffcdf75f88) at 
/home/wangbo/incubator-doris/be/test/olap/delete_handler_test.cpp:951
   
   it seems that memset memory to 0xff may effect tcmalloc