Gabriel39 commented on code in PR #67784: URL: https://github.com/apache/doris/pull/67784#discussion_r4056550105
########## be/src/format/transformer/viceberg_parquet_transformer.h: ########## @@ -0,0 +1,42 @@ +// 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 "format/table/iceberg/schema.h" +#include "format/transformer/vparquet_transformer.h" + +namespace doris { +#include "common/compile_check_begin.h" + +class VIcebergParquetTransformer final : public VParquetTransformer { Review Comment: Addressed in 19bb5f231e. Renamed the file-writing classes and source files to VParquetWriter and VIcebergParquetWriter, updating the factory, sink callers, and tests. ArrowBlockConvertor remains responsible for in-memory representation conversion. ########## be/src/format/table/paimon/paimon_arrow_block_convertor.h: ########## @@ -0,0 +1,35 @@ +// 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 "format/arrow/arrow_block_convertor.h" + +namespace doris::paimon { + +class PaimonArrowBlockConvertor final : public ArrowBlockConvertor { +protected: + Status write_column(const std::shared_ptr<const IDataType>& type, const DataTypeSerDe& serde, Review Comment: Addressed in 19bb5f231e. Paimon reads use native Parquet/ORC or JNI scanners, not this Arrow write adapter. Documented that contract and added a test that the converter rejects generic Arrow reads. The Paimon reader tests cover logical timestamp semantics, including nested containers, rather than silently reusing Flight decoding. ########## be/src/exec/common/hash_table/hash_map_context.h: ########## @@ -389,9 +389,16 @@ struct MethodStringNoCache : public MethodBase<TData> { if (nested_column.is_column_string64()) { const auto& column_string = assert_cast<const ColumnString64&>(nested_column); serialized_str(column_string, stored_keys); - } else { + } else if (nested_column.is_column_string()) { const auto& column_string = assert_cast<const ColumnString&>(nested_column); serialized_str(column_string, stored_keys); + } else { + // VARBINARY stores StringViews, not a character buffer plus offsets. Keep raw payload + // references so nullable single-key joins use the same key layout on build and probe. + stored_keys.resize(num_rows); Review Comment: Addressed in 19bb5f231e. Removed the binary-specific MethodStringNoCache branch and the added binary hash-key support. Single and composite binary hash keys now return an explicit unsupported error before serialization fallback. Tests cover nullable/non-nullable binary keys in either position. ########## be/src/exprs/hybrid_set.h: ########## @@ -466,7 +466,8 @@ class HybridSet : public HybridSetBase { ObjectPool _pool; }; -template <typename _ContainerType = DynamicContainer<std::string>> +template <typename _ContainerType = DynamicContainer<std::string>, + typename ColumnType = ColumnString> class StringSet : public HybridSetBase { Review Comment: Addressed in 19bb5f231e. Restored StringSet to its original single template parameter. The added binary IN/NOT IN support is removed and now returns NotSupported, so the shared class needs no binary-specific probing or template dimension. ########## be/src/exprs/hybrid_set.h: ########## @@ -466,7 +466,8 @@ class HybridSet : public HybridSetBase { ObjectPool _pool; }; -template <typename _ContainerType = DynamicContainer<std::string>> +template <typename _ContainerType = DynamicContainer<std::string>, + typename ColumnType = ColumnString> class StringSet : public HybridSetBase { Review Comment: Addressed in 19bb5f231e. Addressed together with the preceding comment: removed the ColumnType template parameter and the added binary predicate/IN support. Binary predicate creation and IN/NOT IN explicitly report unsupported. -- 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]
