Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] Support inet_aton function #51883

Merged
merged 6 commits into from
Dec 6, 2024

Conversation

lufeizxy
Copy link
Contributor

@lufeizxy lufeizxy commented Oct 14, 2024

Why I'm doing:

What I'm doing:

Support inet_aton function
For #49664

mysql> select inet_aton('192.168.1.1'); 
+--------------------------------------+ 
| inet_aton('192.168.1.1')             | 
+--------------------------------------+ 
| 3232235777                           | 
+--------------------------------------+ 

What type of PR is this:

  • BugFix
  • Feature
  • Enhancement
  • Refactor
  • UT
  • Doc
  • Tool

Does this PR entail a change in behavior?

  • Yes, this PR will result in a change in behavior.
  • No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

  • Interface/UI changes: syntax, type conversion, expression evaluation, display information
  • Parameter changes: default values, similar parameters but with different default values
  • Policy changes: use new policy to replace old one, functionality automatically enabled
  • Feature removed
  • Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

  • I have added test cases for my bug fix or my new feature
  • This pr needs user documentation (for new or modified features or behaviors)
    • I have added documentation for my new feature or new function
  • This is a backport pr

Bugfix cherry-pick branch check:

  • I have checked the version labels which the pr will be auto-backported to the target branch
    • 3.4
    • 3.3
    • 3.2
    • 3.1
    • 3.0
    • 2.5

@lufeizxy lufeizxy requested review from a team as code owners October 14, 2024 09:24
@CLAassistant
Copy link

CLAassistant commented Oct 14, 2024

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Oct 14, 2024
be/src/exprs/inet_aton.cpp Outdated Show resolved Hide resolved
be/src/common/format_ip.h Outdated Show resolved Hide resolved
@lufeizxy
Copy link
Contributor Author

@Seaven @wangsimo0 Can you help review this PR? Thank you!

@stdpain
Copy link
Contributor

stdpain commented Oct 14, 2024

CI: Run echo "::error::Please check the format of your pr body about the behavior change checkbox!"

be/src/exprs/inet_aton.cpp Outdated Show resolved Hide resolved
be/src/exprs/inet_aton.cpp Outdated Show resolved Hide resolved
be/src/common/format_ip.h Outdated Show resolved Hide resolved
@lufeizxy
Copy link
Contributor Author

@stdpain Thank you for your feedback! I will address these suggestions as soon as possible.

Copy link

sonarcloud bot commented Oct 23, 2024

@lufeizxy
Copy link
Contributor Author

lufeizxy commented Nov 12, 2024

namespace starrocks {

static inline bool try_parse_ipv4(const char* pos, int64& result_value) {
return parse_ipv4(pos, result_value);
}

DEFINE_UNARY_FN_WITH_IMPL(convert_to_ipv4_impl, str) {
int64_t result;
if (!try_parse_ipv4(str.data, result)) {
return -1;
}
return result;
}

StatusOr StringFunctions::inet_aton(FunctionContext* context, const Columns& columns) {

return VectorizedStrictUnaryFunction<convert_to_ipv4_impl>::evaluate<TYPE_VARCHAR, TYPE_BIGINT>(column);

}

} // namespace starrocks
This implementation requires replacing -1 in the final result with null. Aside from modifying the evaluate method and validating input parameters on the FE side, I am not aware of any other efficient approach, so I have changed the implementation method. The other points mentioned in the PR have also been addressed.
@stdpain @Seaven @wangsimo0 Could you please review the PR again? Thank you!

@lufeizxy lufeizxy requested a review from stdpain November 12, 2024 15:57
@lufeizxy lufeizxy closed this Nov 13, 2024
@lufeizxy lufeizxy reopened this Nov 13, 2024
be/src/common/format_ip.h Outdated Show resolved Hide resolved
@lufeizxy lufeizxy force-pushed the inet_aton_function_1014 branch from b3f8bb1 to 6841ff7 Compare December 3, 2024 10:09
@lufeizxy lufeizxy force-pushed the inet_aton_function_1014 branch from 6841ff7 to 485a8c8 Compare December 3, 2024 10:17
stdpain
stdpain previously approved these changes Dec 3, 2024
@stdpain
Copy link
Contributor

stdpain commented Dec 3, 2024

CI failed in your cases:

  *** Aborted at 1733224555 (unix time) try "date -d @1733224555" if you are using GNU date ***
  PC: @     0x7f049a3b20bc (/usr/lib/x86_64-linux-gnu/libc.so.6+0x1b20bb)
  *** SIGSEGV (@0x0) received by PID 9464 (TID 0x7f049a4f2800) from PID 0; stack trace: ***
      @     0x7f049a299ee8 (/usr/lib/x86_64-linux-gnu/libc.so.6+0x99ee7)
      @         0x14240c19 google::(anonymous namespace)::FailureSignalHandler(int, siginfo_t*, void*)
      @     0x7f049a242520 (/usr/lib/x86_64-linux-gnu/libc.so.6+0x4251f)
      @     0x7f049a3b20bc (/usr/lib/x86_64-linux-gnu/libc.so.6+0x1b20bb)
      @          0x8735b57 starrocks::VecStringFunctionsTest1536_inetAtonInvalidIPv4Test_Test::TestBody()
      @         0x166a54d4 void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*)
      @         0x166968ae testing::Test::Run()
      @         0x16696a15 testing::TestInfo::Run()
      @         0x16696b05 testing::TestSuite::Run()
      @         0x166970c6 testing::internal::UnitTestImpl::RunAllTests()
      @         0x16697301 testing::UnitTest::Run()
      @          0x6c8e6cb starrocks::init_test_env(int, char**)
      @          0x6c9b736 main
      @     0x7f049a229d90 (/usr/lib/x86_64-linux-gnu/libc.so.6+0x29d8f)
      @     0x7f049a229e40 __libc_start_main
      @          0x6c8d025 _start

@lufeizxy
Copy link
Contributor Author

lufeizxy commented Dec 4, 2024

@stdpain I will resolve it as soon as possible.

@lufeizxy
Copy link
Contributor Author

lufeizxy commented Dec 5, 2024

@stdpain I have resolved the CI failures in my cases. Could you please review the PR again? Thank you!
image

Copy link

sonarcloud bot commented Dec 6, 2024

Copy link

github-actions bot commented Dec 6, 2024

[Java-Extensions Incremental Coverage Report]

pass : 0 / 0 (0%)

Copy link

github-actions bot commented Dec 6, 2024

[FE Incremental Coverage Report]

pass : 0 / 0 (0%)

Copy link

github-actions bot commented Dec 6, 2024

[BE Incremental Coverage Report]

pass : 37 / 43 (86.05%)

file detail

path covered_line new_line coverage not_covered_line_detail
🔵 be/src/common/format_ip.h 21 25 84.00% [43, 44, 78, 79]
🔵 be/src/exprs/inet_aton.cpp 16 18 88.89% [33, 34]

@stdpain stdpain merged commit f02f655 into StarRocks:main Dec 6, 2024
64 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation version:3.5
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants