github-actions[bot] commented on code in PR #69969:
URL: https://github.com/apache/doris/pull/69969#discussion_r4440865136
##########
be/src/core/data_type_serde/complex_type_deserialize_util.h:
##########
@@ -91,12 +91,16 @@ struct ComplexTypeDeserializeUtil {
static Status process_column(const DataTypeSerDeSPtr& serde, IColumn&
column, StringRef& str,
const DataTypeSerDe::FormatOptions& options) {
DCHECK(column.is_nullable()) << "Column must be nullable but got " <<
column.get_name();
- if (is_null_string(str)) {
+ // JSON owns its quotes and lowercase null: trimming "123" changes its
type,
+ // and treating null as SQL NULL loses a JSON value. Only uppercase
NULL is
+ // the SQL-null marker for a JSON child, matching
Nullable(JSONB)::to_string.
+ const bool is_jsonb = serde->is_jsonb();
+ if (is_jsonb ? str == StringRef("NULL", 4) : is_null_string(str)) {
auto& column_nullable = assert_cast<ColumnNullable&>(column);
column_nullable.insert_default();
return Status::OK();
}
- auto str_without_quote = str.trim_quote();
+ auto str_without_quote = is_jsonb ? str : str.trim_quote();
Review Comment:
[P2] Preserve single-quoted JSON children in typed casts
Bypassing `trim_quote()` for every JSON token also drops the previously
accepted SQL single-quote wrapper. For example, when a STRING column `s`
contains `['{"a":1}']`, `CAST(s AS ARRAY<JSON>)` used to parse the object; this
branch now passes its surrounding apostrophes to `JsonbParser`. Non-strict
casts silently produce a SQL-NULL element, while strict casts fail. MAP/STRUCT
JSON children share the same path. Keep JSON's own double quotes while still
removing a matching outer apostrophe pair, and cover this input in both modes.
This is distinct from the repaired formatter round-trip thread: it regresses
user-supplied text accepted before the PR, in the newly changed reader.
##########
build.sh:
##########
@@ -1189,6 +1189,23 @@ function build_fe_modules() {
return 1
}
+function stamp_fe_build_version() {
+ local
version_java="${DORIS_HOME}/fe/fe-common/target/generated-sources/build/org/apache/doris/common/Version.java"
+ local version_classes="${DORIS_HOME}/fe/fe-common/target/build-version"
+ local common_jar
+
+ # A Maven cache hit can restore fe-common with another commit's
Version.class.
+ # Stamp only the deployed jar so volatile build metadata does not
invalidate the FE cache.
+ # Cached outputs can restore Version.java too, so regenerate after
restoration.
+ rm -f "${version_java}"
+ bash "${DORIS_HOME}/gensrc/script/gen_build_version.sh"
+ mkdir -p "${version_classes}"
+ "${JAVA_HOME}/bin/javac" --release 17 -encoding UTF-8 -d
"${version_classes}" "${version_java}"
Review Comment:
[P2] Preserve PATH-based JDK selection when stamping FE
`env.sh:284-295` explicitly supports `JAVA_HOME` and `JDK_17` both unset,
resolving and validating Java 17 through `PATH`; the version generator supports
that fallback too. In that setup this new command expands to `/bin/javac` (and
the invocation below to `/bin/jar`). A JDK installed only under a PATH entry
such as `/opt/jdk17/bin`, with no `/bin/javac`, therefore makes `./build.sh
--fe` fail during packaging after Maven has succeeded. Resolve these tools
consistently with the existing JDK selection, including its PATH fallback.
--
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]
