github-actions[bot] commented on code in PR #68301:
URL: https://github.com/apache/doris/pull/68301#discussion_r4068110842
##########
be/src/core/value/timestamptz_value.cpp:
##########
@@ -65,6 +73,14 @@ std::string TimestampTzValue::to_string(const
cctz::time_zone& tz, int scale) co
buffer[len++] = ':';
buffer[len++] = static_cast<char>('0' + offset_mins / 10);
buffer[len++] = '0' + offset_mins % 10;
+ // Historical zones can have sub-minute offsets. Dropping their seconds
changes the
+ // instant represented by the client-visible wall clock and offset when
read back.
+ const int offset_seconds = abs_offset % 60;
Review Comment:
[P2] Update the FE typed-literal parser for the new offset grammar. This
formatter can now return `1890-01-01 08:05:43.123456+08:05:43`, and both BE
parsers accept it, but `DateLiteralUtils` recognizes and strips only a
six-character `+HH:MM` suffix. Nereids column-default validation delegates
TIMESTAMPTZ strings there, so copying this result into a DEFAULT leaves five
colon-separated time parts and is rejected. Also, `ZoneId.of(tzString)` cannot
represent offsets beyond Java's 18-hour limit even though these changed BE
branches accept every offset below 24 hours. Please handle the full BE wire
range in the shared parser and add formatter-to-`ColumnDefinition` round trips
for second-granularity and >18-hour offsets.
##########
be/src/core/data_type_serde/data_type_varbinary_serde.cpp:
##########
@@ -301,6 +305,82 @@ Status
DataTypeVarbinarySerDe::deserialize_one_cell_from_json(IColumn& column, S
return Status::OK();
}
+Status DataTypeVarbinarySerDe::from_string(StringRef& str, IColumn& column,
+ const FormatOptions& options) const
{
+ // Partition structs use the same hex representation as nested VARBINARY
output. Decode it
+ // before appending so arbitrary bytes survive JSON transport instead of
becoming NULL.
+ if (str.size < 2 || str.data[0] != '0' || str.data[1] != 'x' || (str.size
- 2) % 2 != 0 ||
+ str.size - 2 > std::numeric_limits<int>::max()) {
+ return Status::InvalidArgument("Invalid VARBINARY hex representation");
+ }
+ // The INT_MAX guard also makes narrowing to the decoder's 32-bit offset
type safe.
+ const auto hex_size = cast_set<ColumnString::Offset>(str.size - 2);
+ std::string bytes(hex_size / 2, '\0');
+ if (string_hex::hex_decode(str.data + 2, hex_size, bytes.data()) !=
bytes.size()) {
+ return Status::InvalidArgument("Invalid VARBINARY hex representation");
+ }
+ assert_cast<ColumnVarbinary&>(column).insert_data(bytes.data(),
bytes.size());
+ return Status::OK();
+}
+
+Status DataTypeVarbinarySerDe::deserialize_one_cell_from_hive_text(
+ IColumn& column, Slice& slice, const FormatOptions& options,
+ int hive_text_complex_type_delimiter_level) const {
+ // Hive LazyBinary uses lenient Base64 (including URL-safe letters and
whitespace),
+ // falling back to the original bytes for non-Base64 input or an empty
decoding.
+ // Keep this separate from JSON/CSV: those formats do not share Hive's
encoding contract.
+ std::string encoded;
Review Comment:
[P1] Put these full-cell Hive staging buffers behind Doris's checked
allocator. For every valid binary field this reserves `slice.size` in
`encoded`, `base64_decode` then resizes a second `std::string` to the same
size, and `ColumnVarbinary` finally copies the decoded bytes into its arena.
Ordinary `std::string` storage does not run the `Allocator<false>` admission
check, so one large external field can build roughly two input-sized buffers
before the tracked column allocation rejects it; the new write path likewise
materializes both `value.to_string()` and its Base64 output. Please
decode/encode directly into checked destination storage (and avoid the hex
path's equivalent whole-cell temporary), with a limited-memory scan/write test.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArrayFunctionUtils.java:
##########
@@ -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.
+
+package org.apache.doris.nereids.trees.expressions.functions.scalar;
+
+import org.apache.doris.nereids.exceptions.AnalysisException;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.types.ArrayType;
+import org.apache.doris.nereids.types.DataType;
+
+/** Argument validation shared by array functions. */
+final class ArrayFunctionUtils {
+ private ArrayFunctionUtils() {
+ }
+
+ static void checkNoVarBinaryArguments(ScalarFunction function) {
+ // Inspect original arguments before coercion can hide unsupported
binary comparison/hash inputs.
+ for (Expression argument : function.getArguments()) {
+ DataType type = argument.getDataType();
+ while (type instanceof ArrayType) {
+ type = ((ArrayType) type).getItemType();
+ }
+ if (type.isVarBinaryType()) {
Review Comment:
[P2] Reject the resolved execution type rather than every original argument.
For `array_contains(ARRAY<STRING>, VARBINARY)`, the FOLLOW signature resolves
the scalar to STRING and the existing implicit cast feeds the supported BE
string kernel; `array_position`, `count_equal`, `array_remove`, and
`array_contains_all` share that pattern. With default new type coercion,
indexed-Any common-type selection likewise resolves mixed
`ARRAY<STRING>`/`ARRAY<VARBINARY>` inputs to STRING for `arrays_overlap`,
`array_except`, and `array_union`, whose BE implementations have `ColumnString`
paths. This guard rejects all of them before coercion, and the new mixed-type
test locks in the regression. Please reject signatures that resolve to
VARBINARY, but allow binary inputs that resolve to supported execution types.
##########
be/src/core/data_type_serde/data_type_varbinary_serde.cpp:
##########
@@ -301,6 +305,82 @@ Status
DataTypeVarbinarySerDe::deserialize_one_cell_from_json(IColumn& column, S
return Status::OK();
}
+Status DataTypeVarbinarySerDe::from_string(StringRef& str, IColumn& column,
+ const FormatOptions& options) const
{
+ // Partition structs use the same hex representation as nested VARBINARY
output. Decode it
+ // before appending so arbitrary bytes survive JSON transport instead of
becoming NULL.
+ if (str.size < 2 || str.data[0] != '0' || str.data[1] != 'x' || (str.size
- 2) % 2 != 0 ||
+ str.size - 2 > std::numeric_limits<int>::max()) {
+ return Status::InvalidArgument("Invalid VARBINARY hex representation");
+ }
+ // The INT_MAX guard also makes narrowing to the decoder's 32-bit offset
type safe.
+ const auto hex_size = cast_set<ColumnString::Offset>(str.size - 2);
+ std::string bytes(hex_size / 2, '\0');
+ if (string_hex::hex_decode(str.data + 2, hex_size, bytes.data()) !=
bytes.size()) {
+ return Status::InvalidArgument("Invalid VARBINARY hex representation");
+ }
+ assert_cast<ColumnVarbinary&>(column).insert_data(bytes.data(),
bytes.size());
+ return Status::OK();
+}
+
+Status DataTypeVarbinarySerDe::deserialize_one_cell_from_hive_text(
Review Comment:
[P1] Propagate the Hive BINARY decoding contract instead of guessing it from
each cell. Hive 4 supports `hive.serialization.decode.binary.as.base64=false`
for raw TEXTFILE values, but FE drops that property, so a configured raw value
such as `test` is silently Base64-decoded here to different bytes. In default
mode Hive 4/current `LazyBinary` also uses the strict Basic decoder with raw
fallback, while this code accepts URL-safe letters/whitespace and truncates a
one-sextet tail (for example Hive preserves `-_8=` but Doris returns `0xfb
0xff`). Please pass the property/version-selected mode through
`TFileAttributes`, use the matching decoder, and cover raw plus Hive 3/4 Base64
cases. See the [Hive TEXTFILE
contract](https://hive.apache.org/docs/latest/language/languagemanual-ddl/#storage-formats)
and [Hive 4
`LazyBinary`](https://github.com/apache/hive/blob/rel/release-4.0.1/serde/src/java/org/apache/hadoop/hive/serde2/lazy/LazyBinary.java#L43-L57).
##########
be/src/exprs/function/cast/cast_to_datetimev2_impl.hpp:
##########
@@ -705,24 +706,47 @@ inline bool
CastToDatetimeV2::from_string_strict_mode_internal(
SET_PARAMS_RET_FALSE_IFN((consume_digit<UInt32, 2>(ptr, end,
part[0])),
"invalid hour offset '{}'",
std::string {ptr, end});
}
- SET_PARAMS_RET_FALSE_IFN(part[0] <= 14, "invalid hour offset
'{}'", part[0]);
+ SET_PARAMS_RET_FALSE_IFN(
+ part[0] < (type == DataTimeCastEnumType::TIMESTAMP_TZ ?
24U : 15U),
Review Comment:
[P1] Apply the offset before rejecting a fractional carry at the local-year
boundary. This branch now admits `+15:00`, but both BE parsers round the local
civil value before parsing/applying that offset. Thus `CAST('9999-12-31
23:59:59.5+15:00' AS TIMESTAMPTZ(0))` tries to carry the local value into year
10000 and errors (or becomes NULL in non-strict mode) before the offset can
produce the valid UTC value `9999-12-31 09:00:00+00:00`. FE folding converts to
UTC before scale rounding and returns that value, so `debug_skip_fold_constant`
changes the result. Please defer this carry/range validation until after offset
conversion and cover folded/non-folded strict and non-strict boundary cases.
--
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]
