Skip to content

Commit

Permalink
apacheGH-35915: [Ruby] Add support for converting function options fr…
Browse files Browse the repository at this point in the history
…om Hash automatically

This also fixes a crash bug with `CallExpression.new(name, args,
Arrow::MatchSubstringOptions.new)`. `Arrow::MatchSubstringOptions.new`
is freed multiple times.
  • Loading branch information
kou committed Jun 6, 2023
1 parent e2ae492 commit 8f11133
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 4 deletions.
2 changes: 1 addition & 1 deletion c_glib/arrow-glib/expression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ garrow_call_expression_new(const gchar *function,
}
std::shared_ptr<arrow::compute::FunctionOptions> arrow_options;
if (options) {
arrow_options.reset(garrow_function_options_get_raw(options));
arrow_options.reset(garrow_function_options_get_raw(options)->Copy().release());
}
auto arrow_expression = arrow::compute::call(function,
arrow_arguments,
Expand Down
8 changes: 6 additions & 2 deletions ruby/red-arrow/lib/arrow/expression.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,14 @@ def try_convert(value)
else
return nil
end
options = nil
if arguments.last.is_a?(FunctionOptions)
options = arguments.pop
else
options = nil
elsif arguments.last.is_a?(Hash)
function = Function.find(function_name)
if function
options = function.resolve_options(arguments.pop)
end
end
CallExpression.new(function_name, arguments, options)
else
Expand Down
1 change: 0 additions & 1 deletion ruby/red-arrow/lib/arrow/function.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ def execute(args, options=nil, context=nil)
end
alias_method :call, :execute

private
def resolve_options(options)
return nil if options.nil?
return options if options.is_a?(FunctionOptions)
Expand Down
11 changes: 11 additions & 0 deletions ruby/red-arrow/test/test-expression.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,16 @@ class TestExpression < Test::Unit::TestCase
assert_equal(Arrow::CallExpression.new("func", ["argument1"]),
Arrow::Expression.try_convert(["func", "argument1"]))
end

test("[Symbol, String, Hash]") do
options = Arrow::MatchSubstringOptions.new
options.pattern = "hello"
assert_equal(Arrow::CallExpression.new("match_substring",
["content"],
options),
Arrow::Expression.try_convert([:match_substring,
"content",
{pattern: "hello"}]))
end
end
end

0 comments on commit 8f11133

Please sign in to comment.