fix: Fix SQL syntax error when UPDATE has no fields to update#2422
fix: Fix SQL syntax error when UPDATE has no fields to update#2422an-tao merged 1 commit intodrogonframework:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a SQL syntax error that occurs when UPDATE operations have no fields to update by adding early return checks when the column list is empty. The fix prevents generating invalid SQL like "UPDATE table SET WHERE ..." which would cause database errors.
Key changes:
- Adds early return logic for empty column lists across all update and increment methods
- Modifies update-related methods to return 0 affected rows when no columns need updating
- Applies the fix to synchronous, callback-based, and future-based variants
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| static_assert(sizeof...(args) > 0); | ||
| assert(colNames.size() == sizeof...(args)); | ||
| clear(); | ||
| if (colNames.empty()) | ||
| { | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
The empty check on line 1384 is unreachable due to the static_assert on line 1381 and the runtime assert on line 1382. If colNames is empty, then sizeof...(args) must be 0 (due to the assert), which would violate the static_assert that requires sizeof...(args) > 0. Consider removing the empty check or adjusting the assertions to make this logic consistent.
| if (colNames.empty()) | ||
| { | ||
| rcb(0); | ||
| return; | ||
| } |
There was a problem hiding this comment.
The empty check on line 1465 is unreachable due to the static_assert on line 1462 and the runtime assert on line 1463. If colNames is empty, then sizeof...(args) must be 0 (due to the assert), which would violate the static_assert that requires sizeof...(args) > 0. Consider removing the empty check or adjusting the assertions to make this logic consistent.
| if (colNames.empty()) | |
| { | |
| rcb(0); | |
| return; | |
| } |
| assert(colNames.size() == sizeof...(args)); | ||
| clear(); | ||
| if (colNames.empty()) | ||
| { | ||
| std::shared_ptr<std::promise<size_t>> prom = | ||
| std::make_shared<std::promise<size_t>>(); | ||
| prom->set_value(0); | ||
| return prom->get_future(); | ||
| } |
There was a problem hiding this comment.
The empty check on line 1545 is unreachable due to the static_assert on line 1542 and the runtime assert on line 1543. If colNames is empty, then sizeof...(args) must be 0 (due to the assert), which would violate the static_assert that requires sizeof...(args) > 0. Consider removing the empty check or adjusting the assertions to make this logic consistent.
| assert(colNames.size() == sizeof...(args)); | |
| clear(); | |
| if (colNames.empty()) | |
| { | |
| std::shared_ptr<std::promise<size_t>> prom = | |
| std::make_shared<std::promise<size_t>>(); | |
| prom->set_value(0); | |
| return prom->get_future(); | |
| } | |
| assert(!colNames.empty()); | |
| assert(colNames.size() == sizeof...(args)); | |
| clear(); |
| static_assert(sizeof...(args) > 0); | ||
| assert(colNames.size() == sizeof...(args)); | ||
| clear(); | ||
| if (colNames.empty()) | ||
| { | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
The empty check on line 1592 is unreachable due to the static_assert on line 1589 and the runtime assert on line 1590. If colNames is empty, then sizeof...(args) must be 0 (due to the assert), which would violate the static_assert that requires sizeof...(args) > 0. Consider removing the empty check or adjusting the assertions to make this logic consistent.
| if (colNames.empty()) | ||
| { | ||
| rcb(0); | ||
| return; | ||
| } |
There was a problem hiding this comment.
The empty check on line 1650 is unreachable due to the static_assert on line 1647 and the runtime assert on line 1648. If colNames is empty, then sizeof...(args) must be 0 (due to the assert), which would violate the static_assert that requires sizeof...(args) > 0. Consider removing the empty check or adjusting the assertions to make this logic consistent.
| if (colNames.empty()) | |
| { | |
| rcb(0); | |
| return; | |
| } |
| static_assert(sizeof...(args) > 0); | ||
| assert(colNames.size() == sizeof...(args)); | ||
| clear(); | ||
| if (colNames.empty()) | ||
| { | ||
| std::shared_ptr<std::promise<size_t>> prom = | ||
| std::make_shared<std::promise<size_t>>(); | ||
| prom->set_value(0); | ||
| return prom->get_future(); | ||
| } |
There was a problem hiding this comment.
The empty check on line 1703 is unreachable due to the static_assert on line 1700 and the runtime assert on line 1701. If colNames is empty, then sizeof...(args) must be 0 (due to the assert), which would violate the static_assert that requires sizeof...(args) > 0. Consider removing the empty check or adjusting the assertions to make this logic consistent.
|
@1980243524 Thanks so much for your patch, please also modify the coroutine version (CoroMapper.h) |
|
@an-tao The update method provided by the user (which is used to update specified fields) throws an exception when there are no fields to update. However, the update method implemented with the dirty flag directly returns without throwing an exception. Is such an inconsistency permissible? |
|
@an-tao The updated fields provided by the user are visible to the user, whereas the update operation implemented via the dirty flag is invisible to the user (unless the user manually sets a dirty flag themselves). |
This is currently designed because the former is likely to be a mistake, while the latter may be the result of repeated calls to an update interface. |
|
@an-tao Ok, I've finished the remaining parts of the code as per your previous suggestions. |
fix: Fix SQL syntax error when UPDATE has no fields to update (drogonframework#2422)
fix #2421