github-actions[bot] commented on code in PR #60910:
URL: https://github.com/apache/doris/pull/60910#discussion_r2875880797
##########
be/src/vec/functions/function_fake.cpp:
##########
@@ -151,6 +151,34 @@ struct FunctionExplodeJsonObject {
static std::string get_error_msg() { return "Fake function do not support
execute"; }
};
+// json_each(json) -> Nullable(Struct(key Nullable(String), value
Nullable(JSONB)))
+struct FunctionJsonEach {
+ static DataTypePtr get_return_type_impl(const DataTypes& arguments) {
+ DCHECK_EQ(arguments[0]->get_primitive_type(),
PrimitiveType::TYPE_JSONB)
+ << " json_each " << arguments[0]->get_name() << " not
supported";
Review Comment:
Nit: `DCHECK_EQ` vanishes in RELEASE builds, leaving no argument type
validation. Per coding standards, `DORIS_CHECK` is preferred for invariant
assertions. This is a pre-existing pattern from `FunctionExplodeJsonObject`
(line 143), but new code should prefer `DORIS_CHECK`.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/BuiltinTableGeneratingFunctions.java:
##########
@@ -63,6 +65,8 @@ public class BuiltinTableGeneratingFunctions implements
FunctionHelper {
tableGenerating(ExplodeMapOuter.class, "explode_map_outer"),
tableGenerating(ExplodeJsonObject.class, "explode_json_object"),
tableGenerating(ExplodeJsonObjectOuter.class,
"explode_json_object_outer"),
+ tableGenerating(JsonEach.class, "json_each"),
+ tableGenerating(JsonEachText.class, "json_each_text"),
tableGenerating(ExplodeNumbers.class, "explode_numbers"),
Review Comment:
Nit: This line has extra indentation (20 spaces) compared to the adjacent
`tableGenerating(JsonEach.class, ...)` line above (12 spaces). Should be
aligned consistently:
```java
tableGenerating(JsonEach.class, "json_each"),
tableGenerating(JsonEachText.class, "json_each_text"),
```
##########
be/src/vec/functions/function_fake.cpp:
##########
@@ -239,6 +267,8 @@ void register_function_fake(SimpleFunctionFactory& factory)
{
register_table_function_expand_outer<FunctionExplodeMap>(factory,
"explode_map");
register_table_function_expand_outer<FunctionExplodeJsonObject>(factory,
"explode_json_object");
+ register_function<FunctionJsonEach>(factory, "json_each");
+ register_function<FunctionJsonEachText>(factory, "json_each_text");
Review Comment:
**Missing outer variant registration**: `explode_json_object` is registered
with `register_table_function_expand_outer` which automatically registers both
the base and `_outer` variants. `json_each` and `json_each_text` are registered
with plain `register_function`, meaning no `json_each_outer` /
`json_each_text_outer` variants exist.
Every other `explode_*` table function has an `_outer` variant. Without it,
users cannot get left-join semantics (preserving input rows when the function
generates zero rows). Consider:
```cpp
register_table_function_expand_outer<FunctionJsonEach>(factory, "json_each");
register_table_function_expand_outer<FunctionJsonEachText>(factory,
"json_each_text");
```
This also requires adding the corresponding FE classes
(`JsonEachOuter.java`, `JsonEachTextOuter.java`), visitor methods, builtin
registrations, and BE factory entries.
--
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]
