Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add support for to_substrait for virtual table expression #124

Closed
wants to merge 1 commit into from
Closed

feat: Add support for to_substrait for virtual table expression #124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/include/to_substrait.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ class DuckDBToSubstrait {
substrait::Rel *TransformOrderBy(LogicalOperator &dop);
substrait::Rel *TransformComparisonJoin(LogicalOperator &dop);
substrait::Rel *TransformAggregateGroup(LogicalOperator &dop);
substrait::Rel *TransformExpressionGet(LogicalOperator &dop);
substrait::Rel *TransformGet(LogicalOperator &dop);
substrait::Rel *TransformCrossProduct(LogicalOperator &dop);
substrait::Rel *TransformUnion(LogicalOperator &dop);
Expand Down
21 changes: 21 additions & 0 deletions src/to_substrait.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@
} else {
auto interval_day = make_uniq<substrait::Expression_Literal_IntervalDayToSecond>();
interval_day->set_days(dval.GetValue<interval_t>().days);
interval_day->set_microseconds(static_cast<int32_t>(dval.GetValue<interval_t>().micros));

Check warning on line 204 in src/to_substrait.cpp

View workflow job for this annotation

GitHub Actions / Build extension binaries / MacOS (osx_amd64, x86_64, x64-osx) 

'set_microseconds' is deprecated [-Wdeprecated-declarations]

Check warning on line 204 in src/to_substrait.cpp

View workflow job for this annotation

GitHub Actions / Build extension binaries / MacOS (osx_arm64, arm64, arm64-osx) 

'set_microseconds' is deprecated [-Wdeprecated-declarations]
sval.set_allocated_interval_day_to_second(interval_day.release());
}
}
Expand Down Expand Up @@ -1012,7 +1012,7 @@
// TODO push projection or push substrait to allow expressions here
throw NotImplementedException("No expressions in groupings yet");
}
TransformExpr(*dgrp, *sgrp->add_grouping_expressions());

Check warning on line 1015 in src/to_substrait.cpp

View workflow job for this annotation

GitHub Actions / Build extension binaries / MacOS (osx_amd64, x86_64, x64-osx) 

'add_grouping_expressions' is deprecated [-Wdeprecated-declarations]

Check warning on line 1015 in src/to_substrait.cpp

View workflow job for this annotation

GitHub Actions / Build extension binaries / MacOS (osx_arm64, arm64, arm64-osx) 

'add_grouping_expressions' is deprecated [-Wdeprecated-declarations]
}
for (auto &dmeas : daggr.expressions) {
auto smeas = saggr->add_measures()->mutable_measure();
Expand Down Expand Up @@ -1280,7 +1280,7 @@
auto virtual_table = sget->mutable_virtual_table();

// Add a dummy value to emit one row
auto dummy_value = virtual_table->add_values();

Check warning on line 1283 in src/to_substrait.cpp

View workflow job for this annotation

GitHub Actions / Build extension binaries / MacOS (osx_amd64, x86_64, x64-osx) 

'add_values' is deprecated [-Wdeprecated-declarations]

Check warning on line 1283 in src/to_substrait.cpp

View workflow job for this annotation

GitHub Actions / Build extension binaries / MacOS (osx_arm64, arm64, arm64-osx) 

'add_values' is deprecated [-Wdeprecated-declarations]
dummy_value->add_fields()->set_i32(42);
return get_rel;
}
Expand Down Expand Up @@ -1339,6 +1339,25 @@
return get_rel;
}

substrait::Rel *DuckDBToSubstrait::TransformExpressionGet(LogicalOperator &dop) {
auto get_rel = new substrait::Rel();
auto &dget = dop.Cast<LogicalExpressionGet>();

auto sget = get_rel->mutable_read();
auto virtual_table = sget->mutable_virtual_table();

for (auto &row : dget.expressions) {
auto row_item = virtual_table->add_expressions();
for (auto &expr : row) {
auto s_expr = new substrait::Expression();
TransformExpr(*expr, *s_expr);
*row_item->add_fields() = *s_expr;
delete s_expr;
}
}
return get_rel;
}

substrait::Rel *DuckDBToSubstrait::TransformCrossProduct(LogicalOperator &dop) {
auto rel = new substrait::Rel();
auto sub_cross_prod = rel->mutable_cross();
Expand Down Expand Up @@ -1435,6 +1454,8 @@
return TransformAggregateGroup(dop);
case LogicalOperatorType::LOGICAL_GET:
return TransformGet(dop);
case LogicalOperatorType::LOGICAL_EXPRESSION_GET:
return TransformExpressionGet(dop);
case LogicalOperatorType::LOGICAL_CROSS_PRODUCT:
return TransformCrossProduct(dop);
case LogicalOperatorType::LOGICAL_UNION:
Expand Down
21 changes: 21 additions & 0 deletions test/c/test_substrait_c_api.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include <chrono>
#include <thread>
#include <iostream>

using namespace duckdb;
using namespace std;
Expand Down Expand Up @@ -45,3 +46,23 @@ TEST_CASE("Test C Get and To Json-Substrait API", "[substrait-api]") {

REQUIRE_THROWS(con.FromSubstraitJSON("this is not valid"));
}


TEST_CASE("Test C VirtualTable input Literal", "[substrait-api]") {
DuckDB db(nullptr);
Connection con(db);

auto json = con.GetSubstraitJSON("select * from (values (1, 2),(3, 4))");
REQUIRE(!json.empty());
std::cout << json << std::endl;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to test anything else besides that there wasn't an error during parsing? Perhaps call GetSubstrait and check a few things like the number of rows/columns is right, that the type of the read operation is a virtual table, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will done as part of from_substrait PR. Waiting on a change from duckdb (Constructor having RelationContextWrapper and ParsedExpression) once it is in, same test case will be enhanced to check roundtrip

}

TEST_CASE("Test C VirtualTable input Expression", "[substrait-api]") {
DuckDB db(nullptr);
Connection con(db);

auto json = con.GetSubstraitJSON("select * from (values (1+1,2+2),(3+3,4+4)) as temp(a,b)");
REQUIRE(!json.empty());
std::cout << json << std::endl;

}
Loading