fix

apache · Apr 22, 2024 · fb359af · fb359af
1 parent 4d9f699
commit fb359af
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 36 deletions.
diff --git a/be/src/vec/columns/column_object.cpp b/be/src/vec/columns/column_object.cpp
@@ -399,22 +399,30 @@ void ColumnObject::Subcolumn::insert(Field field, FieldInfo info) {
         value_dim = 0;
         type_changed = true;
     }
-    if (is_nullable && !is_nothing(base_type)) {
+    // Always expected nullable at present
+    if (!is_nothing(base_type)) {
         base_type = make_nullable(base_type);
     }
-
     const auto& least_common_base_type = least_common_type.get_base();
+    DCHECK(base_type->is_nullable() && least_common_base_type->is_nullable());
+    const auto& nested_common_base_type =
+            static_cast<const DataTypeNullable&>(*least_common_base_type).get_nested_type();
+    const auto& nested_base_type =
+            static_cast<const DataTypeNullable&>(*base_type).get_nested_type();
+
     if (data.empty()) {
         add_new_column_part(create_array_of_type(std::move(base_type), value_dim, is_nullable));
-    } else if (!least_common_base_type->equals(*base_type) && !is_nothing(base_type)) {
-        if (schema_util::is_conversion_required_between_integers(*base_type,
-                                                                 *least_common_base_type)) {
+    } else if (!nested_common_base_type->equals(*nested_base_type) &&
+               !is_nothing(nested_base_type)) {
+        if (schema_util::is_conversion_required_between_integers(*nested_base_type,
+                                                                 *nested_common_base_type)) {
+            LOG_EVERY_N(INFO, 100) << "Conversion between " << base_type->get_name() << " and "
+                                   << least_common_base_type->get_name();
             get_least_supertype<LeastSupertypeOnError::Jsonb>(
-                    DataTypes {std::move(base_type), least_common_base_type}, &base_type);
+                    DataTypes {std::move(nested_base_type), nested_common_base_type}, &base_type);
             type_changed = true;
-            if (is_nullable) {
-                base_type = make_nullable(base_type);
-            }
+            // Always expected nullable at present
+            base_type = make_nullable(base_type);
             if (!least_common_base_type->equals(*base_type)) {
                 add_new_column_part(
                         create_array_of_type(std::move(base_type), value_dim, is_nullable));

diff --git a/be/src/vec/common/schema_util.cpp b/be/src/vec/common/schema_util.cpp
@@ -114,31 +114,8 @@ bool is_conversion_required_between_integers(const IDataType& lhs, const IDataTy
     WhichDataType which_rhs(rhs);
     bool is_native_int = which_lhs.is_native_int() && which_rhs.is_native_int();
     bool is_native_uint = which_lhs.is_native_uint() && which_rhs.is_native_uint();
-    return (is_native_int || is_native_uint) &&
-           lhs.get_size_of_value_in_memory() <= rhs.get_size_of_value_in_memory();
-}
-
-bool is_conversion_required_between_integers(FieldType lhs, FieldType rhs) {
-    // We only support signed integers for semi-structure data at present
-    // TODO add unsigned integers
-    if (lhs == FieldType::OLAP_FIELD_TYPE_BIGINT) {
-        return !(rhs == FieldType::OLAP_FIELD_TYPE_TINYINT ||
-                 rhs == FieldType::OLAP_FIELD_TYPE_SMALLINT ||
-                 rhs == FieldType::OLAP_FIELD_TYPE_INT || rhs == FieldType::OLAP_FIELD_TYPE_BIGINT);
-    }
-    if (lhs == FieldType::OLAP_FIELD_TYPE_INT) {
-        return !(rhs == FieldType::OLAP_FIELD_TYPE_TINYINT ||
-                 rhs == FieldType::OLAP_FIELD_TYPE_SMALLINT ||
-                 rhs == FieldType::OLAP_FIELD_TYPE_INT);
-    }
-    if (lhs == FieldType::OLAP_FIELD_TYPE_SMALLINT) {
-        return !(rhs == FieldType::OLAP_FIELD_TYPE_TINYINT ||
-                 rhs == FieldType::OLAP_FIELD_TYPE_SMALLINT);
-    }
-    if (lhs == FieldType::OLAP_FIELD_TYPE_TINYINT) {
-        return !(rhs == FieldType::OLAP_FIELD_TYPE_TINYINT);
-    }
-    return true;
+    return (!is_native_int && !is_native_uint) ||
+           lhs.get_size_of_value_in_memory() > rhs.get_size_of_value_in_memory();
 }
 
 Status cast_column(const ColumnWithTypeAndName& arg, const DataTypePtr& type, ColumnPtr* result) {

diff --git a/be/src/vec/common/schema_util.h b/be/src/vec/common/schema_util.h
@@ -67,7 +67,6 @@ Status cast_column(const ColumnWithTypeAndName& arg, const DataTypePtr& type, Co
 /// is less than right type, we don't need to convert field,
 /// because all integer fields are stored in Int64/UInt64.
 bool is_conversion_required_between_integers(const IDataType& lhs, const IDataType& rhs);
-bool is_conversion_required_between_integers(FieldType lhs, FieldType rhs);
 
 struct ExtraInfo {
     // -1 indicates it's not a Frontend generated column

diff --git a/regression-test/suites/variant_p0/concurrent_insert.groovy b/regression-test/suites/variant_p0/concurrent_insert.groovy
@@ -49,6 +49,7 @@ suite("regression_test_variant_concurrent_schema_update", ""){
     t1.join()
     t2.join()
     t3.join()
-    qt_sql_1 "select * from ${table_name} order by k, cast(v as string), cast(v1 as string) limit 100"
+    qt_sql "sync"
+    qt_sql_1 "select * from var_concurrent order by k, cast(v as string), cast(v1 as string) limit 100"
     // qt_sql_3 """desc ${table_name}"""
 }