xiaokang commented on code in PR #30603:
URL: https://github.com/apache/doris/pull/30603#discussion_r1479189220
##########
be/src/vec/functions/simple_function_factory.h:
##########
@@ -178,8 +179,12 @@ class SimpleFunctionFactory {
auto iter = function_creators.find(key_str);
if (iter == function_creators.end()) {
- LOG(WARNING) << fmt::format("Function signature {} is not found",
key_str);
- return nullptr;
+ // use original name as signature without variadic arguments
Review Comment:
What's the bad case?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java:
##########
@@ -1658,27 +1658,30 @@ &&
findOlapScanNodesByPassExchangeAndJoinNode(inputFragment.getPlanRoot())) {
}
// Get top most PushDownToProjectionFunction from expression
- private Expression getOriginalFunctionForRewritten(NamedExpression
expression) {
- List<Expression> targetExpr =
expression.collectFirst(PushDownToProjectionFunction.class::isInstance);
- if (!targetExpr.isEmpty()) {
- return targetExpr.get(0);
- }
- return null;
+ private List<Expression>
getPushDownToProjectionFunctionForRewritten(NamedExpression expression) {
Review Comment:
Why change return value to List?
##########
fe/fe-core/src/main/java/org/apache/doris/rewrite/ElementAtToSlotRefRule.java:
##########
@@ -42,12 +42,6 @@ public class ElementAtToSlotRefRule implements
ExprRewriteRule {
@Override
public Expr apply(Expr expr, Analyzer analyzer, ClauseType clauseType)
throws AnalysisException {
- // Only check element at of variant all rewrited to slots
Review Comment:
why delete
##########
be/src/olap/rowset/segment_v2/segment.cpp:
##########
@@ -446,11 +446,12 @@ Status Segment::new_column_iterator_with_path(const
TabletColumn& tablet_column,
return Status::OK();
}
bool output_as_raw_json = true;
+ // Not use path info in TabletColumn since the path info may not be
set in schema change process
+ vectorized::PathInData path(tablet_column.name_lower_case());
// Alter table operation should read the whole variant column, since
it does not aware of
Review Comment:
use root_path
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/FunctionBinder.java:
##########
@@ -205,15 +205,14 @@ public Expression visitElementAt(ElementAt elementAt,
ExpressionRewriteContext c
if (ConnectContext.get() != null
&& ConnectContext.get().getSessionVariable() != null
&&
!ConnectContext.get().getSessionVariable().isEnableRewriteElementAtToSlot()) {
- throw new AnalysisException(
Review Comment:
why delete Exception if session var is false?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java:
##########
@@ -1658,27 +1658,30 @@ &&
findOlapScanNodesByPassExchangeAndJoinNode(inputFragment.getPlanRoot())) {
}
// Get top most PushDownToProjectionFunction from expression
Review Comment:
change comment
##########
fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java:
##########
@@ -793,6 +795,9 @@ public class SessionVariable implements Serializable,
Writable {
@VariableMgr.VarAttr(name = SEND_BATCH_PARALLELISM, needForward = true)
public int sendBatchParallelism = 1;
+ @VariableMgr.VarAttr(name = ENABLE_VARIANT_ACCESS_IN_ORIGINAL_PLANNER)
+ public boolean enableVariantAccessInOriginalPlanner = false;
Review Comment:
Maybe we can delete support for old planner totally.
##########
be/src/vec/functions/function_variant_element.cpp:
##########
@@ -0,0 +1,178 @@
+// 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.
+
+#include <glog/logging.h>
+#include <stddef.h>
+
+#include <memory>
+#include <ostream>
+#include <string>
+#include <string_view>
+#include <utility>
+#include <vector>
+
+#include "common/status.h"
+#include "exprs/json_functions.h"
+#include "simdjson.h"
+#include "vec/columns/column.h"
+#include "vec/columns/column_nullable.h"
+#include "vec/columns/column_object.h"
+#include "vec/columns/column_string.h"
+#include "vec/common/assert_cast.h"
+#include "vec/common/string_ref.h"
+#include "vec/core/block.h"
+#include "vec/data_types/data_type.h"
+#include "vec/data_types/data_type_nothing.h"
+#include "vec/data_types/data_type_nullable.h"
+#include "vec/data_types/data_type_object.h"
+#include "vec/data_types/data_type_string.h"
+#include "vec/functions/function.h"
+#include "vec/functions/function_helpers.h"
+#include "vec/functions/simple_function_factory.h"
+
+namespace doris::vectorized {
+
+class FunctionVariantElement : public IFunction {
+public:
+ static constexpr auto name = "element_at";
+ static FunctionPtr create() { return
std::make_shared<FunctionVariantElement>(); }
+
+ // Get function name.
+ String get_name() const override { return name; }
+
+ bool use_default_implementation_for_nulls() const override { return true; }
+
+ size_t get_number_of_arguments() const override { return 2; }
+
+ ColumnNumbers get_arguments_that_are_always_constant() const override {
return {1}; }
+
+ DataTypes get_variadic_argument_types_impl() const override {
+ return {std::make_shared<vectorized::DataTypeObject>(),
std::make_shared<DataTypeString>()};
+ }
+
+ DataTypePtr get_return_type_impl(const DataTypes& arguments) const
override {
+
DCHECK((WhichDataType(remove_nullable(arguments[0]))).is_variant_type())
+ << "First argument for function: " << name
+ << " should be DataTypeObject but it has type " <<
arguments[0]->get_name() << ".";
+ DCHECK(is_string(arguments[1]))
+ << "Second argument for function: " << name << " should be
String but it has type "
+ << arguments[1]->get_name() << ".";
+ return make_nullable(std::make_shared<DataTypeObject>());
+ }
+
+ Status execute_impl(FunctionContext* context, Block& block, const
ColumnNumbers& arguments,
+ size_t result, size_t input_rows_count) const override
{
+ const auto* variant_col = check_and_get_column<ColumnObject>(
+ block.get_by_position(arguments[0]).column.get());
+ if (!variant_col) {
+ return Status::RuntimeError(
+ fmt::format("unsupported types for function {}({}, {})",
get_name(),
+
block.get_by_position(arguments[0]).type->get_name(),
+
block.get_by_position(arguments[1]).type->get_name()));
+ }
+ if (block.empty()) {
+ block.replace_by_position(result,
block.get_by_position(result).type->create_column());
+ return Status::OK();
+ }
+
+ auto index_column = block.get_by_position(arguments[1]).column;
+ ColumnPtr result_column;
+ RETURN_IF_ERROR(get_element_column(*variant_col, index_column,
&result_column));
+ block.replace_by_position(result, result_column);
+ return Status::OK();
+ }
+
+private:
+ static Status get_element_column(const ColumnObject& src, const ColumnPtr&
index_column,
+ ColumnPtr* result) {
+ std::string field_name = index_column->get_data_at(0).to_string();
+ if (src.empty()) {
+ *result = ColumnObject::create(true);
+ return Status::OK();
+ }
+ if (src.is_scalar_variant() &&
+
WhichDataType(remove_nullable(src.get_root_type())).is_string_or_fixed_string())
{
Review Comment:
variant column should be string?
--
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]
