Skip to content

Commit 0e86a0a

Browse files
zhangxffffzhangxffff
authored
fix: support narrow to TINYINT/SMALLINT for with physical_type INT32 to compatible with legacy parquet (#584)
1 parent 296fa0b commit 0e86a0a

2 files changed

Lines changed: 88 additions & 69 deletions

File tree

‎bolt/dwio/parquet/reader/ParquetReader.cpp‎

Lines changed: 39 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -74,32 +74,6 @@ ::parquet::EncryptionAlgorithm FromThrift(
7474
constexpr const char* kTypeMappingErrorFmtStr =
7575
"Schema mismatch, Column: [{}], From Kind: {}, To Kind: {}";
7676

77-
// Checks whether 'type' is an integer type at least as wide as
78-
// 'minTypeKind'. Used to gate integer widening:
79-
// minTypeKind=TINYINT accepts TINYINT/SMALLINT/INTEGER/BIGINT (INT_8 leaf)
80-
// minTypeKind=SMALLINT accepts SMALLINT/INTEGER/BIGINT (INT_16 leaf)
81-
// minTypeKind=INTEGER accepts INTEGER/BIGINT (INT_32 leaf)
82-
// minTypeKind=BIGINT accepts BIGINT only (INT_64 leaf)
83-
bool isIntCompatible(
84-
const bytedance::bolt::TypePtr& type,
85-
bytedance::bolt::TypeKind minTypeKind) {
86-
using TK = bytedance::bolt::TypeKind;
87-
static_assert(
88-
static_cast<int>(TK::TINYINT) < static_cast<int>(TK::SMALLINT) &&
89-
static_cast<int>(TK::SMALLINT) < static_cast<int>(TK::INTEGER) &&
90-
static_cast<int>(TK::INTEGER) < static_cast<int>(TK::BIGINT));
91-
const auto kind = type->kind();
92-
switch (kind) {
93-
case TK::TINYINT:
94-
case TK::SMALLINT:
95-
case TK::INTEGER:
96-
case TK::BIGINT:
97-
return static_cast<int>(kind) >= static_cast<int>(minTypeKind);
98-
default:
99-
return false;
100-
}
101-
}
102-
10377
// Predicates for the cross-family implicit cast that the column reader
10478
// layer performs at read time (Spark/Hive STRING<->INT compatibility):
10579
// - StringColumnReader::makeCastExpr handles VARCHAR/VARBINARY file ->
@@ -111,6 +85,11 @@ bool isIntCompatible(
11185
// still get the strict check at ParquetColumnReader.cpp:95 via
11286
// matchType() for the VARCHAR-file -> INT-requested direction, which
11387
// fires after convertType and throws with its existing error message.
88+
// Cross-family implicit cast for VARCHAR/VARBINARY file columns:
89+
// StringColumnReader::makeCastExpr handles VARCHAR/VARBINARY file -> int
90+
// family requested type. Non-Spark builds also get a strict check at
91+
// ParquetColumnReader.cpp:95 via matchType() that fires after convertType
92+
// and throws with its own error message for that case.
11493
bool acceptsVarcharFileForReaderCast(const bytedance::bolt::TypePtr& t) {
11594
using TK = bytedance::bolt::TypeKind;
11695
const auto k = t->kind();
@@ -122,6 +101,30 @@ bool acceptsIntFileForReaderCast(const bytedance::bolt::TypePtr& t) {
122101
t->kind() == bytedance::bolt::TypeKind::VARBINARY;
123102
}
124103

104+
// Compatibility predicate for Parquet INT32-physical source columns
105+
// (INT_8 / INT_16 / INT_32 / UINT_* annotated, plus unannotated INT32).
106+
// Accepts:
107+
// - Any int-family target. Narrowing (file INT32 -> requested ByteType /
108+
// ShortType) is silently truncated at read time, matching Spark's
109+
// vectorized reader. Covers HIVE-14294 where Hive 1.x writes
110+
// TINYINT/SMALLINT as unannotated INT32 (SPARK-16632).
111+
// - VARCHAR / VARBINARY target via IntegerColumnReader::makeCastExpr,
112+
// which performs the int-to-string cast at read time.
113+
bool isInt32Compatible(const bytedance::bolt::TypePtr& type) {
114+
using TK = bytedance::bolt::TypeKind;
115+
const auto k = type->kind();
116+
return k == TK::TINYINT || k == TK::SMALLINT || k == TK::INTEGER ||
117+
k == TK::BIGINT || acceptsIntFileForReaderCast(type);
118+
}
119+
120+
// Compatibility predicate for Parquet INT64-physical source columns.
121+
// Accepts BIGINT only (no integer narrowing), plus VARCHAR / VARBINARY
122+
// via IntegerColumnReader::makeCastExpr.
123+
bool isInt64Compatible(const bytedance::bolt::TypePtr& type) {
124+
using TK = bytedance::bolt::TypeKind;
125+
return type->kind() == TK::BIGINT || acceptsIntFileForReaderCast(type);
126+
}
127+
125128
} // namespace
126129

127130
/// Metadata and options for reading Parquet.
@@ -1023,87 +1026,63 @@ TypePtr ReaderBase::convertType(
10231026
schemaElement.type,
10241027
thrift::Type::INT32,
10251028
"INT8 converted type can only be set for value of thrift::Type::INT32");
1026-
checkRequested([](const TypePtr& t) {
1027-
return isIntCompatible(t, TypeKind::TINYINT) ||
1028-
acceptsIntFileForReaderCast(t);
1029-
});
1029+
checkRequested([](const TypePtr& t) { return isInt32Compatible(t); });
10301030
return TINYINT();
10311031

10321032
case thrift::ConvertedType::INT_16:
10331033
BOLT_CHECK_EQ(
10341034
schemaElement.type,
10351035
thrift::Type::INT32,
10361036
"INT16 converted type can only be set for value of thrift::Type::INT32");
1037-
checkRequested([](const TypePtr& t) {
1038-
return isIntCompatible(t, TypeKind::SMALLINT) ||
1039-
acceptsIntFileForReaderCast(t);
1040-
});
1037+
checkRequested([](const TypePtr& t) { return isInt32Compatible(t); });
10411038
return SMALLINT();
10421039

10431040
case thrift::ConvertedType::INT_32:
10441041
BOLT_CHECK_EQ(
10451042
schemaElement.type,
10461043
thrift::Type::INT32,
10471044
"INT32 converted type can only be set for value of thrift::Type::INT32");
1048-
checkRequested([](const TypePtr& t) {
1049-
return isIntCompatible(t, TypeKind::INTEGER) ||
1050-
acceptsIntFileForReaderCast(t);
1051-
});
1045+
checkRequested([](const TypePtr& t) { return isInt32Compatible(t); });
10521046
return INTEGER();
10531047

10541048
case thrift::ConvertedType::INT_64:
10551049
BOLT_CHECK_EQ(
10561050
schemaElement.type,
10571051
thrift::Type::INT64,
10581052
"INT64 converted type can only be set for value of thrift::Type::INT64");
1059-
checkRequested([](const TypePtr& t) {
1060-
return isIntCompatible(t, TypeKind::BIGINT) ||
1061-
acceptsIntFileForReaderCast(t);
1062-
});
1053+
checkRequested([](const TypePtr& t) { return isInt64Compatible(t); });
10631054
return BIGINT();
10641055

10651056
case thrift::ConvertedType::UINT_8:
10661057
BOLT_CHECK_EQ(
10671058
schemaElement.type,
10681059
thrift::Type::INT32,
10691060
"UINT_8 converted type can only be set for value of thrift::Type::INT32");
1070-
checkRequested([](const TypePtr& t) {
1071-
return isIntCompatible(t, TypeKind::TINYINT) ||
1072-
acceptsIntFileForReaderCast(t);
1073-
});
1061+
checkRequested([](const TypePtr& t) { return isInt32Compatible(t); });
10741062
return TINYINT();
10751063

10761064
case thrift::ConvertedType::UINT_16:
10771065
BOLT_CHECK_EQ(
10781066
schemaElement.type,
10791067
thrift::Type::INT32,
10801068
"UINT_16 converted type can only be set for value of thrift::Type::INT32");
1081-
checkRequested([](const TypePtr& t) {
1082-
return isIntCompatible(t, TypeKind::SMALLINT) ||
1083-
acceptsIntFileForReaderCast(t);
1084-
});
1069+
checkRequested([](const TypePtr& t) { return isInt32Compatible(t); });
10851070
return SMALLINT();
10861071

10871072
case thrift::ConvertedType::UINT_32:
10881073
BOLT_CHECK_EQ(
10891074
schemaElement.type,
10901075
thrift::Type::INT32,
10911076
"UINT_32 converted type can only be set for value of thrift::Type::INT32");
1092-
checkRequested([](const TypePtr& t) {
1093-
return isIntCompatible(t, TypeKind::INTEGER) ||
1094-
acceptsIntFileForReaderCast(t);
1095-
});
1077+
checkRequested([](const TypePtr& t) { return isInt32Compatible(t); });
10961078
return INTEGER();
10971079

10981080
case thrift::ConvertedType::UINT_64:
10991081
BOLT_CHECK_EQ(
11001082
schemaElement.type,
11011083
thrift::Type::INT64,
11021084
"UINT_64 converted type can only be set for value of thrift::Type::INT64");
1103-
checkRequested([](const TypePtr& t) {
1104-
return isIntCompatible(t, TypeKind::BIGINT) ||
1105-
acceptsIntFileForReaderCast(t);
1106-
});
1085+
checkRequested([](const TypePtr& t) { return isInt64Compatible(t); });
11071086
return BIGINT();
11081087

11091088
case thrift::ConvertedType::DATE:
@@ -1188,10 +1167,7 @@ TypePtr ReaderBase::convertType(
11881167
[](const TypePtr& t) { return t->kind() == TypeKind::BOOLEAN; });
11891168
return BOOLEAN();
11901169
case thrift::Type::type::INT32:
1191-
checkRequested([](const TypePtr& t) {
1192-
return isIntCompatible(t, TypeKind::INTEGER) ||
1193-
acceptsIntFileForReaderCast(t);
1194-
});
1170+
checkRequested([](const TypePtr& t) { return isInt32Compatible(t); });
11951171
return INTEGER();
11961172
case thrift::Type::type::INT64:
11971173
// For Int64 Timestamp in nano precision
@@ -1212,10 +1188,7 @@ TypePtr ReaderBase::convertType(
12121188
});
12131189
return TIMESTAMP();
12141190
}
1215-
checkRequested([](const TypePtr& t) {
1216-
return isIntCompatible(t, TypeKind::BIGINT) ||
1217-
acceptsIntFileForReaderCast(t);
1218-
});
1191+
checkRequested([](const TypePtr& t) { return isInt64Compatible(t); });
12191192
return BIGINT();
12201193
case thrift::Type::type::INT96:
12211194
checkRequested(

‎bolt/dwio/parquet/tests/reader/ParquetTableScanTest.cpp‎

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1333,10 +1333,21 @@ TEST_F(ParquetTableScanTest, convertTypePolicyMatrix) {
13331333
{"array_declared_vs_varchar_file", VARCHAR(), ARRAY(VARCHAR()), true},
13341334
{"array_declared_vs_bigint_file", BIGINT(), ARRAY(BIGINT()), true},
13351335

1336-
// ---- Reject: integer narrowing ----
1336+
// ---- Reject: INT64 narrowing (both flavours; Spark INT64 reader
1337+
// also rejects Byte/Short/Integer requests). ----
13371338
{"bigint_to_integer", BIGINT(), INTEGER(), true},
1338-
{"integer_to_smallint", INTEGER(), SMALLINT(), true},
1339-
{"integer_to_tinyint", INTEGER(), TINYINT(), true},
1339+
{"bigint_to_smallint", BIGINT(), SMALLINT(), true},
1340+
{"bigint_to_tinyint", BIGINT(), TINYINT(), true},
1341+
1342+
// ---- Accept: INT32 narrowing. File INT32 -> requested Byte/Short
1343+
// is silently truncated at read time by IntegerColumnReader,
1344+
// matching Spark's vectorized reader (ByteUpdater/ShortUpdater)
1345+
// and covering HIVE-14294 / SPARK-16632 where Hive 1.x writes
1346+
// TINYINT/SMALLINT as unannotated INT32. See the
1347+
// integer-narrowing block in convertTypePolicyValueChecks
1348+
// for end-to-end values. ----
1349+
{"integer_to_smallint", INTEGER(), SMALLINT(), false},
1350+
{"integer_to_tinyint", INTEGER(), TINYINT(), false},
13401351

13411352
// ---- Reject: cross-family with no column-reader auto-cast ----
13421353
{"integer_to_double", INTEGER(), DOUBLE(), true},
@@ -1535,6 +1546,41 @@ TEST_F(ParquetTableScanTest, convertTypePolicyValueChecks) {
15351546
}
15361547
#endif
15371548

1549+
// 3a/3b. INT32 narrowing: file INT32 read back as TINYINT and SMALLINT.
1550+
// Models HIVE-14294 / SPARK-16632 where Hive 1.x writes TINYINT/
1551+
// SMALLINT as unannotated INT32. The matching matrix cases only
1552+
// assert no-throw; these check the data path actually narrows
1553+
// correctly instead of returning garbage. IntegerColumnReader
1554+
// does the silent truncation, matching Spark / Trino / parquet-mr.
1555+
{
1556+
auto data = makeRowVector({"c0"}, {makeFlatVector<int32_t>({1, 2, 3})});
1557+
auto file = exec::test::TempFilePath::create();
1558+
writeToParquetFile(file->getPath(), {data}, WriterOptions{});
1559+
1560+
{
1561+
auto declared = ROW({"c0"}, {TINYINT()});
1562+
auto plan =
1563+
PlanBuilder(pool()).tableScan(declared, {}, "", declared).planNode();
1564+
auto result = AssertQueryBuilder(plan)
1565+
.split(makeSplit(file->getPath()))
1566+
.copyResults(pool());
1567+
auto expected =
1568+
makeRowVector({"c0"}, {makeFlatVector<int8_t>({1, 2, 3})});
1569+
EXPECT_TRUE(assertEqualResults({expected}, {result}));
1570+
}
1571+
{
1572+
auto declared = ROW({"c0"}, {SMALLINT()});
1573+
auto plan =
1574+
PlanBuilder(pool()).tableScan(declared, {}, "", declared).planNode();
1575+
auto result = AssertQueryBuilder(plan)
1576+
.split(makeSplit(file->getPath()))
1577+
.copyResults(pool());
1578+
auto expected =
1579+
makeRowVector({"c0"}, {makeFlatVector<int16_t>({1, 2, 3})});
1580+
EXPECT_TRUE(assertEqualResults({expected}, {result}));
1581+
}
1582+
}
1583+
15381584
// 4. Auto-cast INTEGER -> VARCHAR: [1,2,3] -> ["1","2","3"].
15391585
// Works in both build flavours: matchType() lets INT fileType fall
15401586
// through, and IntegerColumnReader::makeCastExpr handles the cast.

0 commit comments

Comments
 (0)