[Gravity] Logging mysqli warnings

Review Request #795 — Created Sept. 16, 2021 and submitted

b.stoop
Lunr
796
a016729...
lunr

Logging mysqli warnings

Unit tests

  • 0
  • 0
  • 7
  • 0
  • 7
Description From Last Updated
pprkut
  1. 
      
  2. src/Lunr/Gravity/Database/DatabaseAccessObject.php (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    You can still use $context here.

    Something like {query}; had {warning_count} warnings:

  3. src/Lunr/Gravity/Database/DatabaseAccessObject.php (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    Please move this out of the has_failed() check

    1. As in outside of the has_failed() check, or make a new method?

    2. Outside is fine

    3. I'm not sure if that would work, if the query is successful then it doesn't log the warnings because of the return; and as far as i know and tested there are no warnings if the query fails

    4. I could not find any proof that there are no warnings when the query fails

    5. Let's move it outside then :)

  4. src/Lunr/Gravity/Database/DatabaseQueryResultInterface.php (Diff revision 1)
     
     
     
     
     
     
     
     
     

    Separate review please :)

  5. 
      
b.stoop
pprkut
  1. 
      
  2. src/Lunr/Gravity/Database/DatabaseAccessObject.php (Diff revision 2)
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    Minor optimization:

    $warnings = $query->warnings();
    

    and then:

    if ($warnings !== NULL)...
    foreach ($warnings as ...
    count($warnings)...
    
  3. I don't think putting the warnings into context is the most elegant solution here.
    At the very least, every warning should appear on a new line, so not separated by ;.

  4. Now we also need a test for warnings on error :)

  5. 
      
b.stoop
b.stoop
pprkut
  1. 
      
  2. You can already define the {query}; had ... string here. Saves a string concat later

  3. 
      
b.stoop
pprkut
Review request changed

Status: Closed (submitted)

Change Summary:

Merged into master

Loading...