Skip to content

Conversation

@fabzzap
Copy link
Contributor

@fabzzap fabzzap commented Aug 29, 2018

An empty array ref as value for -and can cause incorrect SQL. Example:

$ perl -Mv5.12 -MSQL::Abstract -e 'my ($sql,@bind)=SQL::Abstract->new->where({-and=>[],-or=>[a=>1]});say $sql'
Use of uninitialized value in join or string at /usr/share/perl5/SQL/Abstract.pm line 1482.
 WHERE ( (  AND a = ? ) )

This is a proposed fix to that bug

@codecov-io
Copy link

codecov-io commented Aug 29, 2018

Codecov Report

Merging #15 into master will increase coverage by 0.12%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #15      +/-   ##
==========================================
+ Coverage   83.06%   83.18%   +0.12%     
==========================================
  Files           3        3              
  Lines         927      928       +1     
  Branches      189      189              
==========================================
+ Hits          770      772       +2     
+ Misses        107      106       -1     
  Partials       50       50
Impacted Files Coverage Δ
lib/SQL/Abstract.pm 82.47% <100%> (+0.18%) ⬆️

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 4805666...3e759b1. Read the comment docs.

@coveralls
Copy link

coveralls commented Aug 29, 2018

Pull Request Test Coverage Report for Build 28

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.2%) to 88.414%

Totals Coverage Status
Change from base Build 26: -0.2%
Covered Lines: 786
Relevant Lines: 889

💛 - Coveralls

@mohawk2
Copy link

mohawk2 commented Jan 29, 2019

Surely there should be tests? And rather than just silently dropping things, this should cause an exception?

@fabzzap
Copy link
Contributor Author

fabzzap commented Feb 5, 2019

I just pushed a test for the patch. I don't know what "silently drop[s] things" here. -and=>[] is supposed to have no effect, but the main code instead generates a warning and incorrect SQL. With the patch, it behaves correctly: it does nothing.

@dbsrgits-sync dbsrgits-sync force-pushed the master branch 2 times, most recently from cd96b8a to d291f33 Compare January 21, 2021 21:23
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.

4 participants