github-actions[bot] commented on code in PR #69969: URL: https://github.com/apache/doris/pull/69969#discussion_r4440213249
########## docker/thirdparties/docker-compose/mongodb/mongodb.yaml.tpl: ########## @@ -0,0 +1,50 @@ +# +# 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. +# + +name: doris--mongodb + +services: + mongodb: + image: mongo:8.0.20 + container_name: doris--mongodb + ports: + - "${MONGODB_PORT:-27017}:27017" + environment: + MONGO_INITDB_ROOT_USERNAME: mongodb_test + MONGO_INITDB_ROOT_PASSWORD: mongodb_test + volumes: + - ./init:/docker-entrypoint-initdb.d:ro + healthcheck: + test: + - CMD + - mongosh Review Comment: [P2] Require the final MongoDB server in the readiness probe `mongosh` defaults to loopback. The [pinned image entrypoint](https://github.com/docker-library/mongo/blob/2108dab3319c14e9862120659a0e59e384cb4c9d/docker-entrypoint.sh) creates the users and runs these fixtures on a temporary `mongod` bound to `127.0.0.1:27017`, then shuts it down and starts the final network-facing server. A probe after the final `numeric_limits` marker is inserted can authenticate and pass against that temporary server. Its successful Docker health state survives this internal process transition, so `compose up --wait` can return while the published endpoint is unavailable and subsequent suites race startup. Probe the container's network interface or explicitly require final-server readiness in addition to the fixture marker. ########## be/src/core/data_type_serde/data_type_jsonb_serde.cpp: ########## @@ -341,14 +341,10 @@ void DataTypeJsonbSerDe::to_string(const IColumn& column, size_t row_num, Buffer const auto& col = assert_cast<const ColumnString&, TypeCheckOnRelease::DISABLE>(column); const auto& data_ref = col.get_data_at(row_num); if (data_ref.size > 0) { - if (_nesting_level > 1) { - bw.write('"'); - } + // JSON text already quotes strings and escapes their contents. Wrapping a nested Review Comment: [P2] Update the typed complex reader with the JSON formatter `CAST(CAST(json_array AS STRING) AS ARRAY<JSONB>)` on the new `json_records` fixture (`seq = 1`) now turns the JSON string `"text"` and JSON `null` into SQL NULL (or raises a strict-cast error for the string). The unchanged `ComplexTypeDeserializeUtil::process_column` handles bare `null` as SQL NULL and strips surrounding quotes before calling the JSON serde. Previously one trim removed the extra nesting wrapper; now it removes the JSON string's own quotes, so even `"123"` becomes a JSON number. Coordinate the serializer and complex JSON-child parsing, with typed ARRAY/MAP/STRUCT round-trip cases; the new regression only reparses the formatted result as one JSON document. ########## docker/thirdparties/docker-compose/mongodb/mongodb.yaml.tpl: ########## @@ -0,0 +1,50 @@ +# +# 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. +# + +name: doris--mongodb + +services: + mongodb: + image: mongo:8.0.20 + container_name: doris--mongodb + ports: + - "${MONGODB_PORT:-27017}:27017" + environment: + MONGO_INITDB_ROOT_USERNAME: mongodb_test + MONGO_INITDB_ROOT_PASSWORD: mongodb_test + volumes: + - ./init:/docker-entrypoint-initdb.d:ro Review Comment: [P3] Clean up the fixture's anonymous data volumes The [MongoDB 8.0.20 image](https://github.com/docker-library/mongo/blob/2108dab3319c14e9862120659a0e59e384cb4c9d/8.0/Dockerfile#L111) declares `/data/db` and `/data/configdb` as volumes, while this fixture mounts only the init directory. Docker therefore creates anonymous data volumes. `start_mongodb` uses the common `down --remove-orphans` teardown without `-v`; [Compose preserves those volumes and does not remount them on the next `up`](https://docs.docker.com/reference/cli/docker/compose/down/). Every normal restart or `--stop` leaves the previous databases on the host. Give the disposable fixture an explicit storage lifecycle, such as resettable mounts or Mongo-specific volume removal during teardown. -- 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]
