Skip to content

Commit

Permalink
[Fix-3.0](smooth-upgrade) Fix smooth upgrade of function is_ip_addres…
Browse files Browse the repository at this point in the history
…s_in_range (#45428)

### What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary:

The origin PR is #35239. for
branch-3.0 it was merged in 3.0.0 but forgot to register old version.
now in branch-3.0 we fix it in this PR. and will pick it to branch-2.0
in #45358 which must be merged in
2.1.8.
then:
```
FROM    TO    result
217-    218+    ✅
217-    303-    💥
218+    303-    ✅
218+    304+    ✅
303-    304+    ✅
```
this is our best result.

### Release note

None

### Check List (For Author)

- Test <!-- At least one of them must be included. -->
    - [ ] Regression test
    - [ ] Unit Test
    - [ ] Manual test (add detailed scripts or steps below)
    - [x] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
        - [x] Previous test can cover this change.
        - [ ] No code files have been changed.
        - [ ] Other reason <!-- Add your reason?  -->

- Behavior changed:
    - [x] No.
    - [ ] Yes. <!-- Explain the behavior change -->

- Does this need documentation?
    - [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
apache/doris-website#1214 -->

### Check List (For Reviewer who merge this PR)

- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR
should merge into -->
  • Loading branch information
zclllyybb authored Dec 17, 2024
1 parent 820b300 commit 2010f9f
Show file tree
Hide file tree
Showing 8 changed files with 104 additions and 1 deletion.
5 changes: 5 additions & 0 deletions be/src/runtime/runtime_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,11 @@ class RuntimeState {
return _query_options.__isset.enable_decimal256 && _query_options.enable_decimal256;
}

bool new_is_ip_address_in_range() const {
return _query_options.__isset.new_is_ip_address_in_range &&
_query_options.new_is_ip_address_in_range;
}

bool enable_common_expr_pushdown() const {
return _query_options.__isset.enable_common_expr_pushdown &&
_query_options.enable_common_expr_pushdown;
Expand Down
4 changes: 3 additions & 1 deletion be/src/vec/exprs/vectorized_fn_call.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,9 @@ Status VectorizedFnCall::prepare(RuntimeState* state, const RowDescriptor& desc,
// get the function. won't prepare function.
_function = SimpleFunctionFactory::instance().get_function(
_fn.name.function_name, argument_template, _data_type,
{.enable_decimal256 = state->enable_decimal256()}, state->be_exec_version());
{.enable_decimal256 = state->enable_decimal256(),
.new_is_ip_address_in_range = state->new_is_ip_address_in_range()},
state->be_exec_version());
}
if (_function == nullptr) {
return Status::InternalError("Could not find function {}, arg {} return {} ",
Expand Down
1 change: 1 addition & 0 deletions be/src/vec/functions/function.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ namespace doris::vectorized {

struct FunctionAttr {
bool enable_decimal256 {false};
bool new_is_ip_address_in_range {false};
};

#define RETURN_REAL_TYPE_FOR_DATEV2_FUNCTION(TYPE) \
Expand Down
2 changes: 2 additions & 0 deletions be/src/vec/functions/function_ip.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ void register_function_ip(SimpleFunctionFactory& factory) {
factory.register_function<FunctionIsIPString<IPv6>>();
factory.register_function<FunctionIsIPAddressInRange>();

factory.register_function<FunctionIsIPAddressInRangeOld>();

/// CIDR part
factory.register_function<FunctionIPv4CIDRToRange>();
factory.register_function<FunctionIPv6CIDRToRange>();
Expand Down
75 changes: 75 additions & 0 deletions be/src/vec/functions/function_ip.h
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +789,81 @@ class FunctionIsIPAddressInRange : public IFunction {
}
};

// old version throw exception when meet null value
class FunctionIsIPAddressInRangeOld : public IFunction {
public:
static constexpr auto name = "is_ip_address_in_range";
static FunctionPtr create() { return std::make_shared<FunctionIsIPAddressInRange>(); }

String get_name() const override { return name; }

size_t get_number_of_arguments() const override { return 2; }

DataTypePtr get_return_type_impl(const DataTypes& arguments) const override {
return std::make_shared<DataTypeUInt8>();
}

bool use_default_implementation_for_nulls() const override { return false; }
Status execute_impl(FunctionContext* context, Block& block, const ColumnNumbers& arguments,
size_t result, size_t input_rows_count) const override {
const auto& addr_column_with_type_and_name = block.get_by_position(arguments[0]);
const auto& cidr_column_with_type_and_name = block.get_by_position(arguments[1]);
WhichDataType addr_type(addr_column_with_type_and_name.type);
WhichDataType cidr_type(cidr_column_with_type_and_name.type);
const auto& [addr_column, addr_const] =
unpack_if_const(addr_column_with_type_and_name.column);
const auto& [cidr_column, cidr_const] =
unpack_if_const(cidr_column_with_type_and_name.column);
const ColumnString* str_addr_column = nullptr;
const ColumnString* str_cidr_column = nullptr;
const NullMap* null_map_addr = nullptr;
const NullMap* null_map_cidr = nullptr;
if (addr_type.is_nullable()) {
const auto* addr_column_nullable =
assert_cast<const ColumnNullable*>(addr_column.get());
str_addr_column = assert_cast<const ColumnString*>(
addr_column_nullable->get_nested_column_ptr().get());
null_map_addr = &addr_column_nullable->get_null_map_data();
} else {
str_addr_column = assert_cast<const ColumnString*>(addr_column.get());
}

if (cidr_type.is_nullable()) {
const auto* cidr_column_nullable =
assert_cast<const ColumnNullable*>(cidr_column.get());
str_cidr_column = assert_cast<const ColumnString*>(
cidr_column_nullable->get_nested_column_ptr().get());
null_map_cidr = &cidr_column_nullable->get_null_map_data();
} else {
str_cidr_column = assert_cast<const ColumnString*>(cidr_column.get());
}

auto col_res = ColumnUInt8::create(input_rows_count, 0);
auto& col_res_data = col_res->get_data();
for (size_t i = 0; i < input_rows_count; ++i) {
auto addr_idx = index_check_const(i, addr_const);
auto cidr_idx = index_check_const(i, cidr_const);
if (null_map_addr && (*null_map_addr)[addr_idx]) [[unlikely]] {
throw Exception(ErrorCode::INVALID_ARGUMENT,
"The arguments of function {} must be String, not NULL",
get_name());
}
if (null_map_cidr && (*null_map_cidr)[cidr_idx]) [[unlikely]] {
throw Exception(ErrorCode::INVALID_ARGUMENT,
"The arguments of function {} must be String, not NULL",
get_name());
}
const auto addr =
IPAddressVariant(str_addr_column->get_data_at(addr_idx).to_string_view());
const auto cidr =
parse_ip_with_cidr(str_cidr_column->get_data_at(cidr_idx).to_string_view());
col_res_data[i] = is_address_in_range(addr, cidr) ? 1 : 0;
}
block.replace_by_position(result, std::move(col_res));
return Status::OK();
}
};

class FunctionIPv4CIDRToRange : public IFunction {
public:
static constexpr auto name = "ipv4_cidr_to_range";
Expand Down
5 changes: 5 additions & 0 deletions be/src/vec/functions/simple_function_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@ class SimpleFunctionFactory {
int be_version = BeExecVersionManager::get_newest_version()) {
std::string key_str = name;

// special function replacement
if (key_str == "is_ip_address_in_range" && !attr.new_is_ip_address_in_range) [[unlikely]] {
key_str = "__is_ip_address_in_range_OLD__";
}

if (function_alias.contains(name)) {
key_str = function_alias[name];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -681,6 +681,8 @@ public class SessionVariable implements Serializable, Writable {

public static final String ENABLE_COOLDOWN_REPLICA_AFFINITY =
"enable_cooldown_replica_affinity";

public static final String NEW_IS_IP_ADDRESS_IN_RANGE = "new_is_ip_address_in_range";
/**
* Inserting overwrite for auto partition table allows creating partition for
* datas which cannot find partition to overwrite.
Expand Down Expand Up @@ -2309,6 +2311,11 @@ public void setIgnoreShapePlanNodes(String ignoreShapePlanNodes) {
})
public int adaptivePipelineTaskSerialReadOnLimit = 10000;

// only to control some function behaviour. not visible or mutable.
@VariableMgr.VarAttr(name = NEW_IS_IP_ADDRESS_IN_RANGE, needForward = true, flag = VariableMgr.INVISIBLE
| VariableMgr.READ_ONLY)
public boolean newIsIpAddressInRange = true;

public void setEnableEsParallelScroll(boolean enableESParallelScroll) {
this.enableESParallelScroll = enableESParallelScroll;
}
Expand Down Expand Up @@ -3957,6 +3964,8 @@ public TQueryOptions toThrift() {
tResult.setOrcOnceMaxReadBytes(orcOnceMaxReadBytes);
tResult.setIgnoreRuntimeFilterError(ignoreRuntimeFilterError);

tResult.setNewIsIpAddressInRange(newIsIpAddressInRange);

return tResult;
}

Expand Down
4 changes: 4 additions & 0 deletions gensrc/thrift/PaloInternalService.thrift
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,10 @@ struct TQueryOptions {
140: optional i64 orc_max_merge_distance_bytes = 1048576;

141: optional bool ignore_runtime_filter_error = false;

// upgrade options. keep them same in every branch.
200: optional bool new_is_ip_address_in_range = false;

// For cloud, to control if the content would be written into file cache
// In write path, to control if the content would be written into file cache.
// In read path, read from file cache or remote storage when execute query.
Expand Down

0 comments on commit 2010f9f

Please sign in to comment.