Fix bad code

alexey-milovidov · alexey-milovidov · commit c51c2656ebc0 · 2020-06-23T20:39:30.000+03:00
diff --git a/src/DataTypes/getLeastSupertype.cpp b/src/DataTypes/getLeastSupertype.cpp
@@ -208,7 +208,7 @@ DataTypePtr getLeastSupertype(const DataTypes & types)
         }
     }
 
-    //github.com/ For Date and DateTime, the common type is DateTime. No other types are compatible.
+    //github.com/ For Date and DateTime/DateTime64, the common type is DateTime/DateTime64. No other types are compatible.
     {
         UInt32 have_date = type_ids.count(TypeIndex::Date);
         UInt32 have_datetime = type_ids.count(TypeIndex::DateTime);
@@ -218,40 +218,25 @@ DataTypePtr getLeastSupertype(const DataTypes & types)
         {
             bool all_date_or_datetime = type_ids.size() == (have_date + have_datetime + have_datetime64);
             if (!all_date_or_datetime)
-                throw Exception(getExceptionMessagePrefix(types) + " because some of them are Date/DateTime and some of them are not", ErrorCodes::NO_COMMON_TYPE);
+                throw Exception(getExceptionMessagePrefix(types) + " because some of them are Date/DateTime/DateTime64 and some of them are not",
+                    ErrorCodes::NO_COMMON_TYPE);
 
             if (have_datetime64 == 0)
-            {
                 return std::make_shared<DataTypeDateTime>();
-            }
 
-            // When DateTime64 involved, make sure that supertype has whole-part precision
-            // big enough to hold max whole-value of any type from `types`.
-            // That would sacrifice scale when comparing DateTime64 of different scales.
+            UInt8 max_scale = 0;
 
-            UInt32 max_datetime64_whole_precision = 0;
             for (const auto & t : types)
             {
                 if (const auto * dt64 = typeid_cast<const DataTypeDateTime64 *>(t.get()))
                 {
-                    const auto whole_precision = dt64->getPrecision() - dt64->getScale();
-                    max_datetime64_whole_precision = std::max(whole_precision, max_datetime64_whole_precision);
+                    const auto scale = dt64->getScale();
+                    if (scale > max_scale)
+                        max_scale = scale;
                 }
             }
 
-            UInt32 least_decimal_precision = 0;
-            if (have_datetime)
-            {
-                least_decimal_precision = leastDecimalPrecisionFor(TypeIndex::UInt32);
-            }
-            else if (have_date)
-            {
-                least_decimal_precision = leastDecimalPrecisionFor(TypeIndex::UInt16);
-            }
-            max_datetime64_whole_precision = std::max(least_decimal_precision, max_datetime64_whole_precision);
-
-            const UInt32 scale = DataTypeDateTime64::maxPrecision() - max_datetime64_whole_precision;
-            return std::make_shared<DataTypeDateTime64>(scale);
+            return std::make_shared<DataTypeDateTime64>(max_scale);
         }
     }
 
diff --git a/src/DataTypes/tests/gtest_data_type_get_common_type.cpp b/src/DataTypes/tests/gtest_data_type_get_common_type.cpp
@@ -86,7 +86,7 @@ TEST_P(LeastSuperTypeTest, getLeastSupertype)
 
 class MostSubtypeTest : public TypeTest {};
 
-TEST_P(MostSubtypeTest, getLeastSupertype)
+TEST_P(MostSubtypeTest, getMostSubtype)
 {
     if (this->expected_type)
     {
@@ -124,9 +124,7 @@ INSTANTIATE_TEST_SUITE_P(data_type,
             {"Date DateTime64(3)", "DateTime64(3)"},
             {"DateTime DateTime64(3)", "DateTime64(3)"},
             {"DateTime DateTime64(0)", "DateTime64(0)"},
-            {"DateTime64(9) DateTime64(3)", "DateTime64(3)"},
-            {"DateTime DateTime64(12)", "DateTime64(8)"},
-            {"Date DateTime64(15)", "DateTime64(13)"},
+            {"DateTime64(9) DateTime64(3)", "DateTime64(9)"},
 
             {"String FixedString(32) FixedString(8)", "String"},
 

Original file line number	Diff line number	Diff line change
`@@ -208,7 +208,7 @@ DataTypePtr getLeastSupertype(const DataTypes & types)`
`208`	`208`	`}`
`209`	`209`	`}`
`210`	`210`
`211`		`- /// For Date and DateTime, the common type is DateTime. No other types are compatible.`
	`211`	`+ /// For Date and DateTime/DateTime64, the common type is DateTime/DateTime64. No other types are compatible.`
`212`	`212`	`{`
`213`	`213`	`UInt32 have_date = type_ids.count(TypeIndex::Date);`
`214`	`214`	`UInt32 have_datetime = type_ids.count(TypeIndex::DateTime);`
`@@ -218,40 +218,25 @@ DataTypePtr getLeastSupertype(const DataTypes & types)`
`218`	`218`	`{`
`219`	`219`	`bool all_date_or_datetime = type_ids.size() == (have_date + have_datetime + have_datetime64);`
`220`	`220`	`if (!all_date_or_datetime)`
`221`		`- throw Exception(getExceptionMessagePrefix(types) + " because some of them are Date/DateTime and some of them are not", ErrorCodes::NO_COMMON_TYPE);`
	`221`	`+ throw Exception(getExceptionMessagePrefix(types) + " because some of them are Date/DateTime/DateTime64 and some of them are not",`
	`222`	`+ ErrorCodes::NO_COMMON_TYPE);`
`222`	`223`
`223`	`224`	`if (have_datetime64 == 0)`
`224`		`- {`
`225`	`225`	`return std::make_shared<DataTypeDateTime>();`
`226`		`- }`
`227`	`226`
`228`		`- // When DateTime64 involved, make sure that supertype has whole-part precision`
`229`		- // big enough to hold max whole-value of any type from `types`.
`230`		`- // That would sacrifice scale when comparing DateTime64 of different scales.`
	`227`	`+ UInt8 max_scale = 0;`
`231`	`228`
`232`		`- UInt32 max_datetime64_whole_precision = 0;`
`233`	`229`	`for (const auto & t : types)`
`234`	`230`	`{`
`235`	`231`	`if (const auto * dt64 = typeid_cast<const DataTypeDateTime64 *>(t.get()))`
`236`	`232`	`{`
`237`		`- const auto whole_precision = dt64->getPrecision() - dt64->getScale();`
`238`		`- max_datetime64_whole_precision = std::max(whole_precision, max_datetime64_whole_precision);`
	`233`	`+ const auto scale = dt64->getScale();`
	`234`	`+ if (scale > max_scale)`
	`235`	`+ max_scale = scale;`
`239`	`236`	`}`
`240`	`237`	`}`
`241`	`238`
`242`		`- UInt32 least_decimal_precision = 0;`
`243`		`- if (have_datetime)`
`244`		`- {`
`245`		`- least_decimal_precision = leastDecimalPrecisionFor(TypeIndex::UInt32);`
`246`		`- }`
`247`		`- else if (have_date)`
`248`		`- {`
`249`		`- least_decimal_precision = leastDecimalPrecisionFor(TypeIndex::UInt16);`
`250`		`- }`
`251`		`- max_datetime64_whole_precision = std::max(least_decimal_precision, max_datetime64_whole_precision);`
`252`		`-`
`253`		`- const UInt32 scale = DataTypeDateTime64::maxPrecision() - max_datetime64_whole_precision;`
`254`		`- return std::make_shared<DataTypeDateTime64>(scale);`
	`239`	`+ return std::make_shared<DataTypeDateTime64>(max_scale);`
`255`	`240`	`}`
`256`	`241`	`}`
`257`	`242`