From f1e401505ac20c0400eec819b9196f7f506fb927 Mon Sep 17 00:00:00 2001 From: Tim Armstrong Date: Sun, 29 Jul 2018 23:55:12 -0700 Subject: [PATCH] WIP Won't work for Kudu - need to add padding. --- be/src/runtime/collection-value.h | 2 +- be/src/runtime/descriptors.cc | 2 +- be/src/runtime/string-value.h | 2 +- be/src/runtime/timestamp-value.h | 2 +- be/src/runtime/types.h | 5 +++-- be/src/util/static-asserts.cc | 4 ++-- fe/src/main/java/org/apache/impala/catalog/PrimitiveType.java | 7 +++---- fe/src/main/java/org/apache/impala/catalog/Type.java | 4 +--- 8 files changed, 13 insertions(+), 15 deletions(-) diff --git a/be/src/runtime/collection-value.h b/be/src/runtime/collection-value.h index 29ecb17..4681167 100644 --- a/be/src/runtime/collection-value.h +++ b/be/src/runtime/collection-value.h @@ -27,7 +27,7 @@ namespace impala { /// arrays and maps are effectively indistinguishable; a map can be thought of as an array /// of key/value structs (and neither of these fields are necessarily materialized in the /// item tuples). -struct CollectionValue { +struct __attribute__ ((__packed__)) CollectionValue { /// Pointer to buffer containing item tuples. uint8_t* ptr; diff --git a/be/src/runtime/descriptors.cc b/be/src/runtime/descriptors.cc index 12de054..71b6acd 100644 --- a/be/src/runtime/descriptors.cc +++ b/be/src/runtime/descriptors.cc @@ -742,7 +742,7 @@ llvm::StructType* TupleDescriptor::GetLlvmStruct(LlvmCodeGen* codegen) const { for (SlotDescriptor* slot: slots()) { // Verify that the byte offset in the llvm struct matches the tuple offset // computed in the FE. - DCHECK_EQ(layout->getElementOffset(slot->llvm_field_idx()), slot->tuple_offset()); + DCHECK_EQ(layout->getElementOffset(slot->llvm_field_idx()), slot->tuple_offset()) << id_; } return tuple_struct; } diff --git a/be/src/runtime/string-value.h b/be/src/runtime/string-value.h index 5b7a7e4..5fb0292 100644 --- a/be/src/runtime/string-value.h +++ b/be/src/runtime/string-value.h @@ -33,7 +33,7 @@ namespace impala { /// The returned StringValue of all functions that return StringValue /// shares its buffer with the parent. /// TODO: rename this to be less confusing with impala_udf::StringVal. -struct StringValue { +struct __attribute__ ((__packed__)) StringValue { /// The current limitation for a string instance is 1GB character data. /// See IMPALA-1619 for more details. static const int MAX_LENGTH = (1 << 30); diff --git a/be/src/runtime/timestamp-value.h b/be/src/runtime/timestamp-value.h index 2509f21..7ae749f 100644 --- a/be/src/runtime/timestamp-value.h +++ b/be/src/runtime/timestamp-value.h @@ -67,7 +67,7 @@ struct DateTimeFormatContext; /// timings, the local time should never be used since it is affected by daylight savings. /// Also keep in mind that the time component rolls over at midnight so the date should /// always be checked when determining a duration. -class TimestampValue { +class /* __attribute__ ((__packed__)) */ TimestampValue { public: TimestampValue() {} diff --git a/be/src/runtime/types.h b/be/src/runtime/types.h index 1509c1f..4ab39c5 100644 --- a/be/src/runtime/types.h +++ b/be/src/runtime/types.h @@ -244,6 +244,7 @@ struct ColumnType { return 8; case TYPE_TIMESTAMP: // This is the size of the slot, the actual size of the data is 12. + // TODO: padd return 16; case TYPE_DECIMAL: return GetDecimalByteSize(precision); @@ -260,13 +261,13 @@ struct ColumnType { switch (type) { case TYPE_STRING: case TYPE_VARCHAR: - return 16; + return 12; case TYPE_CHAR: case TYPE_FIXED_UDA_INTERMEDIATE: return len; case TYPE_ARRAY: case TYPE_MAP: - return 16; + return 12; case TYPE_STRUCT: DCHECK(false) << "TYPE_STRUCT slot not possible"; default: diff --git a/be/src/util/static-asserts.cc b/be/src/util/static-asserts.cc index 7662906..86198f3 100644 --- a/be/src/util/static-asserts.cc +++ b/be/src/util/static-asserts.cc @@ -28,9 +28,9 @@ namespace impala { // at compile time. If these asserts fail, the compile will fail. class UnusedClass { private: - BOOST_STATIC_ASSERT(sizeof(StringValue) == 16); + BOOST_STATIC_ASSERT(sizeof(StringValue) == 12); BOOST_STATIC_ASSERT(offsetof(StringValue, len) == 8); - BOOST_STATIC_ASSERT(sizeof(TimestampValue) == 16); + BOOST_STATIC_ASSERT(sizeof(TimestampValue) == 16); // TODO BOOST_STATIC_ASSERT(offsetof(TimestampValue, date_) == 8); BOOST_STATIC_ASSERT(sizeof(boost::posix_time::time_duration) == 8); BOOST_STATIC_ASSERT(sizeof(boost::gregorian::date) == 4); diff --git a/fe/src/main/java/org/apache/impala/catalog/PrimitiveType.java b/fe/src/main/java/org/apache/impala/catalog/PrimitiveType.java index e09fd7b..4234855 100644 --- a/fe/src/main/java/org/apache/impala/catalog/PrimitiveType.java +++ b/fe/src/main/java/org/apache/impala/catalog/PrimitiveType.java @@ -36,12 +36,11 @@ public enum PrimitiveType { DOUBLE("DOUBLE", 8, TPrimitiveType.DOUBLE), DATE("DATE", 4, TPrimitiveType.DATE), DATETIME("DATETIME", 8, TPrimitiveType.DATETIME), - // The timestamp structure is 12 bytes, Aligning to 8 bytes makes it 16. + // The timestamp structure is 12 bytes. TODO: remove padding TIMESTAMP("TIMESTAMP", 16, TPrimitiveType.TIMESTAMP), // 8-byte pointer and 4-byte length indicator (12 bytes total). - // Aligning to 8 bytes so 16 total. - STRING("STRING", 16, TPrimitiveType.STRING), - VARCHAR("VARCHAR", 16, TPrimitiveType.VARCHAR), + STRING("STRING", 12, TPrimitiveType.STRING), + VARCHAR("VARCHAR", 12, TPrimitiveType.VARCHAR), // Unsupported scalar type. BINARY("BINARY", -1, TPrimitiveType.BINARY), diff --git a/fe/src/main/java/org/apache/impala/catalog/Type.java b/fe/src/main/java/org/apache/impala/catalog/Type.java index a85cd40..a9a9373 100644 --- a/fe/src/main/java/org/apache/impala/catalog/Type.java +++ b/fe/src/main/java/org/apache/impala/catalog/Type.java @@ -223,9 +223,7 @@ public abstract class Type { */ public int getSlotSize() { // 8-byte pointer and 4-byte length indicator (12 bytes total). - // Per struct alignment rules, there is an extra 4 bytes of padding to align to 8 - // bytes so 16 bytes total. - if (isCollectionType()) return 16; + if (isCollectionType()) return 12; throw new IllegalStateException("getSlotSize() not implemented for type " + toSql()); } -- 2.7.4