Gabriel39 commented on code in PR #67784:
URL: https://github.com/apache/doris/pull/67784#discussion_r4021717479
##########
be/src/core/column/column_varbinary.cpp:
##########
@@ -28,10 +28,103 @@
#include "core/column/column_string.h"
#include "core/column/columns_common.h"
#include "core/data_type/primitive_type.h"
+#include "exec/common/sip_hash.h"
#include "exec/sort/sort_block.h"
+#include "util/hash_util.hpp"
namespace doris {
#include "common/compile_check_begin.h"
+
+void ColumnVarbinary::insert_many_continuous_binary_data(const char* data,
const uint32_t* offsets,
+ size_t num) {
+ reserve(size() + num);
+ for (size_t row = 0; row < num; ++row) {
+ insert_data(data + offsets[row], offsets[row + 1] - offsets[row]);
+ }
+}
+
+void ColumnVarbinary::insert_many_dict_data(const int32_t* data_array, size_t
start_index,
+ const StringRef* dict, size_t
data_num,
+ uint32_t dict_num) {
+ reserve(size() + data_num);
+ // Decoder pages can be released after the call; copy long dictionary
entries into our arena.
+ for (size_t row = start_index; row < start_index + data_num; ++row) {
+ const auto& value = dict[data_array[row]];
+ insert_data(value.data, value.size);
+ }
+}
+
+// Hash the payload rather than StringView's representation: long values
contain pointers,
+// while short values are inline. Equal binary keys must partition identically
on every BE.
+void ColumnVarbinary::update_hash_with_value(size_t n, SipHash& hash) const {
+ const auto value = get_data_at(n);
+ hash.update(reinterpret_cast<const char*>(&value.size),
sizeof(value.size));
+ hash.update(value.data, value.size);
+}
+
+void ColumnVarbinary::update_hashes_with_value(uint64_t* __restrict hashes,
+ const uint8_t* __restrict
null_data) const {
+ for (size_t row = 0; row < size(); ++row) {
+ if (null_data == nullptr || null_data[row] == 0) {
+ const auto value = get_data_at(row);
+ hashes[row] = HashUtil::xxHash64WithSeed(value.data, value.size,
hashes[row]);
+ }
+ }
+}
+
+void ColumnVarbinary::update_xxHash_with_value(size_t start, size_t end,
uint64_t& hash,
+ const uint8_t* __restrict
null_data) const {
+ for (size_t row = start; row < end; ++row) {
+ if (null_data == nullptr || null_data[row] == 0) {
+ const auto value = get_data_at(row);
+ hash = HashUtil::xxHash64WithSeed(value.data, value.size, hash);
+ }
+ }
+}
+
+void ColumnVarbinary::update_crcs_with_value(uint32_t* __restrict hashes,
PrimitiveType type,
Review Comment:
Fixed in 94969b5299. BE RawValue::zlib_crc32 and FE
VarBinaryLiteral.getHashValue now hash the same raw bytes. Added a CRC unit
test and a 7-bucket regression covering empty, NUL, ASCII, high bytes and
binary prefixes, with equality/IN results and EXPLAIN assertions for
single-tablet pruning. The old implementation failed locally; the updated tests
pass. The missing BE branch also caused the reported P0 SIGABRT during binary
CTAS.
##########
be/src/storage/olap_common.h:
##########
@@ -170,6 +170,8 @@ enum class FieldType {
OLAP_FIELD_TYPE_IPV4 = 38,
OLAP_FIELD_TYPE_IPV6 = 39,
OLAP_FIELD_TYPE_TIMESTAMPTZ = 40,
+ // A distinct persisted type keeps binary payloads out of character
conversion paths.
+ OLAP_FIELD_TYPE_VARBINARY = 41,
Review Comment:
Fixed in 94969b5299 by routing/fencing rather than sending binary values
through text delete predicates. VARBINARY conditions use the existing MOW
row-delete fallback, including when light delete is enabled; other table models
receive an explicit error before text serialization. Local equality/IN
regressions pass for empty, NUL and high-byte values, with VARBINARY both as a
key and as a value column. The Duplicate-table rejection test also verifies
that no rows were deleted. VARBINARY remains the stored type.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/VarBinaryLiteral.java:
##########
@@ -165,6 +146,7 @@ public boolean equals(Object o) {
@Override
protected int computeHashCode() {
- return Objects.hash(super.computeHashCode(), byteValues);
+ // equals compares content regardless of the byte array's identity or
declared length.
+ return Arrays.hashCode(byteValues);
Review Comment:
Fixed in 94969b5299. fastChildrenHashCode now uses Arrays.hashCode, matching
the regular content hash. Added equal parent predicates constructed from
separate byte arrays, checking structural equality, hash equality and HashSet
lookup. The new test failed before the fix and passes afterward.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/CreateTableInfo.java:
##########
@@ -470,9 +470,6 @@ public void validate(ConnectContext ctx) {
throw new AnalysisException(
"Disable to create table column with name start with
__DORIS_: " + columnNameUpperCase);
}
- if (columnDef.getType().isVarBinaryType()) {
- throw new AnalysisException("doris do not support varbinary
create table, could use it by catalog");
- }
if (columnDef.getType().isVariantType()) {
Review Comment:
Fixed in 94969b5299. VARBINARY defaults are validated by UTF-8 byte length.
Unit coverage includes a multibyte default accepted at two bytes and rejected
at one byte, plus an empty default. A local native-table regression inserts
omitted columns and verifies ASCII, multibyte and empty defaults byte-for-byte.
Both unit and end-to-end tests pass.
--
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]
