Skip to content

feat: Return not implemented errors instead of internal errors #153

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.

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

Merged
merged 4 commits into from
Mar 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 22 additions & 12 deletions src/from_substrait.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@
case PhysicalType::INT128:
return Value::DECIMAL(substrait_value, substrait_decimal.precision(), substrait_decimal.scale());
default:
throw InternalException("Not accepted internal type for decimal");
throw NotImplementedException("Unsupported internal type for decimal: %s", decimal_type.ToString());
}
}
case substrait::Expression_Literal::LiteralTypeCase::kBoolean: {
Expand Down Expand Up @@ -168,13 +168,14 @@
interval_t interval {};
interval.months = 0;
interval.days = literal.interval_day_to_second().days();
interval.micros = literal.interval_day_to_second().microseconds();

Check warning on line 171 in src/from_substrait.cpp

View workflow job for this annotation

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

'microseconds' is deprecated [-Wdeprecated-declarations]

Check warning on line 171 in src/from_substrait.cpp

View workflow job for this annotation

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

'microseconds' is deprecated [-Wdeprecated-declarations]

Check warning on line 171 in src/from_substrait.cpp

View workflow job for this annotation

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

'microseconds' is deprecated [-Wdeprecated-declarations]

Check warning on line 171 in src/from_substrait.cpp

View workflow job for this annotation

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

'microseconds' is deprecated [-Wdeprecated-declarations]
return Value::INTERVAL(interval);
}
case substrait::Expression_Literal::LiteralTypeCase::kVarChar:
return {literal.var_char().value()};
default:
throw SyntaxException("literals of this type number are not implemented: " + to_string(literal.literal_type_case()));
throw NotImplementedException("literals of this type are not implemented: %s",
substrait::Expression_Literal::GetDescriptor()->FindFieldByNumber(literal.literal_type_case())->name());
}
}

Expand All @@ -184,7 +185,7 @@

unique_ptr<ParsedExpression> SubstraitToDuckDB::TransformSelectionExpr(const substrait::Expression &sexpr) {
if (!sexpr.selection().has_direct_reference() || !sexpr.selection().direct_reference().has_struct_field()) {
throw InternalException("Can only have direct struct references in selections");
throw SyntaxException("Can only have direct struct references in selections");
}
return make_uniq<PositionalReferenceExpression>(sexpr.selection().direct_reference().struct_field().field() + 1);
}
Expand Down Expand Up @@ -322,7 +323,8 @@
case substrait::Type::KindCase::kFp64:
return {LogicalTypeId::DOUBLE};
default:
throw NotImplementedException("Substrait type not yet supported");
throw NotImplementedException("Substrait type not yet supported: %s",
substrait::Type::GetDescriptor()->FindFieldByNumber(s_type.kind_case())->name());
}
}

Expand Down Expand Up @@ -408,13 +410,15 @@
return TransformNested(sexpr, iterator);
case substrait::Expression::RexTypeCase::kSubquery:
default:
throw InternalException("Unsupported expression type " + to_string(sexpr.rex_type_case()));
throw NotImplementedException("Unsupported expression type %s",
substrait::Expression::GetDescriptor()->FindFieldByNumber(sexpr.rex_type_case())->name());
}
}

string SubstraitToDuckDB::FindFunction(uint64_t id) {
if (functions_map.find(id) == functions_map.end()) {
throw InternalException("Could not find aggregate function " + to_string(id));
throw NotImplementedException("Could not find aggregate function %s",
to_string(id));
}
return functions_map[id];
}
Expand Down Expand Up @@ -442,7 +446,8 @@
dnullorder = OrderByNullType::NULLS_LAST;
break;
default:
throw InternalException("Unsupported ordering " + to_string(sordf.direction()));
throw NotImplementedException("Unsupported ordering %s",
substrait::SortField::GetDescriptor()->FindFieldByNumber(sordf.direction())->name());
}

return {dordertype, dnullorder, TransformExpr(sordf.expr())};
Expand Down Expand Up @@ -472,7 +477,8 @@
djointype = JoinType::OUTER;
break;
default:
throw InternalException("Unsupported join type");
throw NotImplementedException("Unsupported join type: %s",
substrait::JoinRel::GetDescriptor()->FindFieldByNumber(sjoin.type())->name());
}
unique_ptr<ParsedExpression> join_condition = TransformExpr(sjoin.expression());
return make_shared_ptr<JoinRelation>(TransformOp(sjoin.left())->Alias("left"),
Expand Down Expand Up @@ -543,7 +549,8 @@
case substrait::Rel::RelTypeCase::kUpdate:
case substrait::Rel::RelTypeCase::kDdl:
default:
throw InternalException("Unsupported relation type " + to_string(sop.rel_type_case()));
throw NotImplementedException("Unsupported relation type %s",
substrait::Rel::GetDescriptor()->FindFieldByNumber(sop.rel_type_case())->name());
}
}

Expand Down Expand Up @@ -840,7 +847,8 @@
return SetOperationType::INTERSECT;
}
default: {
throw NotImplementedException("SetOperationType transform not implemented for SetRel_SetOp type %d", setop);
throw NotImplementedException("SetOperationType transform not implemented for SetRel_SetOp type %s",
substrait::SetRel::GetDescriptor()->FindFieldByNumber(setop)->name());
}
}
}
Expand Down Expand Up @@ -903,7 +911,8 @@
}
}
default:
throw NotImplementedException("Unsupported write operation " + to_string(swrite.op()));
throw NotImplementedException("Unsupported write operation %s",
substrait::WriteRel::GetDescriptor()->FindFieldByNumber(swrite.op())->name());
}
}

Expand Down Expand Up @@ -931,7 +940,8 @@
case substrait::Rel::RelTypeCase::kWrite:
return TransformWriteOp(sop);
default:
throw InternalException("Unsupported relation type " + to_string(sop.rel_type_case()));
throw NotImplementedException("Unsupported relation type %s",
substrait::Rel::GetDescriptor()->FindFieldByNumber(sop.rel_type_case())->name());
}
}

Expand Down
Loading
Loading