You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2020/06/18 01:43:28 UTC

[GitHub] [incubator-doris] yangzhg opened a new pull request #3898: Support import true or false as boolean value

yangzhg opened a new pull request #3898:
URL: https://github.com/apache/incubator-doris/pull/3898


   Fixes #3831


----------------------------------------------------------------
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:
users@infra.apache.org



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


[GitHub] [incubator-doris] morningman merged pull request #3898: Support import true or false as boolean value

Posted by GitBox <gi...@apache.org>.
morningman merged pull request #3898:
URL: https://github.com/apache/incubator-doris/pull/3898


   


----------------------------------------------------------------
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:
users@infra.apache.org



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


[GitHub] [incubator-doris] wutiangan commented on a change in pull request #3898: Support import true or false as boolean value

Posted by GitBox <gi...@apache.org>.
wutiangan commented on a change in pull request #3898:
URL: https://github.com/apache/incubator-doris/pull/3898#discussion_r442788385



##########
File path: be/src/olap/utils.cpp
##########
@@ -1264,6 +1265,15 @@ bool valid_datetime(const string &value_str) {
     }
 }
 
+bool valid_bool(const std::string& value_str) {
+    if (value_str == "0" || value_str == "1") {
+        return true;
+    }
+    StringParser::ParseResult result;
+    StringParser::string_to_bool(value_str.c_str(), value_str.length(), &result);

Review comment:
       in BoolLiteral.BoolLiteral(String value) funciton, string "1" is  consider to true. but in string_to_bool_internal(), string "1"  is not consider to true, so it is puzzle. maybe you can unify 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:
users@infra.apache.org



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


[GitHub] [incubator-doris] caiconghui commented on pull request #3898: Support import true or false as boolean value

Posted by GitBox <gi...@apache.org>.
caiconghui commented on pull request #3898:
URL: https://github.com/apache/incubator-doris/pull/3898#issuecomment-646661966


   @yangzhg Hi, I think the result of process true false value  for stream load and insert stmt should keep   consistent. 
   For now, like insert into xx values(1), result=true, insert into xx values(0) result=false, insert into xxx values("false") result=true  insert into xx values(false) result=false  insert into xx values("0") result=true, insert into xx values(true) result=true. 
   This is because insert stmt can analyze the true data type for insert value, so like 1 2 3, will use int_cast_to_bool, even can analyze boolen type. but for stream load, we lost the data type information. we don't know the real data type of the value, we treat all column values as string, so I think may be stream load should be processed specially to keep consistent with insert stmt. maybe need more discussion.


----------------------------------------------------------------
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:
users@infra.apache.org



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


[GitHub] [incubator-doris] chaoyli commented on a change in pull request #3898: Support import true or false as boolean value

Posted by GitBox <gi...@apache.org>.
chaoyli commented on a change in pull request #3898:
URL: https://github.com/apache/incubator-doris/pull/3898#discussion_r442646702



##########
File path: be/src/olap/utils.cpp
##########
@@ -1264,6 +1265,15 @@ bool valid_datetime(const string &value_str) {
     }
 }
 
+bool valid_bool(const std::string& value_str) {
+    if (value_str == "0" || value_str == "1") {
+        return true;
+    }
+    StringParser::ParseResult result;
+    StringParser::string_to_bool(value_str.c_str(), value_str.length(), &result);

Review comment:
       If "true " will be ok ? @yangzhg 




----------------------------------------------------------------
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:
users@infra.apache.org



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


[GitHub] [incubator-doris] yangzhg edited a comment on pull request #3898: Support import true or false as boolean value

Posted by GitBox <gi...@apache.org>.
yangzhg edited a comment on pull request #3898:
URL: https://github.com/apache/incubator-doris/pull/3898#issuecomment-647270265


   > @yangzhg Hi, I think the result of processing bool value for stream load and insert stmt should keep consistent.
   > For now, like insert into xx values(1), result=true, insert into xx values(0) result=false, insert into xxx values("false") result=true insert into xx values(false) result=false insert into xx values("0") result=true, insert into xx values(true) result=true.
   > This is because insert stmt can analyze the real data type for insert value, so like 1 2 3, will use int_cast_to_bool, even can analyze boolean type. but for stream load, we lost the data type information. we don't know the real data type of the value, we treat all column values as string, so I think may be stream load should be processed specially to keep consistent with insert stmt. maybe need more discussion.
   
   @chaoyli  Insert into xx values("0") and stream load import file 0 is not consistent in the current code. Insert into xx values("0") result=true, stream load 0 result=false, and now cast_to_bool Only occurs when the type of the column is bool, so it is not contradictory to the current insert


----------------------------------------------------------------
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:
users@infra.apache.org



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


[GitHub] [incubator-doris] yangzhg commented on a change in pull request #3898: Support import true or false as boolean value

Posted by GitBox <gi...@apache.org>.
yangzhg commented on a change in pull request #3898:
URL: https://github.com/apache/incubator-doris/pull/3898#discussion_r442160830



##########
File path: be/src/exprs/cast_functions.cpp
##########
@@ -229,6 +229,17 @@ StringVal CastFunctions::cast_to_string_val(FunctionContext* ctx, const StringVa
   return sv;
 }
 
+BooleanVal CastFunctions::cast_to_boolean_val(FunctionContext* ctx, const StringVal& val) {
+    if (val.is_null) return BooleanVal::null(); 
+        StringParser::ParseResult result;
+        BooleanVal ret;
+        ret.val = StringParser::string_to_bool(reinterpret_cast<char*>(val.ptr), val.len, &result);
+        if (UNLIKELY(result != StringParser::PARSE_SUCCESS || std::isnan(ret.val) || std::isinf(ret.val))) { 
+            return BooleanVal::null();
+		}

Review comment:
       Please review it again @chaoyli 




----------------------------------------------------------------
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:
users@infra.apache.org



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


[GitHub] [incubator-doris] yangzhg commented on pull request #3898: Support import true or false as boolean value

Posted by GitBox <gi...@apache.org>.
yangzhg commented on pull request #3898:
URL: https://github.com/apache/incubator-doris/pull/3898#issuecomment-647270265


   > @yangzhg Hi, I think the result of processing bool value for stream load and insert stmt should keep consistent.
   > For now, like insert into xx values(1), result=true, insert into xx values(0) result=false, insert into xxx values("false") result=true insert into xx values(false) result=false insert into xx values("0") result=true, insert into xx values(true) result=true.
   > This is because insert stmt can analyze the real data type for insert value, so like 1 2 3, will use int_cast_to_bool, even can analyze boolean type. but for stream load, we lost the data type information. we don't know the real data type of the value, we treat all column values as string, so I think may be stream load should be processed specially to keep consistent with insert stmt. maybe need more discussion.
   
   
   
   > @yangzhg Hi, I think the result of processing bool value for stream load and insert stmt should keep consistent.
   > For now, like insert into xx values(1), result=true, insert into xx values(0) result=false, insert into xxx values("false") result=true insert into xx values(false) result=false insert into xx values("0") result=true, insert into xx values(true) result=true.
   > This is because insert stmt can analyze the real data type for insert value, so like 1 2 3, will use int_cast_to_bool, even can analyze boolean type. but for stream load, we lost the data type information. we don't know the real data type of the value, we treat all column values as string, so I think may be stream load should be processed specially to keep consistent with insert stmt. maybe need more discussion.
   
   insert into xx values("0") and stream load import file 0 result itself is contradictory, in the current code insert into xx values("0") result=true, stream load 0 result=false, and now cast_to_bool Only occurs when the type of the column is bool, so it is not contradictory to the current insert


----------------------------------------------------------------
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:
users@infra.apache.org



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


[GitHub] [incubator-doris] yangzhg edited a comment on pull request #3898: Support import true or false as boolean value

Posted by GitBox <gi...@apache.org>.
yangzhg edited a comment on pull request #3898:
URL: https://github.com/apache/incubator-doris/pull/3898#issuecomment-648547789


   > @yangzhg Hi, I think the result of processing bool value for stream load and insert stmt should keep consistent.
   > For now, like insert into xx values(1), result=true, insert into xx values(0) result=false, insert into xxx values("false") result=true insert into xx values(false) result=false insert into xx values("0") result=true, insert into xx values(true) result=true.
   > This is because insert stmt can analyze the real data type for insert value, so like 1 2 3, will use int_cast_to_bool, even can analyze boolean type. but for stream load, we lost the data type information. we don't know the real data type of the value, we treat all column values as string, so I think may be stream load should be processed specially to keep consistent with insert stmt. maybe need more discussion.
   
   @caiconghui I will change the insert statement  and stream load to keep consistent with mysql


----------------------------------------------------------------
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:
users@infra.apache.org



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


[GitHub] [incubator-doris] wutiangan commented on a change in pull request #3898: Support import true or false as boolean value

Posted by GitBox <gi...@apache.org>.
wutiangan commented on a change in pull request #3898:
URL: https://github.com/apache/incubator-doris/pull/3898#discussion_r442788385



##########
File path: be/src/olap/utils.cpp
##########
@@ -1264,6 +1265,15 @@ bool valid_datetime(const string &value_str) {
     }
 }
 
+bool valid_bool(const std::string& value_str) {
+    if (value_str == "0" || value_str == "1") {
+        return true;
+    }
+    StringParser::ParseResult result;
+    StringParser::string_to_bool(value_str.c_str(), value_str.length(), &result);

Review comment:
       in BoolLiteral.BoolLiteral(String value) funciton, string "1" is  consider to true. but in string_to_bool_internal(), string "1"  is not consider to true, so it is puzzle. maybe you can unify 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:
users@infra.apache.org



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


[GitHub] [incubator-doris] yangzhg commented on pull request #3898: Support import true or false as boolean value

Posted by GitBox <gi...@apache.org>.
yangzhg commented on pull request #3898:
URL: https://github.com/apache/incubator-doris/pull/3898#issuecomment-648547789


   > @yangzhg Hi, I think the result of processing bool value for stream load and insert stmt should keep consistent.
   > For now, like insert into xx values(1), result=true, insert into xx values(0) result=false, insert into xxx values("false") result=true insert into xx values(false) result=false insert into xx values("0") result=true, insert into xx values(true) result=true.
   > This is because insert stmt can analyze the real data type for insert value, so like 1 2 3, will use int_cast_to_bool, even can analyze boolean type. but for stream load, we lost the data type information. we don't know the real data type of the value, we treat all column values as string, so I think may be stream load should be processed specially to keep consistent with insert stmt. maybe need more discussion.
   
   I will change the insert statement  and stream load to keep consistent with mysql


----------------------------------------------------------------
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:
users@infra.apache.org



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


[GitHub] [incubator-doris] yangzhg edited a comment on pull request #3898: Support import true or false as boolean value

Posted by GitBox <gi...@apache.org>.
yangzhg edited a comment on pull request #3898:
URL: https://github.com/apache/incubator-doris/pull/3898#issuecomment-647270265


   > @yangzhg Hi, I think the result of processing bool value for stream load and insert stmt should keep consistent.
   > For now, like insert into xx values(1), result=true, insert into xx values(0) result=false, insert into xxx values("false") result=true insert into xx values(false) result=false insert into xx values("0") result=true, insert into xx values(true) result=true.
   > This is because insert stmt can analyze the real data type for insert value, so like 1 2 3, will use int_cast_to_bool, even can analyze boolean type. but for stream load, we lost the data type information. we don't know the real data type of the value, we treat all column values as string, so I think may be stream load should be processed specially to keep consistent with insert stmt. maybe need more discussion.
   
   
   
   > @yangzhg Hi, I think the result of processing bool value for stream load and insert stmt should keep consistent.
   > For now, like insert into xx values(1), result=true, insert into xx values(0) result=false, insert into xxx values("false") result=true insert into xx values(false) result=false insert into xx values("0") result=true, insert into xx values(true) result=true.
   > This is because insert stmt can analyze the real data type for insert value, so like 1 2 3, will use int_cast_to_bool, even can analyze boolean type. but for stream load, we lost the data type information. we don't know the real data type of the value, we treat all column values as string, so I think may be stream load should be processed specially to keep consistent with insert stmt. maybe need more discussion.
   
   insert into xx values("0") and stream load import file 0 is not consistent in the current code. Insert into xx values("0") result=true, stream load 0 result=false, and now cast_to_bool Only occurs when the type of the column is bool, so it is not contradictory to the current insert


----------------------------------------------------------------
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:
users@infra.apache.org



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


[GitHub] [incubator-doris] yangzhg commented on a change in pull request #3898: Support import true or false as boolean value

Posted by GitBox <gi...@apache.org>.
yangzhg commented on a change in pull request #3898:
URL: https://github.com/apache/incubator-doris/pull/3898#discussion_r442805904



##########
File path: be/src/exprs/cast_functions.cpp
##########
@@ -229,6 +229,17 @@ StringVal CastFunctions::cast_to_string_val(FunctionContext* ctx, const StringVa
   return sv;
 }
 
+BooleanVal CastFunctions::cast_to_boolean_val(FunctionContext* ctx, const StringVal& val) {
+    if (val.is_null) return BooleanVal::null(); 
+        StringParser::ParseResult result;
+        BooleanVal ret;
+        ret.val = StringParser::string_to_bool(reinterpret_cast<char*>(val.ptr), val.len, &result);

Review comment:
       "1" will convert to int  not bool 




----------------------------------------------------------------
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:
users@infra.apache.org



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


[GitHub] [incubator-doris] wutiangan commented on a change in pull request #3898: Support import true or false as boolean value

Posted by GitBox <gi...@apache.org>.
wutiangan commented on a change in pull request #3898:
URL: https://github.com/apache/incubator-doris/pull/3898#discussion_r442791890



##########
File path: be/src/exprs/cast_functions.cpp
##########
@@ -229,6 +229,17 @@ StringVal CastFunctions::cast_to_string_val(FunctionContext* ctx, const StringVa
   return sv;
 }
 
+BooleanVal CastFunctions::cast_to_boolean_val(FunctionContext* ctx, const StringVal& val) {
+    if (val.is_null) return BooleanVal::null(); 
+        StringParser::ParseResult result;
+        BooleanVal ret;
+        ret.val = StringParser::string_to_bool(reinterpret_cast<char*>(val.ptr), val.len, &result);

Review comment:
       StringParser::string_to_bool will consider "1" to false, is it ok?




----------------------------------------------------------------
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:
users@infra.apache.org



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


[GitHub] [incubator-doris] chaoyli commented on a change in pull request #3898: Support import true or false as boolean value

Posted by GitBox <gi...@apache.org>.
chaoyli commented on a change in pull request #3898:
URL: https://github.com/apache/incubator-doris/pull/3898#discussion_r442097496



##########
File path: be/src/olap/utils.cpp
##########
@@ -1264,6 +1265,15 @@ bool valid_datetime(const string &value_str) {
     }
 }
 
+bool valid_bool(const std::string& value_str) {
+    if (value_str == "0" || value_str == "1") {
+        return true;
+    }
+    StringParser::ParseResult result;
+    StringParser::string_to_bool(value_str.c_str(), value_str.length(), &result);

Review comment:
       I found string_to_bool will convert "Truetest" to be true, it is ok?
   ```
   543 inline bool StringParser::string_to_bool_internal(const char* s, int len, ParseResult* result) {
   544     *result = PARSE_SUCCESS;
   545
   546     if (len >= 4 && (s[0] == 't' || s[0] == 'T')) {
   547         bool match = (s[1] == 'r' || s[1] == 'R') &&
   548             (s[2] == 'u' || s[2] == 'U') &&
   549             (s[3] == 'e' || s[3] == 'E');
   550         if (match && LIKELY(is_all_whitespace(s + 4, len - 4))) {
   551             return true;
   552         }
   553     } else if (len >= 5 && (s[0] == 'f' || s[0] == 'F')) {
   554         bool match = (s[1] == 'a' || s[1] == 'A') &&
   555             (s[2] == 'l' || s[2] == 'L') &&
   556             (s[3] == 's' || s[3] == 'S') &&
   557             (s[4] == 'e' || s[4] == 'E');
   558         if (match && LIKELY(is_all_whitespace(s + 5, len - 5))){
   559             return false;
   560         }
   561     }
   ```

##########
File path: be/src/exprs/cast_functions.cpp
##########
@@ -229,6 +229,17 @@ StringVal CastFunctions::cast_to_string_val(FunctionContext* ctx, const StringVa
   return sv;
 }
 
+BooleanVal CastFunctions::cast_to_boolean_val(FunctionContext* ctx, const StringVal& val) {
+    if (val.is_null) return BooleanVal::null(); 
+        StringParser::ParseResult result;
+        BooleanVal ret;
+        ret.val = StringParser::string_to_bool(reinterpret_cast<char*>(val.ptr), val.len, &result);
+        if (UNLIKELY(result != StringParser::PARSE_SUCCESS || std::isnan(ret.val) || std::isinf(ret.val))) { 
+            return BooleanVal::null();
+		}

Review comment:
       Brackets is not aligned.




----------------------------------------------------------------
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:
users@infra.apache.org



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


[GitHub] [incubator-doris] wutiangan commented on a change in pull request #3898: Support import true or false as boolean value

Posted by GitBox <gi...@apache.org>.
wutiangan commented on a change in pull request #3898:
URL: https://github.com/apache/incubator-doris/pull/3898#discussion_r442784153



##########
File path: be/src/exprs/cast_functions.cpp
##########
@@ -229,6 +229,17 @@ StringVal CastFunctions::cast_to_string_val(FunctionContext* ctx, const StringVa
   return sv;
 }
 
+BooleanVal CastFunctions::cast_to_boolean_val(FunctionContext* ctx, const StringVal& val) {
+    if (val.is_null) return BooleanVal::null(); 
+        StringParser::ParseResult result;

Review comment:
       modify the indent




----------------------------------------------------------------
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:
users@infra.apache.org



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


[GitHub] [incubator-doris] wutiangan commented on a change in pull request #3898: Support import true or false as boolean value

Posted by GitBox <gi...@apache.org>.
wutiangan commented on a change in pull request #3898:
URL: https://github.com/apache/incubator-doris/pull/3898#discussion_r442790495



##########
File path: be/src/exprs/cast_functions.cpp
##########
@@ -229,6 +229,17 @@ StringVal CastFunctions::cast_to_string_val(FunctionContext* ctx, const StringVa
   return sv;
 }
 
+BooleanVal CastFunctions::cast_to_boolean_val(FunctionContext* ctx, const StringVal& val) {
+    if (val.is_null) return BooleanVal::null(); 
+        StringParser::ParseResult result;
+        BooleanVal ret;
+        ret.val = StringParser::string_to_bool(reinterpret_cast<char*>(val.ptr), val.len, &result);
+        if (UNLIKELY(result != StringParser::PARSE_SUCCESS || std::isnan(ret.val) || std::isinf(ret.val))) { 

Review comment:
       why you add the "std::isnan(ret.val) || std::isinf(ret.val)"?if result == StringParser::PARSE_SUCCESS, can val be nan or inf ?
   




----------------------------------------------------------------
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:
users@infra.apache.org



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


[GitHub] [incubator-doris] yangzhg commented on a change in pull request #3898: Support import true or false as boolean value

Posted by GitBox <gi...@apache.org>.
yangzhg commented on a change in pull request #3898:
URL: https://github.com/apache/incubator-doris/pull/3898#discussion_r442160165



##########
File path: be/src/olap/utils.cpp
##########
@@ -1264,6 +1265,15 @@ bool valid_datetime(const string &value_str) {
     }
 }
 
+bool valid_bool(const std::string& value_str) {
+    if (value_str == "0" || value_str == "1") {
+        return true;
+    }
+    StringParser::ParseResult result;
+    StringParser::string_to_bool(value_str.c_str(), value_str.length(), &result);

Review comment:
       "Truetest" won't be converted to `true`  because of `is_all_whitespace()`, it will return  `PARSE_FAILURE`. In additional, this is just test the string is `true` or `false` ignore case;




----------------------------------------------------------------
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:
users@infra.apache.org



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


[GitHub] [incubator-doris] caiconghui edited a comment on pull request #3898: Support import true or false as boolean value

Posted by GitBox <gi...@apache.org>.
caiconghui edited a comment on pull request #3898:
URL: https://github.com/apache/incubator-doris/pull/3898#issuecomment-646661966


   @yangzhg Hi, I think the result of processing bool value  for stream load and insert stmt should keep   consistent. 
   For now, like insert into xx values(1), result=true, insert into xx values(0) result=false, insert into xxx values("false") result=true  insert into xx values(false) result=false  insert into xx values("0") result=true, insert into xx values(true) result=true. 
   This is because insert stmt can analyze the true data type for insert value, so like 1 2 3, will use int_cast_to_bool, even can analyze boolen type. but for stream load, we lost the data type information. we don't know the real data type of the value, we treat all column values as string, so I think may be stream load should be processed specially to keep consistent with insert stmt. maybe need more discussion.


----------------------------------------------------------------
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:
users@infra.apache.org



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


[GitHub] [incubator-doris] chaoyli edited a comment on pull request #3898: Support import true or false as boolean value

Posted by GitBox <gi...@apache.org>.
chaoyli edited a comment on pull request #3898:
URL: https://github.com/apache/incubator-doris/pull/3898#issuecomment-648537122


   I think the meaning of caiconghui is to make consistent of stream load and insert in any situation.
   Otherwise, it will cause some misunderstanding. @yangzhg 


----------------------------------------------------------------
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:
users@infra.apache.org



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


[GitHub] [incubator-doris] chaoyli commented on pull request #3898: Support import true or false as boolean value

Posted by GitBox <gi...@apache.org>.
chaoyli commented on pull request #3898:
URL: https://github.com/apache/incubator-doris/pull/3898#issuecomment-648537122


   I think the meaning of caiconghui is to make consistent of stream load and insert in any situation.
   Otherwise, it will cause some misunderstanding.


----------------------------------------------------------------
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:
users@infra.apache.org



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


[GitHub] [incubator-doris] caiconghui edited a comment on pull request #3898: Support import true or false as boolean value

Posted by GitBox <gi...@apache.org>.
caiconghui edited a comment on pull request #3898:
URL: https://github.com/apache/incubator-doris/pull/3898#issuecomment-646661966


   @yangzhg Hi, I think the result of processing bool value  for stream load and insert stmt should keep   consistent. 
   For now, like insert into xx values(1), result=true, insert into xx values(0) result=false, insert into xxx values("false") result=true  insert into xx values(false) result=false  insert into xx values("0") result=true, insert into xx values(true) result=true. 
   This is because insert stmt can analyze the real data type for insert value, so like 1 2 3, will use int_cast_to_bool, even can analyze boolean type. but for stream load, we lost the data type information. we don't know the real data type of the value, we treat all column values as string, so I think may be stream load should be processed specially to keep consistent with insert stmt. maybe need more discussion.


----------------------------------------------------------------
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:
users@infra.apache.org



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