-
Notifications
You must be signed in to change notification settings - Fork 46
Add transform_row and scalar_parser documentation and make them easier to use #118
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
Conversation
Also fix an issue with `Query#transform_rows`. Co-authored-by: fionaochs <fionaochs@github.com>
Co-authored-by: fionaochs <fionaochs@github.com>
|
👋 I noticed that we didn't increase the version number for the last change I did:
Do you want me to bump that to 2.1.1 in this PR? |
| def column_value_parsers | ||
| @column_value_parsers ||= columns.map {|column| | ||
| ColumnValueParser.new(column) | ||
| ColumnValueParser.new(column, scalar_parser) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
scalar_parser is replaceable but looks like ColumnValueParser won't be recreated once instanciated even if scalar_parser is replaced.
client.query("select * from sys.node") do |q|
q.each_row {|row|
q.scalar_parser = scalar_parser1
p q.transform_row(row)
# Expect scalar_parser2 is used but scalar_parser1 is actually used?
q.scalar_parser = scalar_parser2
p q.transform_row(row)
}
endI know this example is artificial but behavior is counterintuitive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks very good. Thank you!
takezoe
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
@kmcq We released v2.2.0. Thanks for your contribution. |
Purpose
As a follow up to #117, we wanted to document the new capabilities in the README and to make it a bit easier to use the new
scalar_parserconfiguration of queries.Overview
attr_accessorforscalar_parsertoQuerycolumn_value_parsersQuery#transform_rowswas incorrect (sorry!)transform_rowandscalar_parserto the README.@fionaochs paired on this with me 🎉
Checklist