-
Notifications
You must be signed in to change notification settings - Fork 22
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 overloadable _select_field_values method #13
Conversation
Codecov Report
@@ Coverage Diff @@
## master #13 +/- ##
==========================================
+ Coverage 83% 83.06% +0.05%
==========================================
Files 3 3
Lines 924 927 +3
Branches 189 189
==========================================
+ Hits 767 770 +3
Misses 107 107
Partials 50 50
Continue to review full report at Codecov.
|
Pull Request Test Coverage Report for Build 14
💛 - Coveralls |
|
Not sure about the name, maybe |
lib/SQL/Abstract.pm
Outdated
| : $fields; | ||
| my $sql = join(' ', $self->_sqlcase('select'), $f, | ||
| my ($fields_sql, @fields_bind) = $self->_select_field_values($fields); | ||
| push @bind, @fields_bind; |
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.
Because $fields_sql comes before $where_sql, @fields_bind need to come before the bind values returned by ->where(). I'd suggest moving the whole fields part before the where part, to make this clearer.
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.
Oh, of course.
…alues and _insert_values
|
Ok, fixed and squashed. |
|
Thanks, merged with minor tweaks as daa4ccd. |
This patch is very similar to #12. It adds a new method
_select_field_valuesto be used as an extension hook, similar to_update_set_valuesand_insert_values.