Upgrade DuckDB dependency to 1.1.1

Summary: Upgrading DuckDB dependency to 1.1.1 and fixing incompatibilities with the new version. Differential Revision: D62529188
pedroerp · Nov 19, 2024 · cf084b7 · cf084b7
1 parent 463ebad
commit cf084b7
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 20 deletions.
diff --git a/CMake/resolve_dependency_modules/duckdb.cmake b/CMake/resolve_dependency_modules/duckdb.cmake
@@ -13,9 +13,9 @@
 # limitations under the License.
 include_guard(GLOBAL)
 
-set(VELOX_DUCKDB_VERSION 0.8.1)
+set(VELOX_DUCKDB_VERSION 1.1.1)
 set(VELOX_DUCKDB_BUILD_SHA256_CHECKSUM
-    a0674f7e320dc7ebcf51990d7fc1c0e7f7b2c335c08f5953702b5285e6c30694)
+    a764cef80287ccfd8555884d8facbe962154e7c747043c0842cd07873b4d6752)
 set(VELOX_DUCKDB_SOURCE_URL
     "https://github.com/duckdb/duckdb/archive/refs/tags/v${VELOX_DUCKDB_VERSION}.tar.gz"
 )

diff --git a/velox/duckdb/conversion/DuckParser.cpp b/velox/duckdb/conversion/DuckParser.cpp
@@ -195,22 +195,47 @@ std::shared_ptr<const core::IExpr> parseColumnRefExpr(
               colRefExpr.GetTableName(), std::nullopt)});
 }
 
+// DuckDB parses intervals such as "INTERVAL 5 HOURS" as the following
+// expression:
+//
+// > "to_hours(CAST(trunc(CAST(5 AS DOUBLE)) AS BIGINT))"
+//
+// This format is also valid:
+//
+// > "to_seconds(CAST(7 AS DOUBLE))"
+//
+// So we need to unpack the tree to fetch the right interval value. `input`
+// represents the only parameter to the top level function `functionName`.
 std::shared_ptr<const core::ConstantExpr> tryParseInterval(
     const std::string& functionName,
     const std::shared_ptr<const core::IExpr>& input,
     std::optional<std::string> alias) {
   std::optional<int64_t> value;
-  if (auto constInput = dynamic_cast<const core::ConstantExpr*>(input.get())) {
-    if (constInput->type()->isBigint() && !constInput->value().isNull()) {
-      value = constInput->value().value<int64_t>();
-    }
-  } else if (
-      auto castInput = dynamic_cast<const core::CastExpr*>(input.get())) {
-    if (castInput->type()->isBigint()) {
+
+  // Extracts the literal from inside the intermediate innocuous cast.
+  auto tryExtractFromCast =
+      [](const core::IExpr* input) -> std::optional<int64_t> {
+    if (auto castInput = dynamic_cast<const core::CastExpr*>(input)) {
       if (auto constInput = dynamic_cast<const core::ConstantExpr*>(
               castInput->getInput().get())) {
-        if (constInput->type()->isBigint() && !constInput->value().isNull()) {
-          value = constInput->value().value<int64_t>();
+        if (!constInput->value().isNull() && constInput->type()->isBigint()) {
+          return constInput->value().value<int64_t>();
+        }
+      }
+    }
+    return std::nullopt;
+  };
+
+  // Try to extract the format "CAST(7 AS DOUBLE)".
+  value = tryExtractFromCast(input.get());
+
+  // Try to extracts the format "CAST(trunc(CAST(5 AS DOUBLE)) AS BIGINT))".
+  if (!value.has_value()) {
+    if (auto castInput = dynamic_cast<const core::CastExpr*>(input.get())) {
+      if (auto callInput = dynamic_cast<const core::CallExpr*>(
+              castInput->getInput().get())) {
+        if (callInput->getFunctionName() == "trunc") {
+          value = tryExtractFromCast(callInput->getInput().get());
         }
       }
     }
@@ -289,9 +314,11 @@ std::shared_ptr<const core::IExpr> parseBetweenExpr(
   const auto& betweenExpr = dynamic_cast<BetweenExpression&>(expr);
   return callExpr(
       "between",
-      {parseExpr(*betweenExpr.input, options),
-       parseExpr(*betweenExpr.lower, options),
-       parseExpr(*betweenExpr.upper, options)},
+      {
+          parseExpr(*betweenExpr.input, options),
+          parseExpr(*betweenExpr.lower, options),
+          parseExpr(*betweenExpr.upper, options),
+      },
       getAlias(expr),
       options);
 }

diff --git a/velox/duckdb/conversion/tests/DuckParserTest.cpp b/velox/duckdb/conversion/tests/DuckParserTest.cpp
@@ -628,8 +628,7 @@ TEST(DuckParserTest, windowWithIntegerConstant) {
 
 TEST(DuckParserTest, invalidExpression) {
   VELOX_ASSERT_THROW(
-      parseExpr("func(a b)"),
-      "Cannot parse expression: func(a b). Parser Error: syntax error at or near \"b\"");
+      parseExpr("func(a b)"), "Cannot parse expression: func(a b).");
 }
 
 TEST(DuckParserTest, parseDecimalConstant) {

diff --git a/velox/parse/DuckLogicalOperator.h b/velox/parse/DuckLogicalOperator.h
@@ -97,7 +97,7 @@ class LogicalGet : public LogicalOperator {
   TableFilterSet table_filters;
 
   string GetName() const override;
-  string ParamsToString() const override;
+  InsertionOrderPreservingMap<string> ParamsToString() const override;
   //! Returns the underlying table that is being scanned, or nullptr if there is
   //! none
   TableCatalogEntry* GetTable() const;
@@ -176,7 +176,7 @@ class LogicalAggregate : public LogicalOperator {
   vector<unique_ptr<BaseStatistics>> group_stats;
 
  public:
-  string ParamsToString() const override;
+  InsertionOrderPreservingMap<string> ParamsToString() const override;
 
   vector<ColumnBinding> GetColumnBindings() override;
 

diff --git a/velox/parse/QueryPlanner.cpp b/velox/parse/QueryPlanner.cpp
@@ -465,11 +465,14 @@ static void customScalarFunction(
   VELOX_UNREACHABLE();
 }
 
-static ::duckdb::idx_t customAggregateState() {
+static ::duckdb::idx_t customAggregateState(
+    const ::duckdb::AggregateFunction&) {
   VELOX_UNREACHABLE();
 }
 
-static void customAggregateInitialize(::duckdb::data_ptr_t) {
+static void customAggregateInitialize(
+    const ::duckdb::AggregateFunction&,
+    ::duckdb::data_ptr_t) {
   VELOX_UNREACHABLE();
 }