feat: Add Spark to_json function #11995

wecharyu · 2024-12-31T11:02:14Z

Support Spark to_json function: https://docs.databricks.com/en/sql/language-manual/functions/to_json.html

netlify · 2024-12-31T11:02:55Z

✅ Deploy Preview for meta-velox canceled.

Name	Link
🔨 Latest commit	`d468270`
🔍 Latest deploy log	https://app.netlify.com/sites/meta-velox/deploys/67c1313fc983e3000856e91d

zhouyuan · 2025-01-02T02:13:42Z

CC @rui-mo

assignUser

CMake 👍

rui-mo

Thanks! Added some initial comments.

rui-mo · 2025-01-08T02:21:16Z

velox/docs/functions/spark/json.rst

@@ -44,3 +44,14 @@ JSON Functions
        SELECT json_object_keys(''); -- NULL
        SELECT json_object_keys(1); -- NULL
        SELECT json_object_keys('"hello"'); -- NULL
+
+.. spark:function:: to_json(json_object) -> jsonString


nit: use camel-case naming style for 'json_object'.

rui-mo · 2025-01-08T02:33:24Z

velox/functions/sparksql/specialforms/ToJson.h

+
+namespace facebook::velox::functions::sparksql {
+
+class ToJsonCallToSpecialForm : public exec::FunctionCallToSpecialForm {


Since the result type is determined (varchar), we might don't need to implement 'to_json' as a special form. Special form is only used for the functions whose result type cannot be inferred from input type. For this case, we can implement it as a simple function.
More information on simple functions: https://github.com/facebookincubator/velox/blob/main/velox/docs/develop/scalar-functions.rst.

Remove the special form, pls take a look again.

rui-mo · 2025-02-11T14:51:51Z

velox/functions/sparksql/ToJson.cpp

+  });
+}
+
+class ToJsonFunction final : public exec::VectorFunction {


When implementing a function, simple function is preferred unless the implementation of vector function provides a significant performance gain which can be demonstrated with a benchmark.

Hi @wecharyu, thanks for the update! My comment is still about the way of implementation. As documented at https://github.com/facebookincubator/velox/blob/main/velox/docs/develop/scalar-functions.rst#vector-functions, simple function would be preferred if no benchmark to prove a large performance improvement. I'm wondering if there is any block issue you have met to prevent implementing it as simple function. Thanks.

Here I cannot find a way to register the Row input for different size of input schemas when writing it as a simple function. For example, for struct(bigint), I need register a function as follows:

registerFunction<ToJsonFunction, Varchar, Row<int64_t>>( {prefix + "to_json"});

and for other schemas like struct(bigint, string), I need register a new function. Is there a way to register a generic function for Row type?

I understand your point. It makes sense to me.

You may check this:

velox/velox/functions/sparksql/Comparisons.h

Line 179 in 5d0e276

// For arbitrary nested complex types.

rui-mo

Thanks!

rui-mo · 2025-02-17T14:00:38Z

velox/docs/functions/spark/json.rst

+    Converts a Json object (ROW, ARRAY or MAP) into a JSON string.
+    The current implementation has following limitations.
+
+    * Does not support user provided options. ::


Would you please provide an example for the unsupported case?

rui-mo · 2025-02-17T14:05:24Z

velox/functions/sparksql/ToJson.cpp

+    const SimpleVector<T>& input,
+    int row,
+    std::string& result,
+    const TypePtr& type) {


Perhaps add specilizations to handle different types.

rui-mo · 2025-02-17T14:05:45Z

velox/functions/sparksql/ToJson.cpp

+    result.data()[resultSize + 1] = '"';
+  } else if constexpr (std::is_same_v<T, UnknownValue>) {
+    VELOX_FAIL(
+        "Convert UNKNOWN to JSON: Vextors of UNKNOWN type should not contain non-null rows");


typo for Vextors

rui-mo · 2025-02-17T14:18:50Z

velox/functions/sparksql/ToJson.cpp

+      result.clear();
+      generateJsonTyped<T>(*inputVector, row, result, inputVector->type());
+
+      flatResult.set(row, StringView(result));


For the to-string functions, usually we can optimize by estimating total size, allocating the data buffer once and writing to the data buffer directly. This helps avoid generating intermediate strings and improve the performance. Some example:
https://github.com/facebookincubator/velox/blob/main/velox/expression/CastExpr-inl.h#L609-L624

rui-mo · 2025-02-17T14:19:52Z

velox/functions/sparksql/ToJson.cpp

+    });
+  }
+
+  // Extra length for commas and brackets


nit: document are full sentences and should end with .

rui-mo · 2025-02-17T14:20:37Z

velox/functions/sparksql/ToJson.cpp

+  size_t elementsStringSize = 0;
+  context.applyToSelectedNoThrow(rows, [&](auto row) {
+    if (inputArray->isNullAt(row)) {
+      // "null" will be inlined in the StringView.


ditto, please fix it throughout

rui-mo · 2025-02-17T14:23:21Z

Hi @zhli1142015, would you like to take a review? Thanks.

zhli1142015 · 2025-02-18T02:20:45Z

Hi @zhli1142015, would you like to take a review? Thanks.

Sure.

zhli1142015 · 2025-02-18T06:39:07Z

velox/functions/sparksql/registration/RegisterJson.cpp

@@ -16,6 +16,7 @@
 #include "velox/functions/lib/RegistrationHelpers.h"
 #include "velox/functions/sparksql/GetJsonObject.h"
 #include "velox/functions/sparksql/JsonObjectKeys.h"
+#include <expression/VectorFunction.h>


I think this include is not necessary.

zhli1142015 · 2025-02-18T06:49:08Z

velox/functions/sparksql/ToJson.cpp

+  auto value = input.valueAt(row);
+
+  if constexpr (std::is_same_v<T, StringView>) {
+    size_t resultSize = escapedStringSize(value.data(), value.size());


I think this function is not available, please sync,

sorry, do you mean we can not use this function here?

This function is not available in main branch, I think you need to find the replacement and update your code.

zhli1142015 · 2025-02-18T06:58:51Z

velox/functions/sparksql/tests/ToJsonTest.cpp

+       std::nullopt});
+  auto input = makeRowVector({"a"}, {data});
+  auto expected = makeFlatVector<std::string>(
+      {R"({"a":"1970-01-01T00:00:00.000000000"})",


I think timestamp in spark is with micros precision, can you confirm if this matches the spark output?

Spark would display the milliseconds by default, and can change the format by options. Since we do not support user provided options yet, we can force the Timestamp format to be the same as Spark default behavior.
https://github.com/apache/spark/blob/412da42ab2e91a643980634d032fec0738a59590/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/JsonExpressionsSuite.scala#L646-L659

zhli1142015 · 2025-02-18T07:17:04Z

velox/functions/sparksql/ToJson.cpp

+    result->clearNulls(rows);
+    auto* rawResults = result->as<FlatVector<StringView>>();
+
+    VELOX_DYNAMIC_TYPE_DISPATCH_ALL(


I think column names are not always passed from Spark to this function. Instead, col_XX is used as a placeholder. We may need to explicitly pass the column names.

zhli1142015 · 2025-02-18T07:18:36Z

velox/functions/sparksql/ToJson.cpp

+#include <functions/prestosql/json/JsonStringUtil.h>
+#include <type/Conversions.h>
+#include <type/DecimalUtil.h>
+#include <type/StringView.h>


Please remove unnessary include here.

zhli1142015 · 2025-02-18T07:27:31Z

velox/functions/sparksql/ToJson.cpp

+  static std::vector<std::shared_ptr<exec::FunctionSignature>> signatures() {
+    // T(ROW/ARRAY/MAP) -> varchar
+    return {exec::FunctionSignatureBuilder()
+                .typeVariable("T")


I think we only allow ROW/ARRAY/MAP types as input types, we should add some check

Add the input type check in apply() method.

We should check the type before calling apply. Please perform the check in the initialize method or the constructor.
Please follow either this or this.

zhli1142015 · 2025-02-18T07:35:09Z

velox/docs/functions/spark/json.rst

+
+.. spark:function:: to_json(jsonObject) -> jsonString
+
+    Converts a Json object (ROW, ARRAY or MAP) into a JSON string.


Please test your change with JsonFunctionsSuite and JsonExpressionsSuite, thanks.

feat: Add Spark to_json function

20a9a15

wecharyu requested review from assignUser and majetideepak as code owners December 31, 2024 11:02

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 31, 2024

assignUser approved these changes Jan 6, 2025

View reviewed changes

rui-mo reviewed Jan 8, 2025

View reviewed changes

boneanxs mentioned this pull request Jan 10, 2025

[VL] Unsupported spark function list [please leave a comment if you plan to pick some] apache/incubator-gluten#4039

Open

99 tasks

remove special form of ToJsonFunction

e0e7f28

wecharyu requested a review from rui-mo January 22, 2025 11:23

rui-mo reviewed Feb 11, 2025

View reviewed changes

rui-mo reviewed Feb 17, 2025

View reviewed changes

zhli1142015 reviewed Feb 18, 2025

View reviewed changes

wecharyu added 3 commits February 27, 2025 17:59

use buffer for string

fc6eedd

Merge remote-tracking branch 'facebook/main' into support_to_json

6539746

fix conflicts

8834290

wecharyu force-pushed the support_to_json branch from df9dbda to 8834290 Compare February 28, 2025 03:08

fix code format

d468270

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add Spark to_json function #11995

feat: Add Spark to_json function #11995

wecharyu commented Dec 31, 2024

netlify bot commented Dec 31, 2024 •

edited

Loading

zhouyuan commented Jan 2, 2025

assignUser left a comment

rui-mo left a comment

rui-mo Jan 8, 2025

rui-mo Jan 8, 2025

wecharyu Jan 22, 2025

rui-mo Feb 11, 2025

wecharyu Feb 13, 2025

rui-mo Feb 17, 2025

zhli1142015 Feb 28, 2025

rui-mo left a comment

rui-mo Feb 17, 2025

rui-mo Feb 17, 2025

rui-mo Feb 17, 2025

rui-mo Feb 17, 2025

rui-mo Feb 17, 2025

rui-mo Feb 17, 2025

rui-mo commented Feb 17, 2025

zhli1142015 commented Feb 18, 2025

zhli1142015 Feb 18, 2025

zhli1142015 Feb 18, 2025 •

edited

Loading

wecharyu Feb 27, 2025

zhli1142015 Feb 27, 2025

zhli1142015 Feb 18, 2025

wecharyu Feb 27, 2025

zhli1142015 Feb 18, 2025

zhli1142015 Feb 18, 2025

zhli1142015 Feb 18, 2025

wecharyu Feb 27, 2025

zhli1142015 Feb 28, 2025

zhli1142015 Feb 18, 2025


		namespace facebook::velox::functions::sparksql {

		class ToJsonCallToSpecialForm : public exec::FunctionCallToSpecialForm {


		.. spark:function:: to_json(jsonObject) -> jsonString

		Converts a Json object (ROW, ARRAY or MAP) into a JSON string.

feat: Add Spark to_json function #11995

Are you sure you want to change the base?

feat: Add Spark to_json function #11995

Conversation

wecharyu commented Dec 31, 2024

netlify bot commented Dec 31, 2024 • edited Loading

✅ Deploy Preview for meta-velox canceled.

zhouyuan commented Jan 2, 2025

assignUser left a comment

Choose a reason for hiding this comment

rui-mo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rui-mo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rui-mo commented Feb 17, 2025

zhli1142015 commented Feb 18, 2025

Choose a reason for hiding this comment

zhli1142015 Feb 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

netlify bot commented Dec 31, 2024 •

edited

Loading

zhli1142015 Feb 18, 2025 •

edited

Loading