Copilot commented on code in PR #60910: URL: https://github.com/apache/doris/pull/60910#discussion_r2909625736
########## be/src/exprs/table_function/vjson_each.cpp: ########## @@ -0,0 +1,206 @@ +// 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 "exprs/table_function/vjson_each.h" + +#include <glog/logging.h> + +#include <ostream> +#include <string> + +#include "common/status.h" +#include "core/assert_cast.h" +#include "core/block/block.h" +#include "core/block/column_with_type_and_name.h" +#include "core/column/column.h" +#include "core/column/column_const.h" +#include "core/column/column_struct.h" +#include "core/string_ref.h" +#include "exprs/vexpr.h" +#include "exprs/vexpr_context.h" +#include "util/jsonb_document.h" +#include "util/jsonb_utils.h" +#include "util/jsonb_writer.h" + +namespace doris { +#include "common/compile_check_begin.h" + +template <bool TEXT_MODE> +VJsonEachTableFunction<TEXT_MODE>::VJsonEachTableFunction() { + _fn_name = TEXT_MODE ? "vjson_each_text" : "vjson_each"; +} + +template <bool TEXT_MODE> +Status VJsonEachTableFunction<TEXT_MODE>::process_init(Block* block, RuntimeState* /*state*/) { + int value_column_idx = -1; + RETURN_IF_ERROR(_expr_context->root()->children()[0]->execute(_expr_context.get(), block, + &value_column_idx)); + auto [col, is_const] = unpack_if_const(block->get_by_position(value_column_idx).column); + _json_column = col; + _is_const = is_const; + return Status::OK(); +} + +// Helper: insert one JsonbValue as plain text into a ColumnNullable<ColumnString>. +// For strings: raw blob content (quotes stripped, matching json_each_text PG semantics). +// For null JSON values: SQL NULL (insert_default). +// For all others (numbers, bools, objects, arrays): JSON text representation. +static void insert_value_as_text(const JsonbValue* value, MutableColumnPtr& col) { + if (value == nullptr || value->isNull()) { + col->insert_default(); + return; + } + if (value->isString()) { + const auto* str_val = value->unpack<JsonbStringVal>(); + col->insert_data(str_val->getBlob(), str_val->getBlobLen()); + } else { + JsonbToJson converter; + std::string text = converter.to_json_string(value); + col->insert_data(text.data(), text.size()); + } +} + +// Helper: insert one JsonbValue in JSONB binary form into a ColumnNullable<ColumnString>. +// For null JSON values: SQL NULL (insert_default). +// For all others: write JSONB binary via JsonbWriter. +static void insert_value_as_json(const JsonbValue* value, MutableColumnPtr& col, + JsonbWriter& writer) { + if (value == nullptr || value->isNull()) { + col->insert_default(); + return; + } + writer.reset(); + writer.writeValue(value); + const auto* buf = writer.getOutput()->getBuffer(); + size_t len = writer.getOutput()->getSize(); + col->insert_data(buf, len); +} + +template <bool TEXT_MODE> +void VJsonEachTableFunction<TEXT_MODE>::process_row(size_t row_idx) { + TableFunction::process_row(row_idx); + if (_is_const && _cur_size > 0) { + return; + } + + StringRef text; + const size_t idx = _is_const ? 0 : row_idx; + if (const auto* nullable_col = check_and_get_column<ColumnNullable>(*_json_column)) { + if (nullable_col->is_null_at(idx)) { + return; + } + text = assert_cast<const ColumnString&>(nullable_col->get_nested_column()).get_data_at(idx); + } else { + text = assert_cast<const ColumnString&>(*_json_column).get_data_at(idx); + } + + const JsonbDocument* doc = nullptr; + auto st = JsonbDocument::checkAndCreateDocument(text.data, text.size, &doc); + if (!st.ok() || !doc || !doc->getValue()) [[unlikely]] { + return; + } + + const JsonbValue* jv = doc->getValue(); + if (!jv->isObject()) { + return; + } + + const auto* obj = jv->unpack<ObjectVal>(); + _cur_size = obj->numElem(); + if (_cur_size == 0) { + return; + } + + _kv_pairs.first = ColumnNullable::create(ColumnString::create(), ColumnUInt8::create()); + _kv_pairs.second = ColumnNullable::create(ColumnString::create(), ColumnUInt8::create()); + _kv_pairs.first->reserve(_cur_size); + _kv_pairs.second->reserve(_cur_size); + + if constexpr (TEXT_MODE) { + for (const auto& kv : *obj) { + _kv_pairs.first->insert_data(kv.getKeyStr(), kv.klen()); + insert_value_as_text(kv.value(), _kv_pairs.second); + } + } else { + JsonbWriter writer; + for (const auto& kv : *obj) { + _kv_pairs.first->insert_data(kv.getKeyStr(), kv.klen()); + insert_value_as_json(kv.value(), _kv_pairs.second, writer); + } + } +} + +template <bool TEXT_MODE> +void VJsonEachTableFunction<TEXT_MODE>::process_close() { + _json_column = nullptr; + _kv_pairs.first = nullptr; + _kv_pairs.second = nullptr; +} Review Comment: `process_row()` has a const-argument fast path (`if (_is_const && _cur_size > 0) return;`), but `process_close()` clears `_kv_pairs` without resetting `_cur_size`. Since `TableFunctionOperator` calls `process_close()` after each child block and then reuses the same TableFunction instance, the next block can hit the fast path with `_cur_size > 0` while `_kv_pairs` is null, leading to a crash when `get_value()`/`get_same_many_values()` dereference `_kv_pairs`. Reset the cached state in `process_close()` (e.g., set `_cur_size = 0` / `_cur_offset = 0` and/or clear `_is_const`), or keep the cached `_kv_pairs` for const inputs across blocks so the fast path remains valid. ########## be/test/exprs/function/table_function_test.cpp: ########## @@ -308,4 +308,392 @@ TEST_F(TableFunctionTest, vexplode_numbers) { } } +// --------------------------------------------------------------------------- +// Direct-API helpers for json_each / json_each_text tests. +// The test framework's check_vec_table_function does not properly support +// TYPE_STRUCT output (insert_cell always expects ColumnNullable wrapping the +// struct column), so we drive the table function API directly. +// --------------------------------------------------------------------------- + +// Build a one-column JSONB input block. An empty string means SQL NULL. +static std::unique_ptr<Block> build_jsonb_input_block(const std::vector<std::string>& json_rows) { + auto str_col = ColumnString::create(); + auto null_col = ColumnUInt8::create(); + for (const auto& json : json_rows) { + if (json.empty()) { + str_col->insert_default(); + null_col->insert_value(1); + } else { + JsonBinaryValue jbv; + if (jbv.from_json_string(json.c_str(), json.size()).ok()) { + str_col->insert_data(jbv.value(), jbv.size()); + null_col->insert_value(0); + } else { + str_col->insert_default(); + null_col->insert_value(1); + } + } + } + auto col = ColumnNullable::create(std::move(str_col), std::move(null_col)); + auto block = Block::create_unique(); + block->insert({std::move(col), + make_nullable(DataTypeFactory::instance().create_data_type( + doris::PrimitiveType::TYPE_JSONB, false)), + "jval"}); + return block; +} + +// Run the given table function over all rows in block. +// Returns list of (key, value) pairs where value == "__NULL__" means SQL NULL. +// val_is_jsonb controls whether the value column is decoded as JSONB→JSON text or plain text. +static std::vector<std::pair<std::string, std::string>> run_json_each_fn(TableFunction* fn, + Block* block, + bool val_is_jsonb) { + // Output type: Nullable(Struct(Nullable(VARCHAR key), Nullable(VARCHAR/JSONB value))) + DataTypePtr key_dt = make_nullable(DataTypeFactory::instance().create_data_type( + doris::PrimitiveType::TYPE_VARCHAR, false)); + DataTypePtr val_dt = make_nullable(DataTypeFactory::instance().create_data_type( + val_is_jsonb ? doris::PrimitiveType::TYPE_JSONB : doris::PrimitiveType::TYPE_VARCHAR, + false)); + DataTypePtr struct_dt = + make_nullable(std::make_shared<DataTypeStruct>(DataTypes {key_dt, val_dt})); + + auto out_col = struct_dt->create_column(); + fn->set_nullable(); + + TQueryOptions q_opts; + TQueryGlobals q_globals; + RuntimeState rs(q_opts, q_globals); + EXPECT_TRUE(fn->process_init(block, &rs).ok()); + + for (size_t row = 0; row < block->rows(); ++row) { + fn->process_row(row); + if (!fn->current_empty()) { + do { + fn->get_value(out_col, 1); + } while (!fn->eos()); + } + } + fn->process_close(); + + std::vector<std::pair<std::string, std::string>> result; + const auto& nullable_out = assert_cast<const ColumnNullable&>(*out_col); + const auto& struct_col = assert_cast<const ColumnStruct&>(nullable_out.get_nested_column()); + const auto& key_col = assert_cast<const ColumnNullable&>(struct_col.get_column(0)); + const auto& val_col = assert_cast<const ColumnNullable&>(struct_col.get_column(1)); + + for (size_t i = 0; i < struct_col.size(); ++i) { + if (nullable_out.is_null_at(i)) { + result.emplace_back("__NULL_ROW__", "__NULL_ROW__"); + continue; + } + std::string key; + if (!key_col.is_null_at(i)) { + StringRef sr = key_col.get_nested_column().get_data_at(i); + key.assign(sr.data, sr.size); + } + std::string val; + if (val_col.is_null_at(i)) { + val = "__NULL__"; + } else { + StringRef sr = val_col.get_nested_column().get_data_at(i); + if (val_is_jsonb) { + // JSONB binary → JSON text for comparison + const JsonbDocument* doc = nullptr; + if (JsonbDocument::checkAndCreateDocument(sr.data, sr.size, &doc).ok() && doc && + doc->getValue()) { + val = JsonbToJson().to_json_string(doc->getValue()); + } else { + val = "__BAD_JSONB__"; + } + } else { + val.assign(sr.data, sr.size); + } + } + result.emplace_back(std::move(key), std::move(val)); + } + return result; +} + +TEST_F(TableFunctionTest, vjson_each) { Review Comment: This new test code uses types that are not included in this translation unit (e.g. `VJsonEachTableFn`/`VJsonEachTextTableFn`, `ColumnString`, `JsonBinaryValue`, `JsonbDocument`, `JsonbToJson`, `RuntimeState`). Unless they are pulled in transitively by existing includes, this will fail to compile. Add the missing headers explicitly (e.g. `exprs/table_function/vjson_each.h`, `core/column/column_string.h`, `core/value/jsonb_value.h`, `util/jsonb_document.h`, `util/jsonb_utils.h`, `runtime/runtime_state.h`, etc.) so the file is self-contained. -- 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]
