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

add MAP/PLUCK functions #221

Closed
wants to merge 3 commits into from

Conversation

vlazar
Copy link
Contributor

@vlazar vlazar commented Oct 23, 2020

For PLUCK function I had to directly access hash keys with item[attr.to_sym] || item[attr]. Using Dentaku node.value(context) is not working for symbol keys as described here: #218 (comment)

I'll try to figure out how to fix this issue and use Dentaku in PLUCK function definition to pluck keys form hashes.

@codecov-io
Copy link

codecov-io commented Oct 24, 2020

Codecov Report

Merging #221 into master will increase coverage by 0.08%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #221      +/-   ##
==========================================
+ Coverage   96.46%   96.54%   +0.08%     
==========================================
  Files          58       60       +2     
  Lines        1641     1680      +39     
==========================================
+ Hits         1583     1622      +39     
  Misses         58       58              
Impacted Files Coverage Δ
lib/dentaku/ast.rb 100.00% <100.00%> (ø)
lib/dentaku/ast/functions/map.rb 100.00% <100.00%> (ø)
lib/dentaku/ast/functions/pluck.rb 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b901a59...c63067f. Read the comment docs.

@vlazar
Copy link
Contributor Author

vlazar commented Oct 24, 2020

I've fixed the issue with symbol keys not woking for PLUCK similar to how it works for MAP. See my last commit. I hope this is acceptable implementation (building expression in function seems weird, but I don't know what would be a better way).

Basically the issue boils down to this:

pp Dentaku::FlatHash.from_hash("u" => { age: 44 }) # => {"u.age"=>44}
pp Dentaku::AST::Identifier.new(Dentaku::Token.new(:identifier, "u.age")).value("u.age" => 44) # => 44

pp Dentaku::FlatHash.from_hash({ age: 44 }) # => {:age=>44}
# pp Dentaku::AST::Identifier.new(Dentaku::Token.new(:identifier, "age")).value({ age: 44 }) # => UnboundVariableError
pp Dentaku::AST::Identifier.new(Dentaku::Token.new(:identifier, :age)).value({ age: 44 }) # 44

pp Dentaku::FlatHash.from_hash(u: { age: 44 }) # => {:"u.age"=>44}
# pp Dentaku::AST::Identifier.new(Dentaku::Token.new(:identifier, "u.age")).value({:"u.age"=>44}) # => UnboundVariableError
pp Dentaku::AST::Identifier.new(Dentaku::Token.new(:identifier, :"u.age")).value({:"u.age"=>44}) # => 44

Since Dentaku::FlatHash.from_hash for PLUCK was originally called on each collection item and not { "i" => item } the keys in hash were not converted to strings. And Dentaku::AST::Identifier#value is just fetching a value from hash. So for symbol keys the value wasn't found by string key in my original implementation of PLUCK which was cargo culted from MAP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants