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.

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

chore: prepare 0.0.11 release #65

Merged
merged 7 commits into from
Nov 29, 2022
Merged
Show file tree
Hide file tree
Changes from 5 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
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ older version. Refer to the table below for the version compatibility matrix.

| Substrait... | ... is supported by validator ... |
| -------------- | ------------------------------------ |
| 0.20.x | 0.0.11 (current version) |
| 0.19.x | 0.0.10 |
| 0.18.x | 0.0.9 |
| 0.9.x - 0.17.x | 0.0.8 |
Expand Down
17 changes: 17 additions & 0 deletions rs/src/output/extension/simple/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ pub struct Definition {
/// The expected arguments of the function.
pub arguments: Vec<ArgumentSlot>,

/// The options of the function.
pub options: HashMap<OptionName, OptionValues>,

/// Specifies the variadic behavior of the last argument slot, if any.
pub variadic: VariadicBehavior,

Expand Down Expand Up @@ -136,6 +139,20 @@ pub struct EnumerationArgumentSlot {
pub required: bool,
}

/// Definition of a function option name.
#[derive(Clone, Debug)]
pub struct OptionName {
/// A human-readable name for this option.
pub name: String,
}

/// Definition of a valid options for a function option.
#[derive(Clone, Debug)]
pub struct OptionValues {
/// A list of valid strings for this option.
pub values: Vec<String>,
}

/// Definition of the variadic behavior of the last argument slot.
#[derive(Clone, Debug)]
pub struct VariadicBehavior {
Expand Down
111 changes: 74 additions & 37 deletions rs/src/parse/expressions/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ pub enum FunctionArgument {
/// Used for type arguments.
Type(data::Type),

/// Used for enum option arguments.
Enum(Option<String>),
/// Used for enum arguments.
Enum(String),
}

impl Default for FunctionArgument {
Expand All @@ -61,8 +61,7 @@ impl Describe for FunctionArgument {
FunctionArgument::Unresolved => write!(f, "!"),
FunctionArgument::Value(_, e) => e.describe(f, limit),
FunctionArgument::Type(t) => t.describe(f, limit),
FunctionArgument::Enum(Some(s)) => util::string::describe_identifier(f, s, limit),
FunctionArgument::Enum(None) => write!(f, "-"),
FunctionArgument::Enum(s) => util::string::describe_identifier(f, s, limit),
}
}
}
Expand All @@ -73,6 +72,16 @@ impl std::fmt::Display for FunctionArgument {
}
}

/// An optional function argument. Typically used for specifying behavior in
/// invalid or corner cases.
#[derive(Clone, Debug)]
pub struct FunctionOption {
/// Name of the option to set.
pub name: String,
/// List of behavior options allowed by the producer.
pub preference: Vec<String>,
}

/// Information about the context in which a function is being called.
#[derive(Clone, Debug)]
pub struct FunctionContext {
Expand All @@ -82,6 +91,9 @@ pub struct FunctionContext {
/// The list of arguments bound to the function.
pub arguments: Vec<FunctionArgument>,

/// The list of optional function arguments.
pub options: Vec<FunctionOption>,

/// If known, the expected return type of the function. If not known this
/// can just be set to unresolved.
pub return_type: data::Type,
Expand All @@ -101,17 +113,16 @@ pub struct FunctionBinding {
}

impl FunctionBinding {
/// Try to bind one of the provided function implementations to the
/// provided function context.
/// Try to bind one of the provided function implementations to the provided
/// function context.
///
/// This is purely a validator thing. For valid plans, there should only
/// ever be one implementation after name resolution, and the return type
/// should already have been specified. Much more intelligence was thrown
/// in here just to help people find and correct mistakes efficiently.
/// Common misconceptions and mistakes, like using the simple function name
/// vs. the compound name, not specifying optional arguments, or not
/// specifying the (correct) return type should yield more than just a
/// generic error message here!
/// should already have been specified. Much more intelligence was thrown in
/// here just to help people find and correct mistakes efficiently. Common
/// misconceptions and mistakes, like using the simple function name vs. the
/// compound name. or not specifying the (correct) return type should yield
/// more than just a generic error message here!
pub fn new(
functions: Option<&extension::simple::function::ResolutionResult>,
function_context: &FunctionContext,
Expand Down Expand Up @@ -154,36 +165,13 @@ impl FunctionBinding {
}
}

/// Parse an enum option argument type.
fn parse_enum_type(
x: &substrait::function_argument::r#enum::EnumKind,
_y: &mut context::Context,
) -> diagnostic::Result<Option<String>> {
match x {
substrait::function_argument::r#enum::EnumKind::Specified(x) => Ok(Some(x.clone())),
substrait::function_argument::r#enum::EnumKind::Unspecified(_) => Ok(None),
}
}

/// Parse an enum option argument.
fn parse_enum(
x: &substrait::function_argument::Enum,
y: &mut context::Context,
) -> diagnostic::Result<Option<String>> {
Ok(proto_required_field!(x, y, enum_kind, parse_enum_type)
.1
.flatten())
}

/// Parse a 0.3.0+ function argument type.
fn parse_function_argument_type(
x: &substrait::function_argument::ArgType,
y: &mut context::Context,
) -> diagnostic::Result<FunctionArgument> {
match x {
substrait::function_argument::ArgType::Enum(x) => {
Ok(FunctionArgument::Enum(parse_enum(x, y)?))
}
substrait::function_argument::ArgType::Enum(x) => Ok(FunctionArgument::Enum(x.to_string())),
substrait::function_argument::ArgType::Type(x) => {
types::parse_type(x, y)?;
Ok(FunctionArgument::Type(y.data_type()))
Expand All @@ -207,14 +195,45 @@ fn parse_function_argument(
)
}

fn parse_function_option(
x: &substrait::FunctionOption,
y: &mut context::Context,
) -> diagnostic::Result<FunctionOption> {
if x.preference.is_empty() {
jvanstraten marked this conversation as resolved.
Show resolved Hide resolved
let err = cause!(IllegalValue, "at least one option must be specified");
diagnostic!(y, Error, err.clone());
comment!(
y,
"To leave an option unspecified, simply don't add an entry to `options`"
);
Err(err)
} else {
Ok(FunctionOption {
name: x.name.clone(),
preference: x.preference.clone(),
})
}
}

/// Parse a pre-0.3.0 function argument expression.
fn parse_legacy_function_argument(
x: &substrait::Expression,
y: &mut context::Context,
) -> diagnostic::Result<FunctionArgument> {
expressions::parse_legacy_function_argument(x, y).map(|x| match x {
expressions::ExpressionOrEnum::Value(x) => FunctionArgument::Value(y.data_type(), x),
expressions::ExpressionOrEnum::Enum(x) => FunctionArgument::Enum(x),
expressions::ExpressionOrEnum::Enum(x) => match x {
Some(x) => FunctionArgument::Enum(x),
None => {
diagnostic!(
y,
Error,
Deprecation,
"support for optional enum arguments was removed in Substrait 0.20.0 (#342)"
);
FunctionArgument::Unresolved
}
},
})
}

Expand Down Expand Up @@ -279,6 +298,11 @@ pub fn parse_scalar_function(
.into_iter()
.map(|x| x.unwrap_or_default())
.collect();
let options = proto_repeated_field!(x, y, options, parse_function_option)
.1
.into_iter()
.flatten()
.collect();
let return_type = proto_required_field!(x, y, output_type, types::parse_type)
.0
.data_type();
Expand All @@ -288,6 +312,7 @@ pub fn parse_scalar_function(
let context = FunctionContext {
function_type: FunctionType::Scalar,
arguments,
options,
return_type,
};
let binding = FunctionBinding::new(functions.as_ref(), &context, y);
Expand Down Expand Up @@ -341,6 +366,11 @@ pub fn parse_window_function(
.into_iter()
.map(|x| x.unwrap_or_default())
.collect();
let options = proto_repeated_field!(x, y, options, parse_function_option)
.1
.into_iter()
.flatten()
.collect();
let return_type = proto_required_field!(x, y, output_type, types::parse_type)
.0
.data_type();
Expand All @@ -359,6 +389,7 @@ pub fn parse_window_function(
let context = FunctionContext {
function_type: FunctionType::Window,
arguments,
options,
return_type,
};
let binding = FunctionBinding::new(functions.as_ref(), &context, y);
Expand Down Expand Up @@ -395,6 +426,11 @@ pub fn parse_aggregate_function(
.into_iter()
.map(|x| x.unwrap_or_default())
.collect();
let options = proto_repeated_field!(x, y, options, parse_function_option)
.1
.into_iter()
.flatten()
.collect();
let return_type = proto_required_field!(x, y, output_type, types::parse_type)
.0
.data_type();
Expand All @@ -416,6 +452,7 @@ pub fn parse_aggregate_function(
let context = FunctionContext {
function_type: FunctionType::Aggregate,
arguments,
options,
return_type,
};
let binding = FunctionBinding::new(functions.as_ref(), &context, y);
Expand Down
15 changes: 15 additions & 0 deletions rs/src/parse/relations/read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,10 @@ pub fn parse_read_rel(x: &substrait::ReadRel, y: &mut context::Context) -> diagn
// Handle filter.
let filter = proto_boxed_field!(x, y, filter, expressions::parse_predicate);

// Handle best effort filter.
let best_effort_filter =
proto_boxed_field!(x, y, best_effort_filter, expressions::parse_predicate);

// Handle projection.
if x.projection.is_some() {
schema =
Expand Down Expand Up @@ -367,6 +371,17 @@ pub fn parse_read_rel(x: &substrait::ReadRel, y: &mut context::Context) -> diagn
);
}

// Add best effort filter summary.
if let (Some(best_effort_filter_node), Some(best_effort_filter_expr)) = best_effort_filter {
let nullable = best_effort_filter_node.data_type().nullable();
summary!(
y,
"This relation may discards all rows for which the expression {} yields {}.",
best_effort_filter_expr,
if nullable { "false or null" } else { "false" }
);
}

// Handle the common field.
handle_rel_common!(x, y);

Expand Down
1 change: 1 addition & 0 deletions rs/src/parse/sorts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ fn parse_comparison_function_reference(
let context = expressions::functions::FunctionContext {
function_type: expressions::functions::FunctionType::Scalar,
arguments: vec![argument.clone(), argument],
options: vec![],
return_type: data::new_unresolved_type(),
};
let binding = expressions::functions::FunctionBinding::new(Some(&functions), &context, y);
Expand Down
2 changes: 1 addition & 1 deletion rs/src/resources/substrait-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
0.19.0
0.20.0
2 changes: 1 addition & 1 deletion tests/tests/version/current-version.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ plan:
__test:
- level: i
version:
minorNumber: 19
minorNumber: 20
producer: validator-test