From 6f5d6bb52a4195dcbaf957c496f0f89f676e5f4c Mon Sep 17 00:00:00 2001 From: Karthikeyan Natarajan Date: Wed, 9 Apr 2025 18:05:51 -0500 Subject: [PATCH 1/7] add OrderByTest.allTypesWithIntegerKey --- velox/experimental/cudf/tests/OrderByTest.cpp | 301 ++++++++++++++++++ 1 file changed, 301 insertions(+) diff --git a/velox/experimental/cudf/tests/OrderByTest.cpp b/velox/experimental/cudf/tests/OrderByTest.cpp index 334671305c43..a93b120e629e 100644 --- a/velox/experimental/cudf/tests/OrderByTest.cpp +++ b/velox/experimental/cudf/tests/OrderByTest.cpp @@ -412,4 +412,305 @@ TEST_F(OrderByTest, outputBatchRows) { } #endif +TEST_F(OrderByTest, allTypesWithIntegerKey) { + vector_size_t batchSize = 500; + std::vector vectors; + + // Define the types we'll use + auto keyType = INTEGER(); + auto boolType = BOOLEAN(); + auto tinyintType = TINYINT(); + auto smallintType = SMALLINT(); + auto bigintType = BIGINT(); + auto realType = REAL(); + auto doubleType = DOUBLE(); + auto hugeIntType = HUGEINT(); // Note:(not supported by ArrowSchema) + auto varcharType = VARCHAR(); + auto varbinaryType = VARBINARY(); // Unsupported + auto timestampType = TIMESTAMP(); + auto arrayType = ARRAY(INTEGER()); + auto rowType = ROW({{"nested1", BOOLEAN()}, {"nested2", INTEGER()}}); + auto shortDecimalType = DECIMAL(10, 2); // Unsupported due to a bug in ArrowSchema in Velox + auto longDecimalType = DECIMAL(38, 10); + auto dateType = DATE(); + auto intervalDayTimeType = INTERVAL_DAY_TIME(); + auto intervalYearMonthType = INTERVAL_YEAR_MONTH(); // Unsupported + // MAP, VARBINARY, UKNOWN, FUNCTION, OPAQUE // Unsupported + + for (int32_t i = 0; i < 3; ++i) { + std::vector children; + std::vector names; + + // Integer key column - used for ordering + auto c0 = makeFlatVector( + batchSize, + [&](vector_size_t row) { return batchSize * i + row; }, + nullEvery(7), + keyType); + children.push_back(c0); + names.push_back("c0"); + // BOOLEAN + auto c1 = makeFlatVector( + batchSize, + [](vector_size_t row) { return row % 2 == 0; }, + nullEvery(5), + boolType); + children.push_back(c1); + names.push_back("c1"); + // TINYINT + auto c2 = makeFlatVector( + batchSize, + [](vector_size_t row) { return row % 127; }, + nullEvery(9), + tinyintType); + children.push_back(c2); + names.push_back("c2"); + // SMALLINT + auto c3 = makeFlatVector( + batchSize, + [](vector_size_t row) { return row % 32767; }, + nullEvery(11), + smallintType); + children.push_back(c3); + names.push_back("c3"); + + // BIGINT + auto c4 = makeFlatVector( + batchSize, + [](vector_size_t row) { return row * 10000; }, + nullEvery(13), + bigintType); + children.push_back(c4); + names.push_back("c4"); + + // HUGEINT Note:(not supported by ArrowSchema) + // auto c5 = makeFlatVector( + // batchSize, + // [](vector_size_t row) { return row * 1000000000000000000ull; }, + // nullEvery(17), + // hugeIntType); + // children.push_back(c5); + + // REAL (float) + auto c6 = makeFlatVector( + batchSize, + [](vector_size_t row) { return row * 0.1f; }, + nullEvery(15), + realType); + children.push_back(c6); + names.push_back("c6"); + + // DOUBLE + auto c7 = makeFlatVector( + batchSize, + [](vector_size_t row) { return row * 0.01; }, + nullEvery(17), + doubleType); + children.push_back(c7); + names.push_back("c7"); + + // VARCHAR + auto c8 = makeFlatVector( + batchSize, + [](vector_size_t row) { + return StringView::makeInline("str_" + std::to_string(row)); + }, + nullEvery(19), + varcharType); + children.push_back(c8); + names.push_back("c8"); + + // VARBINARY + // auto c9 = makeFlatVector( + // batchSize, + // [](vector_size_t row) { + // return StringView::makeInline("bin_" + std::to_string(row)); + // }, + // nullEvery(21), + // varbinaryType); + // children.push_back(c9); + // names.push_back("c9"); + + // TIMESTAMP + auto c10 = makeFlatVector( + batchSize, + [](vector_size_t row) { + // Create timestamps with increasing seconds + return Timestamp(1600000000 + row, row * 1000); + }, + nullEvery(23), + timestampType); + children.push_back(c10); + names.push_back("c10"); + // ARRAY(INTEGER()) + auto c11 = makeArrayVector( + batchSize, + [](vector_size_t row) { return row % 5 + 1; }, // array sizes + [](vector_size_t idx) { return idx * 2; }, // array contents + nullEvery(13)); + children.push_back(c11); + names.push_back("c11"); + + // ROW/STRUCT type + auto nestedBool = makeFlatVector( + batchSize, + [](vector_size_t row) { return row % 3 == 0; }, + nullEvery(11), + BOOLEAN()); + auto nestedInt = makeFlatVector( + batchSize, + [](vector_size_t row) { return row * 5; }, + nullEvery(13), + INTEGER()); + auto c12 = makeRowVector( + {"nested1", "nested2"}, + {nestedBool, nestedInt}); + children.push_back(c12); + names.push_back("c12"); + + // DECIMAL(10, 2) - short decimal + // auto c13 = makeFlatVector( + // batchSize, + // [](vector_size_t row) { return row * 123; }, // Will be interpreted as row*1.23 + // nullEvery(27), + // shortDecimalType); + // children.push_back(c13); + // names.push_back("c13"); + + // DECIMAL(38, 10) - long decimal + auto c14 = makeFlatVector( + batchSize, + [](vector_size_t row) { + // Creating a simple int128 value that increases with row number + // Use a smaller multiplier to avoid overflow + return static_cast(row) * 1234567890; + }, + nullEvery(29), + longDecimalType); + children.push_back(c14); + names.push_back("c14"); + + // DATE + auto c15 = makeFlatVector( + batchSize, + [](vector_size_t row) { + // Days since epoch, starting from 2020-01-01 (18262) plus row offset + return 18262 + row % 1000; + }, + nullEvery(31), + dateType); + children.push_back(c15); + names.push_back("c15"); + + // INTERVAL_DAY_TIME - stored as int64_t + auto c16 = makeFlatVector( + batchSize, + [](vector_size_t row) { + // Interval of days and milliseconds: row days and row*100 milliseconds + return row * 86400000 + row * 100; + }, + nullEvery(33), + intervalDayTimeType); + children.push_back(c16); + names.push_back("c16"); + + // INTERVAL_YEAR_MONTH - stored as int32_t + // auto c17 = makeFlatVector( + // batchSize, + // [](vector_size_t row) { + // // Interval of months: row months + // return row % 120; // 0-120 months (0-10 years) + // }, + // nullEvery(35), + // intervalYearMonthType); + // children.push_back(c17); + // names.push_back("c17"); + // Create a flat vector to wrap in a dictionary + auto flatVector = makeFlatVector(batchSize, [](vector_size_t row) { return row; }); + // Create indices + auto indices = makeIndices(batchSize, [](vector_size_t i) { return i % 100; }); + // Create a null buffer where every 3rd entry is null + auto nulls = makeNulls(batchSize, [](vector_size_t row) { return row % 3 == 0; }); + + // Create dictionary with nulls - note we pass the nulls buffer as first parameter + auto c18 = BaseVector::wrapInDictionary(nulls, indices, batchSize, flatVector); + // For comparison, create a dictionary without nulls + // auto dictWithoutNulls = BaseVector::wrapInDictionary(nullptr, indices, batchSize, flatVector); + children.push_back(c18); + names.push_back("c18"); + + vectors.push_back(makeRowVector(names, children)); + } + + createDuckDbTable(vectors); + + // Test ordering by the integer key + testSingleKey(vectors, "c0"); + + // Test with a filter + testSingleKey(vectors, "c0", "c0 % 100 = 0"); + + // Test different ordering directions + core::PlanNodeId orderById; + auto plan = PlanBuilder() + .values(vectors) + .orderBy({"c0 DESC NULLS LAST"}, false) + .capturePlanNodeId(orderById) + .planNode(); + runTest( + plan, orderById, "SELECT * FROM tmp ORDER BY c0 DESC NULLS LAST", {0}); + + // Test with another column as secondary key + testTwoKeys(vectors, "c0", "c1"); + + // Test ordering by boolean column + testSingleKey(vectors, "c1"); + + // Test ordering by tinyint column + testSingleKey(vectors, "c2"); + + // Test ordering by smallint column + testSingleKey(vectors, "c3"); + + // Test ordering by bigint column + testSingleKey(vectors, "c4"); + + // Test ordering by date column + testSingleKey(vectors, "c15"); + + // Test ordering by interval column + testSingleKey(vectors, "c16"); + + // Test ordering by dictionary column + testSingleKey(vectors, "c18"); + + // Test multiple ordering directions + core::PlanNodeId multiOrderById; + auto multiOrderPlan = PlanBuilder() + .values(vectors) + .orderBy({"c0 ASC NULLS FIRST", "c1 DESC NULLS LAST"}, false) + .capturePlanNodeId(multiOrderById) + .planNode(); + runTest( + multiOrderPlan, + multiOrderById, + "SELECT * FROM tmp ORDER BY c0 ASC NULLS FIRST, c1 DESC NULLS LAST", + {0, 1}); + + // Test with complex filter + testSingleKey(vectors, "c0", "c1 = true AND c2 < 100"); + + // Test with three keys + core::PlanNodeId threeKeysOrderById; + auto threeKeysPlan = PlanBuilder() + .values(vectors) + .orderBy({"c0 ASC", "c1 DESC", "c2 ASC"}, false) + .capturePlanNodeId(threeKeysOrderById) + .planNode(); + runTest( + threeKeysPlan, + threeKeysOrderById, + "SELECT * FROM tmp ORDER BY c0 ASC, c1 DESC, c2 ASC", + {0, 1, 2}); +} } // namespace From fcd7ce0e660da5e36ae5082e1074c8cc2cdddca6 Mon Sep 17 00:00:00 2001 From: Karthikeyan Natarajan Date: Wed, 9 Apr 2025 18:10:05 -0500 Subject: [PATCH 2/7] debug prints --- .../experimental/cudf/exec/CudfConversion.cpp | 24 ++++++++++++++- .../cudf/exec/VeloxCudfInterop.cpp | 29 +++++++++++++++++++ velox/vector/arrow/Bridge.cpp | 12 +++++--- 3 files changed, 60 insertions(+), 5 deletions(-) diff --git a/velox/experimental/cudf/exec/CudfConversion.cpp b/velox/experimental/cudf/exec/CudfConversion.cpp index 404125842f89..4b8c078cd18f 100644 --- a/velox/experimental/cudf/exec/CudfConversion.cpp +++ b/velox/experimental/cudf/exec/CudfConversion.cpp @@ -119,7 +119,10 @@ RowVectorPtr CudfFromVelox::getOutput() { // Combine selected RowVectors into a single RowVector auto input = mergeRowVectors(selectedInputs, inputs_[0]->pool()); - + // print physical type of each column in output + for (auto i = 0; i < input->type()->size(); i++) { + std::cout << "input column " << i << " type: " << input->childAt(i)->type()->toString() << " " << input->childAt(i)->type()->kindName() << std::endl; + } // Remove processed inputs inputs_.erase(inputs_.begin(), inputs_.begin() + selectedInputs.size()); currentOutputSize_ -= totalSize; @@ -134,6 +137,10 @@ RowVectorPtr CudfFromVelox::getOutput() { // Convert RowVector to cudf table auto tbl = with_arrow::toCudfTable(input, input->pool(), stream); + // print types of tbl->view() + for (auto i = 0; i < tbl->num_columns(); i++) { + std::cout << "input cudf column " << i << " type: " << static_cast(tbl->get_column(i).type().id()) << std::endl; + } stream.synchronize(); @@ -206,8 +213,23 @@ RowVectorPtr CudfToVelox::getOutput() { } RowVectorPtr output = with_arrow::toVeloxColumn(tbl->view(), pool(), "", stream); + + // print types of tbl->view() + for (auto i = 0; i < tbl->num_columns(); i++) { + std::cout << "cudf column " << i << " type: " << static_cast(tbl->get_column(i).type().id()) << std::endl; + } stream.synchronize(); finished_ = noMoreInput_ && inputs_.empty(); + std::cout << "output.type: " << output->type()->toString() << std::endl; + std::cout << "outputType_: " << outputType_->toString() << std::endl; + // print physical type of each column in output + for (auto i = 0; i < output->type()->size(); i++) { + std::cout << "column " << i << " type: " << output->childAt(i)->type()->toString() << " " << output->childAt(i)->type()->kindName() << std::endl; + } + // print physical type of each column in outputType_ + for (auto i = 0; i < outputType_->size(); i++) { + std::cout << "column " << i << " type: " << outputType_->childAt(i)->toString() << " " << outputType_->childAt(i)->kindName() << std::endl; + } output->setType(outputType_); return output; } diff --git a/velox/experimental/cudf/exec/VeloxCudfInterop.cpp b/velox/experimental/cudf/exec/VeloxCudfInterop.cpp index 34887f206443..e2b03a9299e0 100644 --- a/velox/experimental/cudf/exec/VeloxCudfInterop.cpp +++ b/velox/experimental/cudf/exec/VeloxCudfInterop.cpp @@ -84,6 +84,34 @@ cudf::type_id velox_to_cudf_type_id(const TypePtr& type) { } namespace with_arrow { + + // Print ArrowSchema format strings recursively + void printArrowSchemaFormat(const ArrowSchema& arrowSchema, int depth = 0) { + std::string indent(depth * 2, ' '); + if (depth == 0) { + std::cout << "arrowSchema.format: " << arrowSchema.format << std::endl; + } + + for (int64_t i = 0; i < arrowSchema.n_children; ++i) { + const ArrowSchema* child = arrowSchema.children[i]; + if (child != nullptr) { + std::cout << indent << " child[" << i << "].format: " + << child->format << std::endl; + if (child->n_children > 0) { + printArrowSchemaFormat(*child, depth + 1); + } + } + } + + if (arrowSchema.dictionary != nullptr) { + std::cout << indent << " dictionary.format: " + << arrowSchema.dictionary->format << std::endl; + if (arrowSchema.dictionary->n_children > 0) { + printArrowSchemaFormat(*arrowSchema.dictionary, depth + 1); + } + } + } + std::unique_ptr toCudfTable( const facebook::velox::RowVectorPtr& veloxTable, facebook::velox::memory::MemoryPool* pool, @@ -102,6 +130,7 @@ std::unique_ptr toCudfTable( std::dynamic_pointer_cast(veloxTable), arrowSchema, arrowOptions); + printArrowSchemaFormat(arrowSchema); auto tbl = cudf::from_arrow(&arrowSchema, &arrowArray, stream); // Release Arrow resources diff --git a/velox/vector/arrow/Bridge.cpp b/velox/vector/arrow/Bridge.cpp index 2c81819163c4..41c9054e2a13 100644 --- a/velox/vector/arrow/Bridge.cpp +++ b/velox/vector/arrow/Bridge.cpp @@ -26,6 +26,8 @@ #include "velox/vector/VectorTypeUtils.h" #include "velox/vector/arrow/Abi.h" +#include + namespace facebook::velox { namespace { @@ -1234,16 +1236,16 @@ TypePtr parseDecimalFormat(const char* format) { // Parse "d:". int precision = std::stoi(&format[2], &sz); - int scale = std::stoi(&format[firstCommaIdx + 1], &sz); - // If bitwidth is provided, check if it is equal to 128. + std::cout << "pformat: " << format << std::endl; + std::cout << "precision: " << precision << " scale: " << scale << std::endl; if (secondCommaIdx != std::string::npos) { int bitWidth = std::stoi(&format[secondCommaIdx + 1], &sz); VELOX_USER_CHECK_EQ( bitWidth, 128, "Conversion failed for '{}'. Velox decimal does not support custom bitwidth.", - format); - } + std::cout << "pformat: " << format << std::endl; + std::cout << "precision: " << precision << " scale: " << scale << std::endl; return DECIMAL(precision, scale); } catch (std::invalid_argument&) { VELOX_USER_FAIL(invalidFormatMsg, format); @@ -1255,6 +1257,8 @@ TypePtr importFromArrowImpl( const ArrowSchema& arrowSchema) { VELOX_CHECK_NOT_NULL(format); + std::cout << "format: " << format << std::endl; + switch (format[0]) { case 'b': return BOOLEAN(); From 05ca9784727447c0a8530b372bc64babeb487d15 Mon Sep 17 00:00:00 2001 From: Karthikeyan Natarajan Date: Wed, 9 Apr 2025 18:10:31 -0500 Subject: [PATCH 3/7] fix for parsing decimal types with optional bit_width --- velox/vector/arrow/Bridge.cpp | 58 ++++++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 15 deletions(-) diff --git a/velox/vector/arrow/Bridge.cpp b/velox/vector/arrow/Bridge.cpp index 41c9054e2a13..5c1111c04bc2 100644 --- a/velox/vector/arrow/Bridge.cpp +++ b/velox/vector/arrow/Bridge.cpp @@ -259,6 +259,30 @@ const char* exportArrowFormatTimestampStr( return formatBuffer.c_str(); } +uint8_t getDecimalWidth(const Type& type) { + // Get the physical type's bit width + switch (type.kind()) { + case TypeKind::BOOLEAN: + return 1; + case TypeKind::TINYINT: + return 8; + case TypeKind::SMALLINT: + return 16; + case TypeKind::INTEGER: + return 32; + case TypeKind::BIGINT: + return 64; + case TypeKind::REAL: + return 32; + case TypeKind::DOUBLE: + return 64; + case TypeKind::HUGEINT: + return 128; + default: + return 0; // Default for complex types + } +} + // Returns the Arrow C data interface format type for a given Velox type. const char* exportArrowFormatStr( const TypePtr& type, @@ -267,7 +291,8 @@ const char* exportArrowFormatStr( if (type->isDecimal()) { // Decimal types encode the precision, scale values. const auto& [precision, scale] = getDecimalPrecisionScale(*type); - formatBuffer = fmt::format("d:{},{}", precision, scale); + auto const width = getDecimalWidth(*type); + formatBuffer = fmt::format("d:{},{},{}", precision, scale, width); return formatBuffer.c_str(); } @@ -1227,23 +1252,26 @@ TypePtr parseDecimalFormat(const char* format) { auto firstCommaIdx = formatStr.find(',', 2); auto secondCommaIdx = formatStr.find(',', firstCommaIdx + 1); - if (firstCommaIdx == std::string::npos || - formatStr.size() == firstCommaIdx + 1 || - (secondCommaIdx != std::string::npos && - formatStr.size() == secondCommaIdx + 1)) { - VELOX_USER_FAIL(invalidFormatMsg, format); - } + // if (firstCommaIdx == std::string::npos || + // formatStr.size() == firstCommaIdx + 1 || + // (secondCommaIdx != std::string::npos && + // formatStr.size() == secondCommaIdx + 1)) { + // VELOX_USER_FAIL(invalidFormatMsg, format); + // } // Parse "d:". int precision = std::stoi(&format[2], &sz); - std::cout << "pformat: " << format << std::endl; - std::cout << "precision: " << precision << " scale: " << scale << std::endl; - if (secondCommaIdx != std::string::npos) { - int bitWidth = std::stoi(&format[secondCommaIdx + 1], &sz); - VELOX_USER_CHECK_EQ( - bitWidth, - 128, - "Conversion failed for '{}'. Velox decimal does not support custom bitwidth.", + int scale = std::stoi(&format[firstCommaIdx + 1], &sz); + // If bitwidth is provided, check if it is equal to 128. + // if (secondCommaIdx != std::string::npos) { + // int bitWidth = std::stoi(&format[secondCommaIdx + 1], &sz); + // VELOX_USER_CHECK_EQ( + // bitWidth, + // 128, + // "Conversion failed for '{}'. Velox decimal does not support custom bitwidth.", + // format); + // } + std::cout << "pformat: " << format << std::endl; std::cout << "precision: " << precision << " scale: " << scale << std::endl; return DECIMAL(precision, scale); From 985b1700124d1bf28de244eaf6b905e119e991cd Mon Sep 17 00:00:00 2001 From: Karthikeyan Natarajan Date: Thu, 10 Apr 2025 18:15:39 -0500 Subject: [PATCH 4/7] cleanup comments --- velox/experimental/cudf/tests/OrderByTest.cpp | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/velox/experimental/cudf/tests/OrderByTest.cpp b/velox/experimental/cudf/tests/OrderByTest.cpp index a93b120e629e..a6689fc6c815 100644 --- a/velox/experimental/cudf/tests/OrderByTest.cpp +++ b/velox/experimental/cudf/tests/OrderByTest.cpp @@ -424,7 +424,7 @@ TEST_F(OrderByTest, allTypesWithIntegerKey) { auto bigintType = BIGINT(); auto realType = REAL(); auto doubleType = DOUBLE(); - auto hugeIntType = HUGEINT(); // Note:(not supported by ArrowSchema) + auto hugeIntType = HUGEINT(); // Note: not supported by ArrowSchema auto varcharType = VARCHAR(); auto varbinaryType = VARBINARY(); // Unsupported auto timestampType = TIMESTAMP(); @@ -441,7 +441,7 @@ TEST_F(OrderByTest, allTypesWithIntegerKey) { std::vector children; std::vector names; - // Integer key column - used for ordering + // Integer key column for ordering auto c0 = makeFlatVector( batchSize, [&](vector_size_t row) { return batchSize * i + row; }, @@ -449,6 +449,7 @@ TEST_F(OrderByTest, allTypesWithIntegerKey) { keyType); children.push_back(c0); names.push_back("c0"); + // BOOLEAN auto c1 = makeFlatVector( batchSize, @@ -457,6 +458,7 @@ TEST_F(OrderByTest, allTypesWithIntegerKey) { boolType); children.push_back(c1); names.push_back("c1"); + // TINYINT auto c2 = makeFlatVector( batchSize, @@ -465,6 +467,7 @@ TEST_F(OrderByTest, allTypesWithIntegerKey) { tinyintType); children.push_back(c2); names.push_back("c2"); + // SMALLINT auto c3 = makeFlatVector( batchSize, @@ -535,13 +538,14 @@ TEST_F(OrderByTest, allTypesWithIntegerKey) { auto c10 = makeFlatVector( batchSize, [](vector_size_t row) { - // Create timestamps with increasing seconds + // seconds, nanoseconds return Timestamp(1600000000 + row, row * 1000); }, nullEvery(23), timestampType); children.push_back(c10); names.push_back("c10"); + // ARRAY(INTEGER()) auto c11 = makeArrayVector( batchSize, @@ -581,8 +585,6 @@ TEST_F(OrderByTest, allTypesWithIntegerKey) { auto c14 = makeFlatVector( batchSize, [](vector_size_t row) { - // Creating a simple int128 value that increases with row number - // Use a smaller multiplier to avoid overflow return static_cast(row) * 1234567890; }, nullEvery(29), @@ -625,14 +627,12 @@ TEST_F(OrderByTest, allTypesWithIntegerKey) { // intervalYearMonthType); // children.push_back(c17); // names.push_back("c17"); - // Create a flat vector to wrap in a dictionary + + // Dictionary vector auto flatVector = makeFlatVector(batchSize, [](vector_size_t row) { return row; }); - // Create indices auto indices = makeIndices(batchSize, [](vector_size_t i) { return i % 100; }); - // Create a null buffer where every 3rd entry is null auto nulls = makeNulls(batchSize, [](vector_size_t row) { return row % 3 == 0; }); - // Create dictionary with nulls - note we pass the nulls buffer as first parameter auto c18 = BaseVector::wrapInDictionary(nulls, indices, batchSize, flatVector); // For comparison, create a dictionary without nulls // auto dictWithoutNulls = BaseVector::wrapInDictionary(nullptr, indices, batchSize, flatVector); @@ -650,7 +650,7 @@ TEST_F(OrderByTest, allTypesWithIntegerKey) { // Test with a filter testSingleKey(vectors, "c0", "c0 % 100 = 0"); - // Test different ordering directions + // Test descending order core::PlanNodeId orderById; auto plan = PlanBuilder() .values(vectors) @@ -660,7 +660,7 @@ TEST_F(OrderByTest, allTypesWithIntegerKey) { runTest( plan, orderById, "SELECT * FROM tmp ORDER BY c0 DESC NULLS LAST", {0}); - // Test with another column as secondary key + // Test with secondary key testTwoKeys(vectors, "c0", "c1"); // Test ordering by boolean column From d2afd5fb431a60343a93fdeb1dcf4213d26eac67 Mon Sep 17 00:00:00 2001 From: Karthikeyan Natarajan Date: Thu, 10 Apr 2025 18:16:11 -0500 Subject: [PATCH 5/7] replace hardcoding with type dispatch --- velox/type/Type.cpp | 20 ++++++++++++++++++++ velox/type/Type.h | 2 ++ velox/vector/arrow/Bridge.cpp | 26 +------------------------- 3 files changed, 23 insertions(+), 25 deletions(-) diff --git a/velox/type/Type.cpp b/velox/type/Type.cpp index a498dd1d4592..46dd2377f9fe 100644 --- a/velox/type/Type.cpp +++ b/velox/type/Type.cpp @@ -119,6 +119,26 @@ std::string mapTypeKindToName(const TypeKind& typeKind) { return found->second; } +namespace { +template +static int32_t kindSize() { + return sizeof(typename KindToFlatVector::HashRowType); +} + +static int32_t typeKindSize(TypeKind kind) { + if (kind == TypeKind::UNKNOWN) { + return sizeof(UnknownValue); + } + + return VELOX_DYNAMIC_TYPE_DISPATCH(kindSize, kind); +} +} // namespace + +uint8_t getDecimalBitWidth(const Type& type) { + // Get the physical type's bit width + return typeKindSize(type.kind()) * 8; +} + std::pair getDecimalPrecisionScale(const Type& type) { if (type.isShortDecimal()) { const auto& decimalType = static_cast(type); diff --git a/velox/type/Type.h b/velox/type/Type.h index e95d7abd87d6..12a9355f38d2 100644 --- a/velox/type/Type.h +++ b/velox/type/Type.h @@ -845,6 +845,8 @@ FOLLY_ALWAYS_INLINE bool isDecimalName(const std::string& name) { return (name == "DECIMAL"); } +uint8_t getDecimalBitWidth(const Type& type); + std::pair getDecimalPrecisionScale(const Type& type); class UnknownType : public CanProvideCustomComparisonType { diff --git a/velox/vector/arrow/Bridge.cpp b/velox/vector/arrow/Bridge.cpp index 5c1111c04bc2..56215f36d26c 100644 --- a/velox/vector/arrow/Bridge.cpp +++ b/velox/vector/arrow/Bridge.cpp @@ -259,30 +259,6 @@ const char* exportArrowFormatTimestampStr( return formatBuffer.c_str(); } -uint8_t getDecimalWidth(const Type& type) { - // Get the physical type's bit width - switch (type.kind()) { - case TypeKind::BOOLEAN: - return 1; - case TypeKind::TINYINT: - return 8; - case TypeKind::SMALLINT: - return 16; - case TypeKind::INTEGER: - return 32; - case TypeKind::BIGINT: - return 64; - case TypeKind::REAL: - return 32; - case TypeKind::DOUBLE: - return 64; - case TypeKind::HUGEINT: - return 128; - default: - return 0; // Default for complex types - } -} - // Returns the Arrow C data interface format type for a given Velox type. const char* exportArrowFormatStr( const TypePtr& type, @@ -291,7 +267,7 @@ const char* exportArrowFormatStr( if (type->isDecimal()) { // Decimal types encode the precision, scale values. const auto& [precision, scale] = getDecimalPrecisionScale(*type); - auto const width = getDecimalWidth(*type); + auto const width = getDecimalBitWidth(*type); formatBuffer = fmt::format("d:{},{},{}", precision, scale, width); return formatBuffer.c_str(); } From 53eb9b8a0564b7c10ee98f2e467a3eeb187a3b61 Mon Sep 17 00:00:00 2001 From: Karthikeyan Natarajan Date: Thu, 10 Apr 2025 18:39:34 -0500 Subject: [PATCH 6/7] fix style --- .../experimental/cudf/exec/CudfConversion.cpp | 22 ++- .../cudf/exec/VeloxCudfInterop.cpp | 46 ++--- velox/experimental/cudf/tests/OrderByTest.cpp | 163 +++++++++--------- velox/type/Type.cpp | 4 +- velox/vector/arrow/Bridge.cpp | 4 +- 5 files changed, 127 insertions(+), 112 deletions(-) diff --git a/velox/experimental/cudf/exec/CudfConversion.cpp b/velox/experimental/cudf/exec/CudfConversion.cpp index 4b8c078cd18f..9c2ff092cef3 100644 --- a/velox/experimental/cudf/exec/CudfConversion.cpp +++ b/velox/experimental/cudf/exec/CudfConversion.cpp @@ -121,7 +121,9 @@ RowVectorPtr CudfFromVelox::getOutput() { auto input = mergeRowVectors(selectedInputs, inputs_[0]->pool()); // print physical type of each column in output for (auto i = 0; i < input->type()->size(); i++) { - std::cout << "input column " << i << " type: " << input->childAt(i)->type()->toString() << " " << input->childAt(i)->type()->kindName() << std::endl; + std::cout << "input column " << i + << " type: " << input->childAt(i)->type()->toString() << " " + << input->childAt(i)->type()->kindName() << std::endl; } // Remove processed inputs inputs_.erase(inputs_.begin(), inputs_.begin() + selectedInputs.size()); @@ -137,9 +139,11 @@ RowVectorPtr CudfFromVelox::getOutput() { // Convert RowVector to cudf table auto tbl = with_arrow::toCudfTable(input, input->pool(), stream); - // print types of tbl->view() + // print types of tbl->view() for (auto i = 0; i < tbl->num_columns(); i++) { - std::cout << "input cudf column " << i << " type: " << static_cast(tbl->get_column(i).type().id()) << std::endl; + std::cout << "input cudf column " << i + << " type: " << static_cast(tbl->get_column(i).type().id()) + << std::endl; } stream.synchronize(); @@ -216,7 +220,9 @@ RowVectorPtr CudfToVelox::getOutput() { // print types of tbl->view() for (auto i = 0; i < tbl->num_columns(); i++) { - std::cout << "cudf column " << i << " type: " << static_cast(tbl->get_column(i).type().id()) << std::endl; + std::cout << "cudf column " << i + << " type: " << static_cast(tbl->get_column(i).type().id()) + << std::endl; } stream.synchronize(); finished_ = noMoreInput_ && inputs_.empty(); @@ -224,11 +230,15 @@ RowVectorPtr CudfToVelox::getOutput() { std::cout << "outputType_: " << outputType_->toString() << std::endl; // print physical type of each column in output for (auto i = 0; i < output->type()->size(); i++) { - std::cout << "column " << i << " type: " << output->childAt(i)->type()->toString() << " " << output->childAt(i)->type()->kindName() << std::endl; + std::cout << "column " << i + << " type: " << output->childAt(i)->type()->toString() << " " + << output->childAt(i)->type()->kindName() << std::endl; } // print physical type of each column in outputType_ for (auto i = 0; i < outputType_->size(); i++) { - std::cout << "column " << i << " type: " << outputType_->childAt(i)->toString() << " " << outputType_->childAt(i)->kindName() << std::endl; + std::cout << "column " << i + << " type: " << outputType_->childAt(i)->toString() << " " + << outputType_->childAt(i)->kindName() << std::endl; } output->setType(outputType_); return output; diff --git a/velox/experimental/cudf/exec/VeloxCudfInterop.cpp b/velox/experimental/cudf/exec/VeloxCudfInterop.cpp index e2b03a9299e0..a43397de52b2 100644 --- a/velox/experimental/cudf/exec/VeloxCudfInterop.cpp +++ b/velox/experimental/cudf/exec/VeloxCudfInterop.cpp @@ -84,34 +84,34 @@ cudf::type_id velox_to_cudf_type_id(const TypePtr& type) { } namespace with_arrow { +// Print ArrowSchema format strings recursively +void printArrowSchemaFormat(const ArrowSchema& arrowSchema, int depth = 0) { + std::string indent(depth * 2, ' '); + if (depth == 0) { + std::cout << "arrowSchema.format: " << arrowSchema.format << std::endl; + } - // Print ArrowSchema format strings recursively - void printArrowSchemaFormat(const ArrowSchema& arrowSchema, int depth = 0) { - std::string indent(depth * 2, ' '); - if (depth == 0) { - std::cout << "arrowSchema.format: " << arrowSchema.format << std::endl; - } - - for (int64_t i = 0; i < arrowSchema.n_children; ++i) { - const ArrowSchema* child = arrowSchema.children[i]; - if (child != nullptr) { - std::cout << indent << " child[" << i << "].format: " - << child->format << std::endl; - if (child->n_children > 0) { - printArrowSchemaFormat(*child, depth + 1); - } + for (int64_t i = 0; i < arrowSchema.n_children; ++i) { + const ArrowSchema* child = arrowSchema.children[i]; + if (child != nullptr) { + std::cout << indent << " child[" << i << "].format: " << child->format + << std::endl; + if (child->n_children > 0) { + printArrowSchemaFormat(*child, depth + 1); } } - - if (arrowSchema.dictionary != nullptr) { - std::cout << indent << " dictionary.format: " - << arrowSchema.dictionary->format << std::endl; - if (arrowSchema.dictionary->n_children > 0) { - printArrowSchemaFormat(*arrowSchema.dictionary, depth + 1); - } + } + + if (arrowSchema.dictionary != nullptr) { + std::cout << indent + << " dictionary.format: " << arrowSchema.dictionary->format + << std::endl; + if (arrowSchema.dictionary->n_children > 0) { + printArrowSchemaFormat(*arrowSchema.dictionary, depth + 1); } } - +} + std::unique_ptr toCudfTable( const facebook::velox::RowVectorPtr& veloxTable, facebook::velox::memory::MemoryPool* pool, diff --git a/velox/experimental/cudf/tests/OrderByTest.cpp b/velox/experimental/cudf/tests/OrderByTest.cpp index a6689fc6c815..172803d27851 100644 --- a/velox/experimental/cudf/tests/OrderByTest.cpp +++ b/velox/experimental/cudf/tests/OrderByTest.cpp @@ -415,7 +415,7 @@ TEST_F(OrderByTest, outputBatchRows) { TEST_F(OrderByTest, allTypesWithIntegerKey) { vector_size_t batchSize = 500; std::vector vectors; - + // Define the types we'll use auto keyType = INTEGER(); auto boolType = BOOLEAN(); @@ -430,17 +430,18 @@ TEST_F(OrderByTest, allTypesWithIntegerKey) { auto timestampType = TIMESTAMP(); auto arrayType = ARRAY(INTEGER()); auto rowType = ROW({{"nested1", BOOLEAN()}, {"nested2", INTEGER()}}); - auto shortDecimalType = DECIMAL(10, 2); // Unsupported due to a bug in ArrowSchema in Velox + auto shortDecimalType = + DECIMAL(10, 2); // Unsupported due to a bug in ArrowSchema in Velox auto longDecimalType = DECIMAL(38, 10); auto dateType = DATE(); auto intervalDayTimeType = INTERVAL_DAY_TIME(); auto intervalYearMonthType = INTERVAL_YEAR_MONTH(); // Unsupported // MAP, VARBINARY, UKNOWN, FUNCTION, OPAQUE // Unsupported - + for (int32_t i = 0; i < 3; ++i) { std::vector children; std::vector names; - + // Integer key column for ordering auto c0 = makeFlatVector( batchSize, @@ -452,8 +453,8 @@ TEST_F(OrderByTest, allTypesWithIntegerKey) { // BOOLEAN auto c1 = makeFlatVector( - batchSize, - [](vector_size_t row) { return row % 2 == 0; }, + batchSize, + [](vector_size_t row) { return row % 2 == 0; }, nullEvery(5), boolType); children.push_back(c1); @@ -461,8 +462,8 @@ TEST_F(OrderByTest, allTypesWithIntegerKey) { // TINYINT auto c2 = makeFlatVector( - batchSize, - [](vector_size_t row) { return row % 127; }, + batchSize, + [](vector_size_t row) { return row % 127; }, nullEvery(9), tinyintType); children.push_back(c2); @@ -470,17 +471,17 @@ TEST_F(OrderByTest, allTypesWithIntegerKey) { // SMALLINT auto c3 = makeFlatVector( - batchSize, - [](vector_size_t row) { return row % 32767; }, + batchSize, + [](vector_size_t row) { return row % 32767; }, nullEvery(11), smallintType); children.push_back(c3); names.push_back("c3"); - + // BIGINT auto c4 = makeFlatVector( - batchSize, - [](vector_size_t row) { return row * 10000; }, + batchSize, + [](vector_size_t row) { return row * 10000; }, nullEvery(13), bigintType); children.push_back(c4); @@ -488,16 +489,16 @@ TEST_F(OrderByTest, allTypesWithIntegerKey) { // HUGEINT Note:(not supported by ArrowSchema) // auto c5 = makeFlatVector( - // batchSize, - // [](vector_size_t row) { return row * 1000000000000000000ull; }, + // batchSize, + // [](vector_size_t row) { return row * 1000000000000000000ull; }, // nullEvery(17), // hugeIntType); // children.push_back(c5); // REAL (float) auto c6 = makeFlatVector( - batchSize, - [](vector_size_t row) { return row * 0.1f; }, + batchSize, + [](vector_size_t row) { return row * 0.1f; }, nullEvery(15), realType); children.push_back(c6); @@ -505,13 +506,13 @@ TEST_F(OrderByTest, allTypesWithIntegerKey) { // DOUBLE auto c7 = makeFlatVector( - batchSize, - [](vector_size_t row) { return row * 0.01; }, + batchSize, + [](vector_size_t row) { return row * 0.01; }, nullEvery(17), doubleType); children.push_back(c7); names.push_back("c7"); - + // VARCHAR auto c8 = makeFlatVector( batchSize, @@ -533,7 +534,7 @@ TEST_F(OrderByTest, allTypesWithIntegerKey) { // varbinaryType); // children.push_back(c9); // names.push_back("c9"); - + // TIMESTAMP auto c10 = makeFlatVector( batchSize, @@ -550,7 +551,7 @@ TEST_F(OrderByTest, allTypesWithIntegerKey) { auto c11 = makeArrayVector( batchSize, [](vector_size_t row) { return row % 5 + 1; }, // array sizes - [](vector_size_t idx) { return idx * 2; }, // array contents + [](vector_size_t idx) { return idx * 2; }, // array contents nullEvery(13)); children.push_back(c11); names.push_back("c11"); @@ -566,38 +567,35 @@ TEST_F(OrderByTest, allTypesWithIntegerKey) { [](vector_size_t row) { return row * 5; }, nullEvery(13), INTEGER()); - auto c12 = makeRowVector( - {"nested1", "nested2"}, - {nestedBool, nestedInt}); + auto c12 = makeRowVector({"nested1", "nested2"}, {nestedBool, nestedInt}); children.push_back(c12); names.push_back("c12"); // DECIMAL(10, 2) - short decimal // auto c13 = makeFlatVector( // batchSize, - // [](vector_size_t row) { return row * 123; }, // Will be interpreted as row*1.23 - // nullEvery(27), - // shortDecimalType); + // [](vector_size_t row) { return row * 123; }, // Will be interpreted + // as row*1.23 nullEvery(27), shortDecimalType); // children.push_back(c13); // names.push_back("c13"); // DECIMAL(38, 10) - long decimal auto c14 = makeFlatVector( batchSize, - [](vector_size_t row) { - return static_cast(row) * 1234567890; + [](vector_size_t row) { + return static_cast(row) * 1234567890; }, nullEvery(29), longDecimalType); children.push_back(c14); names.push_back("c14"); - + // DATE auto c15 = makeFlatVector( batchSize, - [](vector_size_t row) { + [](vector_size_t row) { // Days since epoch, starting from 2020-01-01 (18262) plus row offset - return 18262 + row % 1000; + return 18262 + row % 1000; }, nullEvery(31), dateType); @@ -607,19 +605,20 @@ TEST_F(OrderByTest, allTypesWithIntegerKey) { // INTERVAL_DAY_TIME - stored as int64_t auto c16 = makeFlatVector( batchSize, - [](vector_size_t row) { - // Interval of days and milliseconds: row days and row*100 milliseconds - return row * 86400000 + row * 100; + [](vector_size_t row) { + // Interval of days and milliseconds: row days and row*100 + // milliseconds + return row * 86400000 + row * 100; }, nullEvery(33), intervalDayTimeType); children.push_back(c16); names.push_back("c16"); - + // INTERVAL_YEAR_MONTH - stored as int32_t // auto c17 = makeFlatVector( // batchSize, - // [](vector_size_t row) { + // [](vector_size_t row) { // // Interval of months: row months // return row % 120; // 0-120 months (0-10 years) // }, @@ -627,90 +626,96 @@ TEST_F(OrderByTest, allTypesWithIntegerKey) { // intervalYearMonthType); // children.push_back(c17); // names.push_back("c17"); - + // Dictionary vector - auto flatVector = makeFlatVector(batchSize, [](vector_size_t row) { return row; }); - auto indices = makeIndices(batchSize, [](vector_size_t i) { return i % 100; }); - auto nulls = makeNulls(batchSize, [](vector_size_t row) { return row % 3 == 0; }); - - auto c18 = BaseVector::wrapInDictionary(nulls, indices, batchSize, flatVector); + auto flatVector = makeFlatVector( + batchSize, [](vector_size_t row) { return row; }); + auto indices = + makeIndices(batchSize, [](vector_size_t i) { return i % 100; }); + auto nulls = + makeNulls(batchSize, [](vector_size_t row) { return row % 3 == 0; }); + + auto c18 = + BaseVector::wrapInDictionary(nulls, indices, batchSize, flatVector); // For comparison, create a dictionary without nulls - // auto dictWithoutNulls = BaseVector::wrapInDictionary(nullptr, indices, batchSize, flatVector); + // auto dictWithoutNulls = BaseVector::wrapInDictionary(nullptr, indices, + // batchSize, flatVector); children.push_back(c18); names.push_back("c18"); vectors.push_back(makeRowVector(names, children)); } - + createDuckDbTable(vectors); - + // Test ordering by the integer key testSingleKey(vectors, "c0"); - + // Test with a filter testSingleKey(vectors, "c0", "c0 % 100 = 0"); - + // Test descending order core::PlanNodeId orderById; auto plan = PlanBuilder() - .values(vectors) - .orderBy({"c0 DESC NULLS LAST"}, false) - .capturePlanNodeId(orderById) - .planNode(); + .values(vectors) + .orderBy({"c0 DESC NULLS LAST"}, false) + .capturePlanNodeId(orderById) + .planNode(); runTest( plan, orderById, "SELECT * FROM tmp ORDER BY c0 DESC NULLS LAST", {0}); - + // Test with secondary key testTwoKeys(vectors, "c0", "c1"); - + // Test ordering by boolean column testSingleKey(vectors, "c1"); - + // Test ordering by tinyint column testSingleKey(vectors, "c2"); - + // Test ordering by smallint column testSingleKey(vectors, "c3"); - + // Test ordering by bigint column testSingleKey(vectors, "c4"); - + // Test ordering by date column testSingleKey(vectors, "c15"); - + // Test ordering by interval column testSingleKey(vectors, "c16"); - + // Test ordering by dictionary column testSingleKey(vectors, "c18"); - + // Test multiple ordering directions core::PlanNodeId multiOrderById; - auto multiOrderPlan = PlanBuilder() - .values(vectors) - .orderBy({"c0 ASC NULLS FIRST", "c1 DESC NULLS LAST"}, false) - .capturePlanNodeId(multiOrderById) - .planNode(); + auto multiOrderPlan = + PlanBuilder() + .values(vectors) + .orderBy({"c0 ASC NULLS FIRST", "c1 DESC NULLS LAST"}, false) + .capturePlanNodeId(multiOrderById) + .planNode(); runTest( - multiOrderPlan, - multiOrderById, - "SELECT * FROM tmp ORDER BY c0 ASC NULLS FIRST, c1 DESC NULLS LAST", + multiOrderPlan, + multiOrderById, + "SELECT * FROM tmp ORDER BY c0 ASC NULLS FIRST, c1 DESC NULLS LAST", {0, 1}); - + // Test with complex filter testSingleKey(vectors, "c0", "c1 = true AND c2 < 100"); - + // Test with three keys core::PlanNodeId threeKeysOrderById; auto threeKeysPlan = PlanBuilder() - .values(vectors) - .orderBy({"c0 ASC", "c1 DESC", "c2 ASC"}, false) - .capturePlanNodeId(threeKeysOrderById) - .planNode(); + .values(vectors) + .orderBy({"c0 ASC", "c1 DESC", "c2 ASC"}, false) + .capturePlanNodeId(threeKeysOrderById) + .planNode(); runTest( - threeKeysPlan, - threeKeysOrderById, - "SELECT * FROM tmp ORDER BY c0 ASC, c1 DESC, c2 ASC", + threeKeysPlan, + threeKeysOrderById, + "SELECT * FROM tmp ORDER BY c0 ASC, c1 DESC, c2 ASC", {0, 1, 2}); } } // namespace diff --git a/velox/type/Type.cpp b/velox/type/Type.cpp index 46dd2377f9fe..7ce6771f9474 100644 --- a/velox/type/Type.cpp +++ b/velox/type/Type.cpp @@ -135,8 +135,8 @@ static int32_t typeKindSize(TypeKind kind) { } // namespace uint8_t getDecimalBitWidth(const Type& type) { - // Get the physical type's bit width - return typeKindSize(type.kind()) * 8; + // Get the physical type's bit width + return typeKindSize(type.kind()) * 8; } std::pair getDecimalPrecisionScale(const Type& type) { diff --git a/velox/vector/arrow/Bridge.cpp b/velox/vector/arrow/Bridge.cpp index 56215f36d26c..f8e3fe053311 100644 --- a/velox/vector/arrow/Bridge.cpp +++ b/velox/vector/arrow/Bridge.cpp @@ -1244,8 +1244,8 @@ TypePtr parseDecimalFormat(const char* format) { // VELOX_USER_CHECK_EQ( // bitWidth, // 128, - // "Conversion failed for '{}'. Velox decimal does not support custom bitwidth.", - // format); + // "Conversion failed for '{}'. Velox decimal does not support custom + // bitwidth.", format); // } std::cout << "pformat: " << format << std::endl; From 69da353bf3024a2a27b1776767a4706ed96c4b26 Mon Sep 17 00:00:00 2001 From: Karthikeyan <6488848+karthikeyann@users.noreply.github.com> Date: Mon, 14 Apr 2025 11:31:48 -0500 Subject: [PATCH 7/7] commenting restore temporarily --- .github/workflows/linux-build.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/linux-build.yml b/.github/workflows/linux-build.yml index 264286296008..11a1c5585886 100644 --- a/.github/workflows/linux-build.yml +++ b/.github/workflows/linux-build.yml @@ -65,11 +65,11 @@ jobs: # TODO: Install a newer cmake here until we update the images upstream pip install cmake==3.30.4 - - uses: assignUser/stash/restore@v1 - with: - token: '${{ secrets.ARTIFACT_CACHE_TOKEN }}' - path: '${{ env.CCACHE_DIR }}' - key: ccache-linux-adapters + # - uses: assignUser/stash/restore@v1 + # with: + # token: '${{ secrets.ARTIFACT_CACHE_TOKEN }}' + # path: '${{ env.CCACHE_DIR }}' + # key: ccache-linux-adapters - name: "Zero Ccache Statistics" run: |