-
Notifications
You must be signed in to change notification settings - Fork 47
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 transform_row and scalar_parser documentation and make them easier to use #118
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? |
@@ -102,12 +104,12 @@ def columns | |||
|
|||
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)
}
end
I 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!
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_parser
configuration of queries.Overview
attr_accessor
forscalar_parser
toQuery
column_value_parsers
Query#transform_rows
was incorrect (sorry!)transform_row
andscalar_parser
to the README.@fionaochs paired on this with me 🎉
Checklist