Copilot commented on code in PR #61276: URL: https://github.com/apache/doris/pull/61276#discussion_r3014244832
########## be/src/exprs/function/cast/function_cast_ip.cpp: ########## @@ -0,0 +1,78 @@ +// 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. + +// This translation unit is the ONLY place that includes cast_to_ip.h. +// All CastToImpl<CastMode, From, ToIpType> template instantiations are +// confined here, keeping them out of function_cast.cpp. + +#include "core/data_type/data_type_ipv4.h" +#include "core/data_type/data_type_ipv6.h" +#include "exprs/function/cast/cast_base.h" +#include "exprs/function/cast/cast_to_ip.h" + +namespace doris::CastWrapper { + +template <typename IpType> + requires(std::is_same_v<IpType, DataTypeIPv4> || std::is_same_v<IpType, DataTypeIPv6>) +WrapperType create_ip_wrapper(FunctionContext* context, const DataTypePtr& from_type) { Review Comment: This file uses `std::is_same_v` but doesn’t include `<type_traits>` locally. Please include `<type_traits>` explicitly in this `.cpp` to avoid depending on transitive includes (which can break with header changes and can vary by toolchain). ########## be/src/exprs/function/cast/cast_wrapper_decls.h: ########## @@ -0,0 +1,53 @@ +// 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. + +#pragma once + +#include "exprs/function/cast/cast_base.h" Review Comment: This header uses `PrimitiveType` in public function declarations but doesn’t include the header that defines it. To follow “include what you use” (and avoid relying on transitive includes), add the appropriate include (e.g., the project header that defines `PrimitiveType`) to `cast_wrapper_decls.h`. ```suggestion #include "exprs/function/cast/cast_base.h" #include "runtime/primitive_type.h" ``` ########## be/src/exprs/function/cast/cast_wrapper_decls.h: ########## @@ -0,0 +1,53 @@ +// 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. + +#pragma once + +#include "exprs/function/cast/cast_base.h" + +namespace doris::CastWrapper { + +// Implemented in function_cast_int.cpp +WrapperType create_int_wrapper(FunctionContext* context, const DataTypePtr& from_type, + PrimitiveType to_type); + +// Implemented in function_cast_float.cpp +WrapperType create_float_wrapper(FunctionContext* context, const DataTypePtr& from_type, + PrimitiveType to_type); + +// Implemented in function_cast_decimal.cpp +WrapperType create_decimal_wrapper(FunctionContext* context, const DataTypePtr& from_type, + PrimitiveType to_type); + +// Implemented in function_cast_date.cpp (handles DATE / DATETIME / DATEv2 / DATETIMEv2 / TIMEv2) +WrapperType create_datelike_wrapper(FunctionContext* context, const DataTypePtr& from_type, + PrimitiveType to_type); + +// Implemented in function_cast_date.cpp Review Comment: The comment claims `create_timestamptz_wrapper` is implemented in `function_cast_date.cpp`, but the implementation in this PR is in `function_cast_timestamptz.cpp`. Update the comment to point to the correct translation unit to avoid misleading future changes. ```suggestion // Implemented in function_cast_timestamptz.cpp ``` ########## be/src/exprs/function/cast/function_cast_decimal.cpp: ########## @@ -0,0 +1,80 @@ +// 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. + +#include "core/data_type/data_type_decimal.h" +#include "exprs/function/cast/cast_to_decimal.h" + +namespace doris::CastWrapper { + +template <typename T> +constexpr static bool type_allow_cast_to_decimal = + std::is_same_v<T, DataTypeString> || IsDataTypeNumber<T> || IsDataTypeDecimal<T>; Review Comment: This file uses `std::is_same_v` but doesn’t include `<type_traits>` locally. Add `<type_traits>` to the includes of this `.cpp` to avoid relying on transitive includes. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
