feat: Fix casting of invalid row positions to Json (facebookincubator#15828)

Summary:

When casting nested ROW types to JSON, castToJsonFromRow would process all child elements including those at null positions. Since Velox vectors can have invalid data at null positions (e.g., Timestamp with nanos > 1 billion), this could cause crashes during JSON serialization.

The fix is to filter out null rows from each child's SelectivityVector before creating AsJson. When all rows are null for a child, store std::nullopt and output "null" directly during rendering. This prevents accessing potentially invalid underlying data at null positions.

Reviewed By: Yuhta

Differential Revision: D89577815

Author: Krishna Pai

velox/functions/prestosql/tests/JsonCastTest.cpp

@@ -1837,3 +1837,130 @@ TEST_F(JsonCastTest, uniqueErrorContextMessage) {
       makeRowVector({makeNullableFlatVector<JsonNativeType>({"\"abc\""_sv})}),
       "Top-level Expression: cast((c0) as BIGINT)");
 }
+
+TEST_F(JsonCastTest, fromNestedRowWithNullInnerRowFieldNames) {
+  // Same test as above but with field names enabled in JSON output.
+  // Uses TIMESTAMP as the innermost type.
+  setFieldNamesInJsonCast(true);
+
+  // Inner type is now a TIMESTAMP
+  auto innerRowType = ROW({"ts"}, {TIMESTAMP()});
+  auto outerRowType = ROW({"x", "y"}, {innerRowType, DOUBLE()});
+  auto arrayType = ARRAY(outerRowType);
+
+  // Create timestamp values: epoch, 10M seconds after epoch, etc.
+  // Row 0: {ts: Timestamp{0, 0}} = "1970-01-01 00:00:00.000"
+  // Row 1: null (entire inner row is null) - will have bad nanos value
+  // Row 2: {ts: Timestamp{10000000, 0}} = "1970-04-26 17:46:40.000"
+  // Row 3: null (entire inner row is null) - will have bad nanos value
+  // Note: Rows 1 and 3 have invalid nanos (> 1,000,000,000) but since the
+  // entire inner row is null, these bad values should be ignored during
+  // JSON serialization.
+  auto innerRowChild = makeNullableFlatVector<Timestamp>(
+      {Timestamp{0, 0},
+       Timestamp{0, 0},
+       Timestamp{10000000, 0},
+       Timestamp{0, 0}});
+
+  // Set bad nanos values directly in the raw buffer for null positions.
+  // This bypasses Timestamp validation to test that nulls are properly handled.
+  auto* rawTimestamps =
+      innerRowChild->asFlatVector<Timestamp>()->mutableRawValues();
+  rawTimestamps[1] = Timestamp::fromMillisNoError(0);
+  reinterpret_cast<uint64_t*>(&rawTimestamps[1])[1] = 2000000000ULL;
+  rawTimestamps[3] = Timestamp::fromMillisNoError(0);
+  reinterpret_cast<uint64_t*>(&rawTimestamps[3])[1] = 5000000000ULL;
+
+  auto innerRowVector = makeRowVector({"ts"}, {innerRowChild});
+  innerRowVector->setNull(1, true);
+  innerRowVector->setNull(3, true);
+
+  auto outerRowChild2 = makeNullableFlatVector<double>({1.1, 2.2, 3.3, 4.4});
+  auto outerRowVector =
+      makeRowVector({"x", "y"}, {innerRowVector, outerRowChild2});
+
+  auto offsets = allocateOffsets(2, pool());
+  auto sizes = allocateSizes(2, pool());
+  auto rawOffsets = offsets->asMutable<vector_size_t>();
+  auto rawSizes = sizes->asMutable<vector_size_t>();
+  rawOffsets[0] = 0;
+  rawSizes[0] = 2;
+  rawOffsets[1] = 2;
+  rawSizes[1] = 2;
+
+  auto arrayVector = std::make_shared<ArrayVector>(
+      pool(), arrayType, nullptr, 2, offsets, sizes, outerRowVector);
+
+  // With field names enabled, output uses object notation:
+  // Row 0: [{"x":{"ts":"1970-01-01 00:00:00.000"},"y":1.1},{"x":null,"y":2.2}]
+  // Row 1: [{"x":{"ts":"1970-04-26 17:46:40.000"},"y":3.3},{"x":null,"y":4.4}]
+  auto expectedVector = makeNullableFlatVector<JsonNativeType>(
+      {R"([{"x":{"ts":"1970-01-01 00:00:00.000"},"y":1.1},{"x":null,"y":2.2}])"_sv,
+       R"([{"x":{"ts":"1970-04-26 17:46:40.000"},"y":3.3},{"x":null,"y":4.4}])"_sv},
+      JSON());
+
+  testCast(arrayVector, expectedVector);
+
+  setFieldNamesInJsonCast(false);
+}
+
+TEST_F(JsonCastTest, fromDeeplyNestedRowWithNulls) {
+  // Test ARRAY(ROW(ROW(ROW(...)))) - 3 levels of nesting with nulls at
+  // different levels.
+
+  // Level 3 (innermost): ROW(val: BIGINT)
+  auto level3Type = ROW({"val"}, {BIGINT()});
+
+  // Level 2: ROW(inner: level3Type)
+  auto level2Type = ROW({"inner"}, {level3Type});
+
+  // Level 1 (outermost row): ROW(mid: level2Type, extra: VARCHAR)
+  auto level1Type = ROW({"mid", "extra"}, {level2Type, VARCHAR()});
+
+  // Array type
+  auto arrayType = ARRAY(level1Type);
+
+  // Create level 3 vectors (4 elements, some null):
+  // [0]: {val: 100}
+  // [1]: null (entire level3 row is null)
+  // [2]: {val: 300}
+  // [3]: {val: 400}
+  auto level3Child =
+      makeNullableFlatVector<int64_t>({100, std::nullopt, 300, 400});
+  auto level3Vector = makeRowVector({"val"}, {level3Child});
+  level3Vector->setNull(1, true);
+
+  // Create level 2 vectors:
+  // [0]: {inner: {val: 100}}
+  // [1]: {inner: null}  <- level3 is null, but level2 is not null
+  // [2]: null  <- entire level2 row is null
+  // [3]: {inner: {val: 400}}
+  auto level2Vector = makeRowVector({"inner"}, {level3Vector});
+  level2Vector->setNull(2, true);
+
+  // Create level 1 vectors:
+  auto level1Child2 =
+      makeNullableFlatVector<StringView>({"a"_sv, "b"_sv, "c"_sv, "d"_sv});
+  auto level1Vector =
+      makeRowVector({"mid", "extra"}, {level2Vector, level1Child2});
+
+  // Create array with 2 rows, 2 elements each:
+  auto offsets = allocateOffsets(2, pool());
+  auto sizes = allocateSizes(2, pool());
+  auto rawOffsets = offsets->asMutable<vector_size_t>();
+  auto rawSizes = sizes->asMutable<vector_size_t>();
+  rawOffsets[0] = 0;
+  rawSizes[0] = 2;
+  rawOffsets[1] = 2;
+  rawSizes[1] = 2;
+
+  auto arrayVector = std::make_shared<ArrayVector>(
+      pool(), arrayType, nullptr, 2, offsets, sizes, level1Vector);
+
+  auto expectedVector = makeNullableFlatVector<JsonNativeType>(
+      {R"([[[[100]],"a"],[[null],"b"]])"_sv,
+       R"([[null,"c"],[[[400]],"d"]])"_sv},
+      JSON());
+
+  testCast(arrayVector, expectedVector);
+}

velox/functions/prestosql/types/JsonCastOperator.cpp

@@ -306,15 +306,28 @@ struct AsJson {
 
   // Appends the json string of the value at i to a string writer.
   void append(vector_size_t i, exec::StringWriter& proxy) const {
-    if (fieldName_.has_value()) {
-      proxy.append(fmt::format("\"{}\":", fieldName_.value()));
-    }
-
     if (decoded_->isNullAt(i)) {
-      proxy.append("null");
+      appendString(proxy, fieldName_);
     } else {
-      proxy.append(this->at(i));
+      appendString(proxy, fieldName_, this->at(i));
+    }
+  }
+
+  // Utility function to append string to a string writer.
+  static void appendString(
+      exec::StringWriter& proxy,
+      const std::optional<std::string>& fieldName) {
+    appendString(proxy, fieldName, "null"_sv);
+  }
+
+  static void appendString(
+      exec::StringWriter& proxy,
+      const std::optional<std::string>& fieldName,
+      const StringView& str) {
+    if (fieldName.has_value()) {
+      proxy.append(fmt::format("\"{}\":", fieldName.value()));
     }
+    proxy.append(str);
   }
 
  private:
@@ -556,7 +569,11 @@ void castToJsonFromRow(
     const SelectivityVector& rows,
     FlatVector<StringView>& flatResult,
     const std::shared_ptr<exec::CastHooks>& hooks) {
-  using NameJsonPair = std::pair<std::string, AsJson>;
+  // NameJsonPair also stores index of the child in the row vector.
+  // since we are sorting the children based on their field names, we need
+  // to store the index of the child in the row vector.
+  using NameJsonPair =
+      std::pair<std::pair<std::string, vector_size_t>, std::optional<AsJson>>;
   // input is guaranteed to be in flat encoding when passed in.
   VELOX_CHECK_EQ(input.encoding(), VectorEncoding::Simple::ROW);
   auto inputRow = input.as<RowVector>();
@@ -577,23 +594,43 @@ void castToJsonFromRow(
     std::optional<std::string> fieldName =
         fieldNamesInJsonCastEnabled ? std::optional{name} : std::nullopt;
 
-    jsonChildren.emplace_back(
-        name,
-        AsJson{
-            context,
-            inputRow->childAt(i),
-            rows,
-            nullptr,
-            hooks,
-            false,
-            std::move(fieldName)});
+    auto inputRowChild = inputRow->childAt(i);
+
+    // Use a pointer to avoid copying SelectivityVector when there are no nulls.
+    // Only create a new SelectivityVector when the child has nulls that need
+    // to be filtered out.
+    SelectivityVector childRowsHolder;
+    const SelectivityVector* childRowsPtr = &rows;
+    if (inputRowChild->mayHaveNulls()) {
+      childRowsHolder = rows;
+      childRowsHolder.deselectNulls(
+          inputRowChild->rawNulls(), rows.begin(), rows.end());
+      childRowsPtr = &childRowsHolder;
+    }
+    const SelectivityVector& childRows = *childRowsPtr;
+
+    if (childRows.hasSelections()) {
+      jsonChildren.emplace_back(
+          std::pair{name, i},
+          AsJson{
+              context,
+              inputRowChild,
+              childRows,
+              nullptr,
+              hooks,
+              false,
+              std::move(fieldName)});
+    } else {
+      // All rows are null for this child - store nullopt.
+      jsonChildren.emplace_back(std::pair{name, i}, std::nullopt);
+    }
 
     context.applyToSelectedNoThrow(rows, [&](auto row) {
-      if (inputRow->isNullAt(row)) {
+      if (inputRow->isNullAt(row) || inputRowChild->isNullAt(row)) {
         // "null" will be inlined in the StringView.
         return;
       }
-      childrenStringSize += jsonChildren[i].second.lengthAt(row);
+      childrenStringSize += jsonChildren[i].second->lengthAt(row);
     });
   }
 
@@ -634,7 +671,17 @@ void castToJsonFromRow(
       if (i > 0) {
         proxy.append(","_sv);
       }
-      jsonChildren[i].second.append(row, proxy);
+
+      auto nameIndex = jsonChildren[i].first;
+      if (!jsonChildren[i].second.has_value() ||
+          inputRow->childAt(nameIndex.second)->isNullAt(row)) {
+        AsJson::appendString(
+            proxy,
+            fieldNamesInJsonCastEnabled ? std::make_optional(nameIndex.first)
+                                        : std::nullopt);
+        continue;
+      }
+      jsonChildren[i].second->append(row, proxy);
     }
 
     if (fieldNamesInJsonCastEnabled) {