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/04/29 16:42:04 UTC
[GitHub] [incubator-doris] Youngwb opened a new pull request #3439: Support append_trailing_char_if_absent function
Youngwb opened a new pull request #3439:
URL: https://github.com/apache/incubator-doris/pull/3439
support append_trailing_char_if_absent function
#2598
----------------------------------------------------------------
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] imay commented on a change in pull request #3439: Support append_trailing_char_if_absent function
Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #3439:
URL: https://github.com/apache/incubator-doris/pull/3439#discussion_r417862497
##########
File path: gensrc/script/doris_builtins_functions.py
##########
@@ -531,6 +531,8 @@
[['rpad'], 'VARCHAR', ['VARCHAR', 'INT', 'VARCHAR'],
'_ZN5doris15StringFunctions4rpadEPN9doris_udf'
'15FunctionContextERKNS1_9StringValERKNS1_6IntValES6_'],
+ [['append_trailing_char_if_absent'], 'VARCHAR', ['VARCHAR', 'VARCHAR'],
Review comment:
Could you tell me in what scenario this function is used?
##########
File path: docs/documentation/cn/sql-reference/sql-functions/string-functions/append_trailing_char_if_absent.md
##########
@@ -0,0 +1,46 @@
+<!--
Review comment:
Should change this documents according to the new document framework.
##########
File path: docs/documentation/cn/sql-reference/sql-functions/string-functions/append_trailing_char_if_absent.md
##########
@@ -0,0 +1,46 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+ http://www.apache.org/licenses/LICENSE-2.0
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+# append_trailing_char_if_absent
+## description
+### Syntax
+
+`VARCHAR append_trailing_char_if_absent(VARCHAR str, VARCHAR trailing_char)`
+
+如果's'字符串非空并且末尾不包含'c'字符，则将'c'字符附加到末尾
Review comment:
Should explain when input trailing_char has multiple chars, how will this function act.
----------------------------------------------------------------
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] Youngwb commented on a change in pull request #3439: Support append_trailing_char_if_absent function
Posted by GitBox <gi...@apache.org>.
Youngwb commented on a change in pull request #3439:
URL: https://github.com/apache/incubator-doris/pull/3439#discussion_r420638334
##########
File path: be/test/exprs/string_functions_test.cpp
##########
@@ -252,6 +264,26 @@ TEST_F(StringFunctionsTest, null_or_empty) {
delete context;
}
+TEST_F(StringFunctionsTest, append_trailing_char_if_absent) {
+ ASSERT_EQ(StringVal("ac"), StringFunctions::append_trailing_char_if_absent(ctx,
+ StringVal("a"), StringVal("c")));
+
+ ASSERT_EQ(StringVal("c"), StringFunctions::append_trailing_char_if_absent(ctx,
+ StringVal("c"), StringVal("c")));
+
+ ASSERT_EQ(StringVal("123c"), StringFunctions::append_trailing_char_if_absent(ctx,
+ StringVal("123c"), StringVal("c")));
+
+ ASSERT_EQ(StringVal("c"), StringFunctions::append_trailing_char_if_absent(ctx,
+ StringVal(""), StringVal("c")));
+
+ ASSERT_EQ(StringVal::null(), StringFunctions::append_trailing_char_if_absent(ctx,
+ StringVal::null(), StringVal("c")));
+
+ ASSERT_EQ(StringVal::null(), StringFunctions::append_trailing_char_if_absent(ctx,
+ StringVal("a"), StringVal::null()));
Review comment:
ok，Done
----------------------------------------------------------------
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 commented on pull request #3439: Support append_trailing_char_if_absent function
Posted by GitBox <gi...@apache.org>.
morningman commented on pull request #3439:
URL: https://github.com/apache/incubator-doris/pull/3439#issuecomment-625851825
UT passed. LGTM
----------------------------------------------------------------
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] imay commented on a change in pull request #3439: Support append_trailing_char_if_absent function
Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #3439:
URL: https://github.com/apache/incubator-doris/pull/3439#discussion_r420615579
##########
File path: fe/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java
##########
@@ -605,6 +605,19 @@ public void analyzeImpl(Analyzer analyzer) throws AnalysisException {
return;
}
+ if (fnName.getFunction().equalsIgnoreCase("append_trailing_char_if_absent")) {
Review comment:
I think this check is unnecessary, can be removed.
If you keep it, this will make this function more complicate.
##########
File path: be/src/exprs/string_functions.cpp
##########
@@ -212,6 +212,28 @@ StringVal StringFunctions::rpad(
}
return result;
}
+
+StringVal StringFunctions::append_trailing_char_if_absent(doris_udf::FunctionContext* context,
+ const doris_udf::StringVal& str, const doris_udf::StringVal& trailing_char) {
+ if (str.is_null || trailing_char.is_null) {
+ return StringVal::null();
+ }
+ if (str.len == 0) {
+ return trailing_char;
+ }
+ if (str.ptr[str.len-1] == trailing_char.ptr[0]) {
+ return str;
+ }
+
+ StringVal result(context, str.len + trailing_char.len);
+ if (UNLIKELY(result.is_null)) {
Review comment:
I agree with kangkaisen, this check can be removed
##########
File path: be/src/exprs/string_functions.cpp
##########
@@ -212,6 +212,28 @@ StringVal StringFunctions::rpad(
}
return result;
}
+
+StringVal StringFunctions::append_trailing_char_if_absent(doris_udf::FunctionContext* context,
+ const doris_udf::StringVal& str, const doris_udf::StringVal& trailing_char) {
+ if (str.is_null || trailing_char.is_null || trailing_char.len != 1) {
+ return StringVal::null();
+ }
+ if (str.len == 0) {
+ return trailing_char;
+ }
+ if (str.ptr[str.len-1] == trailing_char.ptr[0]) {
Review comment:
```suggestion
if (str.ptr[str.len - 1] == trailing_char.ptr[0]) {
```
##########
File path: be/test/exprs/string_functions_test.cpp
##########
@@ -252,6 +264,26 @@ TEST_F(StringFunctionsTest, null_or_empty) {
delete context;
}
+TEST_F(StringFunctionsTest, append_trailing_char_if_absent) {
+ ASSERT_EQ(StringVal("ac"), StringFunctions::append_trailing_char_if_absent(ctx,
+ StringVal("a"), StringVal("c")));
+
+ ASSERT_EQ(StringVal("c"), StringFunctions::append_trailing_char_if_absent(ctx,
+ StringVal("c"), StringVal("c")));
+
+ ASSERT_EQ(StringVal("123c"), StringFunctions::append_trailing_char_if_absent(ctx,
+ StringVal("123c"), StringVal("c")));
+
+ ASSERT_EQ(StringVal("c"), StringFunctions::append_trailing_char_if_absent(ctx,
+ StringVal(""), StringVal("c")));
+
+ ASSERT_EQ(StringVal::null(), StringFunctions::append_trailing_char_if_absent(ctx,
+ StringVal::null(), StringVal("c")));
+
+ ASSERT_EQ(StringVal::null(), StringFunctions::append_trailing_char_if_absent(ctx,
+ StringVal("a"), StringVal::null()));
Review comment:
Should add test that second argument has multiple characters.
##########
File path: be/src/exprs/string_functions.cpp
##########
@@ -212,6 +212,28 @@ StringVal StringFunctions::rpad(
}
return result;
}
+
+StringVal StringFunctions::append_trailing_char_if_absent(doris_udf::FunctionContext* context,
+ const doris_udf::StringVal& str, const doris_udf::StringVal& trailing_char) {
+ if (str.is_null || trailing_char.is_null || trailing_char.len != 1) {
+ return StringVal::null();
+ }
+ if (str.len == 0) {
+ return trailing_char;
+ }
+ if (str.ptr[str.len-1] == trailing_char.ptr[0]) {
+ return str;
+ }
+
+ StringVal result(context, str.len + trailing_char.len);
Review comment:
```suggestion
StringVal result(context, str.len + 1);
```
----------------------------------------------------------------
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] imay commented on a change in pull request #3439: Support append_trailing_char_if_absent function
Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #3439:
URL: https://github.com/apache/incubator-doris/pull/3439#discussion_r421940960
##########
File path: docs/en/sql-reference/sql-functions/string-functions/append_trailing_char_if_absent.md
##########
@@ -0,0 +1,58 @@
+---
+{
+ "title": "append_trailing_char_if_absent",
+ "language": "en"
+}
+---
+
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+# append_trailing_char_if_absent
+## description
+### Syntax
Review comment:
split these lines with empty line
----------------------------------------------------------------
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] Youngwb commented on a change in pull request #3439: Support append_trailing_char_if_absent function
Posted by GitBox <gi...@apache.org>.
Youngwb commented on a change in pull request #3439:
URL: https://github.com/apache/incubator-doris/pull/3439#discussion_r417922171
##########
File path: docs/documentation/cn/sql-reference/sql-functions/string-functions/append_trailing_char_if_absent.md
##########
@@ -0,0 +1,46 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+ http://www.apache.org/licenses/LICENSE-2.0
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+# append_trailing_char_if_absent
+## description
+### Syntax
+
+`VARCHAR append_trailing_char_if_absent(VARCHAR str, VARCHAR trailing_char)`
+
+如果's'字符串非空并且末尾不包含'c'字符，则将'c'字符附加到末尾
Review comment:
Done
----------------------------------------------------------------
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] imay commented on a change in pull request #3439: Support append_trailing_char_if_absent function
Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #3439:
URL: https://github.com/apache/incubator-doris/pull/3439#discussion_r420145099
##########
File path: docs/en/sql-reference/sql-functions/string-functions/append_trailing_char_if_absent.md
##########
@@ -0,0 +1,58 @@
+---
+{
+ "title": "append_trailing_char_if_absent",
+ "language": "en"
+}
+---
+
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+# append_trailing_char_if_absent
+## description
+### Syntax
+
+`VARCHAR append_trailing_char_if_absent(VARCHAR str, VARCHAR trailing_char)`
+
+If the s string is non-empty and does not contain the c character at the end, it appends the c character to the end.
+Trailing_char contains only one character, and it will report an error if contains more than one character
Review comment:
In the case `append_trailing_char_if_absent(str, col1)` in which col1 is a column of a table. You can not know if the length of col1 is 1 or not.
Also there is no need to keep the same with CK.
It is convention to return null when the argument is invalid in Doris.
So what I suggest is to return null when the input argument is not valid. And there is no need to check its input in FE.
----------------------------------------------------------------
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] Youngwb commented on a change in pull request #3439: Support append_trailing_char_if_absent function
Posted by GitBox <gi...@apache.org>.
Youngwb commented on a change in pull request #3439:
URL: https://github.com/apache/incubator-doris/pull/3439#discussion_r417923729
##########
File path: gensrc/script/doris_builtins_functions.py
##########
@@ -531,6 +531,8 @@
[['rpad'], 'VARCHAR', ['VARCHAR', 'INT', 'VARCHAR'],
'_ZN5doris15StringFunctions4rpadEPN9doris_udf'
'15FunctionContextERKNS1_9StringValERKNS1_6IntValES6_'],
+ [['append_trailing_char_if_absent'], 'VARCHAR', ['VARCHAR', 'VARCHAR'],
Review comment:
may be such like this :
```
MySQL [test]> select append_trailing_char_if_absent('http://google.com','/');
+----------------------------------------------------------+
| append_trailing_char_if_absent('http://google.com', '/') |
+----------------------------------------------------------+
| http://google.com/ |
+----------------------------------------------------------+
1 row in set (0.01 sec)
MySQL [test]> select append_trailing_char_if_absent('http://google.com/','/');
+-----------------------------------------------------------+
| append_trailing_char_if_absent('http://google.com/', '/') |
+-----------------------------------------------------------+
| http://google.com/ |
+-----------------------------------------------------------+
```
----------------------------------------------------------------
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] Youngwb commented on a change in pull request #3439: Support append_trailing_char_if_absent function
Posted by GitBox <gi...@apache.org>.
Youngwb commented on a change in pull request #3439:
URL: https://github.com/apache/incubator-doris/pull/3439#discussion_r420095253
##########
File path: docs/en/sql-reference/sql-functions/string-functions/append_trailing_char_if_absent.md
##########
@@ -0,0 +1,58 @@
+---
+{
+ "title": "append_trailing_char_if_absent",
+ "language": "en"
+}
+---
+
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+# append_trailing_char_if_absent
+## description
+### Syntax
+
+`VARCHAR append_trailing_char_if_absent(VARCHAR str, VARCHAR trailing_char)`
+
+If the s string is non-empty and does not contain the c character at the end, it appends the c character to the end.
+Trailing_char contains only one character, and it will report an error if contains more than one character
Review comment:
Because it's function name contains `trailing_char`, and this is also consistent with the clickhose function. I did a cast in fe `if (((StringLiteral) arg1).getValue().length() != 1) `, It will throw ClassCastException when SlotRef cast to StringLiteral. I can also add type checking if necessary
----------------------------------------------------------------
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] Youngwb commented on a change in pull request #3439: Support append_trailing_char_if_absent function
Posted by GitBox <gi...@apache.org>.
Youngwb commented on a change in pull request #3439:
URL: https://github.com/apache/incubator-doris/pull/3439#discussion_r420625860
##########
File path: fe/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java
##########
@@ -605,6 +605,19 @@ public void analyzeImpl(Analyzer analyzer) throws AnalysisException {
return;
}
+ if (fnName.getFunction().equalsIgnoreCase("append_trailing_char_if_absent")) {
Review comment:
Yes，I understand there will make this function more complicate, but I need to avoid two parameters be Non-String type，such as `append_trailing_char_if_absent(1,2)`, and I don't known how to handle this case if i don't check here. Maybe there is a better way to handle this case if you known.
----------------------------------------------------------------
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] kangkaisen commented on a change in pull request #3439: Support append_trailing_char_if_absent function
Posted by GitBox <gi...@apache.org>.
kangkaisen commented on a change in pull request #3439:
URL: https://github.com/apache/incubator-doris/pull/3439#discussion_r417732926
##########
File path: be/src/exprs/string_functions.cpp
##########
@@ -212,6 +212,28 @@ StringVal StringFunctions::rpad(
}
return result;
}
+
+StringVal StringFunctions::append_trailing_char_if_absent(doris_udf::FunctionContext* context,
+ const doris_udf::StringVal& str, const doris_udf::StringVal& trailing_char) {
+ if (str.is_null || trailing_char.is_null) {
+ return StringVal::null();
+ }
+ if (str.len == 0) {
+ return trailing_char;
+ }
+ if (str.ptr[str.len-1] == trailing_char.ptr[0]) {
+ return str;
+ }
+
+ StringVal result(context, str.len + trailing_char.len);
+ if (UNLIKELY(result.is_null)) {
Review comment:
Why new result could null?
----------------------------------------------------------------
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] imay commented on a change in pull request #3439: Support append_trailing_char_if_absent function
Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #3439:
URL: https://github.com/apache/incubator-doris/pull/3439#discussion_r420628143
##########
File path: fe/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java
##########
@@ -605,6 +605,19 @@ public void analyzeImpl(Analyzer analyzer) throws AnalysisException {
return;
}
+ if (fnName.getFunction().equalsIgnoreCase("append_trailing_char_if_absent")) {
Review comment:
For this case you mentioned, Doris can cast input argument to function's definition type. So you don't handle this in this place.
----------------------------------------------------------------
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] Youngwb commented on a change in pull request #3439: Support append_trailing_char_if_absent function
Posted by GitBox <gi...@apache.org>.
Youngwb commented on a change in pull request #3439:
URL: https://github.com/apache/incubator-doris/pull/3439#discussion_r417778746
##########
File path: be/src/exprs/string_functions.cpp
##########
@@ -212,6 +212,28 @@ StringVal StringFunctions::rpad(
}
return result;
}
+
+StringVal StringFunctions::append_trailing_char_if_absent(doris_udf::FunctionContext* context,
+ const doris_udf::StringVal& str, const doris_udf::StringVal& trailing_char) {
+ if (str.is_null || trailing_char.is_null) {
+ return StringVal::null();
+ }
+ if (str.len == 0) {
+ return trailing_char;
+ }
+ if (str.ptr[str.len-1] == trailing_char.ptr[0]) {
+ return str;
+ }
+
+ StringVal result(context, str.len + trailing_char.len);
+ if (UNLIKELY(result.is_null)) {
Review comment:
StringVal will alloc memory by context's FreePool, that may be null. I use this check according to other string_functions. But this check `is_null` may be useless, It can be removed
----------------------------------------------------------------
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] kangkaisen commented on a change in pull request #3439: Support append_trailing_char_if_absent function
Posted by GitBox <gi...@apache.org>.
kangkaisen commented on a change in pull request #3439:
URL: https://github.com/apache/incubator-doris/pull/3439#discussion_r418492786
##########
File path: be/src/exprs/string_functions.cpp
##########
@@ -212,6 +212,28 @@ StringVal StringFunctions::rpad(
}
return result;
}
+
+StringVal StringFunctions::append_trailing_char_if_absent(doris_udf::FunctionContext* context,
+ const doris_udf::StringVal& str, const doris_udf::StringVal& trailing_char) {
+ if (str.is_null || trailing_char.is_null) {
+ return StringVal::null();
+ }
+ if (str.len == 0) {
+ return trailing_char;
+ }
+ if (str.ptr[str.len-1] == trailing_char.ptr[0]) {
+ return str;
+ }
+
+ StringVal result(context, str.len + trailing_char.len);
+ if (UNLIKELY(result.is_null)) {
Review comment:
In fact, FreePool allocate never return null.
So `result.is_null` check is unnecessary.
----------------------------------------------------------------
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] Youngwb commented on a change in pull request #3439: Support append_trailing_char_if_absent function
Posted by GitBox <gi...@apache.org>.
Youngwb commented on a change in pull request #3439:
URL: https://github.com/apache/incubator-doris/pull/3439#discussion_r420653086
##########
File path: be/src/exprs/string_functions.cpp
##########
@@ -212,6 +212,28 @@ StringVal StringFunctions::rpad(
}
return result;
}
+
+StringVal StringFunctions::append_trailing_char_if_absent(doris_udf::FunctionContext* context,
+ const doris_udf::StringVal& str, const doris_udf::StringVal& trailing_char) {
+ if (str.is_null || trailing_char.is_null) {
+ return StringVal::null();
+ }
+ if (str.len == 0) {
+ return trailing_char;
+ }
+ if (str.ptr[str.len-1] == trailing_char.ptr[0]) {
+ return str;
+ }
+
+ StringVal result(context, str.len + trailing_char.len);
+ if (UNLIKELY(result.is_null)) {
Review comment:
ok,Done
----------------------------------------------------------------
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] Youngwb commented on a change in pull request #3439: Support append_trailing_char_if_absent function
Posted by GitBox <gi...@apache.org>.
Youngwb commented on a change in pull request #3439:
URL: https://github.com/apache/incubator-doris/pull/3439#discussion_r420562556
##########
File path: docs/en/sql-reference/sql-functions/string-functions/append_trailing_char_if_absent.md
##########
@@ -0,0 +1,58 @@
+---
+{
+ "title": "append_trailing_char_if_absent",
+ "language": "en"
+}
+---
+
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+# append_trailing_char_if_absent
+## description
+### Syntax
+
+`VARCHAR append_trailing_char_if_absent(VARCHAR str, VARCHAR trailing_char)`
+
+If the s string is non-empty and does not contain the c character at the end, it appends the c character to the end.
+Trailing_char contains only one character, and it will report an error if contains more than one character
Review comment:
OK, Done
----------------------------------------------------------------
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] Youngwb commented on a change in pull request #3439: Support append_trailing_char_if_absent function
Posted by GitBox <gi...@apache.org>.
Youngwb commented on a change in pull request #3439:
URL: https://github.com/apache/incubator-doris/pull/3439#discussion_r417922229
##########
File path: docs/documentation/cn/sql-reference/sql-functions/string-functions/append_trailing_char_if_absent.md
##########
@@ -0,0 +1,46 @@
+<!--
Review comment:
Done
----------------------------------------------------------------
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] Youngwb commented on a change in pull request #3439: Support append_trailing_char_if_absent function
Posted by GitBox <gi...@apache.org>.
Youngwb commented on a change in pull request #3439:
URL: https://github.com/apache/incubator-doris/pull/3439#discussion_r420638248
##########
File path: fe/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java
##########
@@ -605,6 +605,19 @@ public void analyzeImpl(Analyzer analyzer) throws AnalysisException {
return;
}
+ if (fnName.getFunction().equalsIgnoreCase("append_trailing_char_if_absent")) {
Review comment:
ok，Done
----------------------------------------------------------------
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] imay commented on a change in pull request #3439: Support append_trailing_char_if_absent function
Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #3439:
URL: https://github.com/apache/incubator-doris/pull/3439#discussion_r419844026
##########
File path: docs/en/sql-reference/sql-functions/string-functions/append_trailing_char_if_absent.md
##########
@@ -0,0 +1,58 @@
+---
+{
+ "title": "append_trailing_char_if_absent",
+ "language": "en"
+}
+---
+
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+# append_trailing_char_if_absent
+## description
+### Syntax
+
+`VARCHAR append_trailing_char_if_absent(VARCHAR str, VARCHAR trailing_char)`
+
+If the s string is non-empty and does not contain the c character at the end, it appends the c character to the end.
+Trailing_char contains only one character, and it will report an error if contains more than one character
Review comment:
Why not return null when the second parameter length is not 1.
You can't check its length if it is not const, such as a table column.
----------------------------------------------------------------
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] kangkaisen commented on a change in pull request #3439: Support append_trailing_char_if_absent function
Posted by GitBox <gi...@apache.org>.
kangkaisen commented on a change in pull request #3439:
URL: https://github.com/apache/incubator-doris/pull/3439#discussion_r417732090
##########
File path: fe/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java
##########
@@ -604,6 +604,23 @@ public void analyzeImpl(Analyzer analyzer) throws AnalysisException {
return;
}
+ if (fn.getFunctionName().getFunction().equals("append_trailing_char_if_absent")) {
Review comment:
```suggestion
if (fnName.getFunction().equalsIgnoreCase("append_trailing_char_if_absent")) {
```
----------------------------------------------------------------
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