Skip to content

Commit b6901ac

Browse files
nmariznmariz
committed
BUG#24364556: Improve warning behavior
This patch introduces the following warning behaviour improvements: - Warnings now propagated using warnings module when get_warnings is set (raise_on_warnings behaves the same as before). - Do not disable get_warnings if raise_on_warnings is explicitly set to False. - errors.Warning subclasses Warning. - errors.get_mysql_exception can now return warnings where appropriate. - cursor_[cext].handle_warnings now also emits warnings, not just exceptions. - connection.close() does not leak socket if cmd_quit fails. Thank you for the contribution. Change-Id: I3a6b8e3630f1b851438d4e6a620d2d9601cbc926
1 parent bff8a18 commit b6901ac

File tree

5 files changed

+54
-22
lines changed

5 files changed

+54
-22
lines changed

‎CHANGES.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ v8.0.32
1919
- BUG#34499578: MySQLCursor.executemany() fails to correctly identify BULK data loading ops
2020
- BUG#30089671: Fix decoding VARBINARY columns when using a prepared cursor
2121
- BUG#28020811: Fix multiple reference leaks in the C extension
22+
- BUG#24364556: Improve warning behavior
2223

2324
v8.0.31
2425
=======

‎lib/mysql/connector/abstracts.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -994,7 +994,9 @@ def raise_on_warnings(self, value: bool) -> None:
994994
if not isinstance(value, bool):
995995
raise ValueError("Expected a boolean type")
996996
self._raise_on_warnings = value
997-
self._get_warnings = value
997+
# Don't disable warning retrieval if raising explicitly disabled
998+
if value:
999+
self._get_warnings = value
9981000

9991001
@property
10001002
def unread_result(self) -> bool:

‎lib/mysql/connector/cursor.py

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
from __future__ import annotations
3333

3434
import re
35+
import warnings
3536
import weakref
3637

3738
from collections import namedtuple
@@ -484,8 +485,6 @@ def _handle_noresultset(self, res: ResultType) -> None:
484485
raise ProgrammingError(f"Failed handling non-resultset; {err}") from None
485486

486487
self._handle_warnings()
487-
if self._connection.raise_on_warnings is True and self._warnings:
488-
raise get_mysql_exception(self._warnings[0][1], self._warnings[0][2])
489488

490489
def _handle_resultset(self) -> None:
491490
"""Handles result set
@@ -895,18 +894,34 @@ def _fetch_warnings(self) -> Optional[List[WarningType]]:
895894
return None
896895

897896
def _handle_warnings(self) -> None:
898-
"""Handle possible warnings after all results are consumed"""
899-
if self._connection.get_warnings is True and self._warning_count:
897+
"""Handle possible warnings after all results are consumed.
898+
899+
Raises:
900+
Error: Also raises exceptions if raise_on_warnings is set.
901+
"""
902+
if self._connection.get_warnings and self._warning_count:
900903
self._warnings = self._fetch_warnings()
901904

905+
if not self._warnings:
906+
return
907+
908+
err = get_mysql_exception(
909+
self._warnings[0][1],
910+
self._warnings[0][2],
911+
warning=not self._connection.raise_on_warnings,
912+
)
913+
914+
if self._connection.raise_on_warnings:
915+
raise err
916+
917+
warnings.warn(err, stacklevel=4)
918+
902919
def _handle_eof(self, eof: EofPacketType) -> None:
903920
"""Handle EOF packet"""
904921
self._connection.unread_result = False
905922
self._nextrow = (None, None)
906923
self._warning_count = eof["warning_count"]
907924
self._handle_warnings()
908-
if self._connection.raise_on_warnings is True and self._warnings:
909-
raise get_mysql_exception(self._warnings[0][1], self._warnings[0][2])
910925

911926
def _fetch_row(self, raw: bool = False) -> Optional[RowType]:
912927
"""Returns the next row in the result set

‎lib/mysql/connector/cursor_cext.py

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
from __future__ import annotations
3333

3434
import re
35+
import warnings
3536
import weakref
3637

3738
from collections import namedtuple
@@ -180,12 +181,12 @@ def _fetch_warnings(self) -> Optional[List[WarningType]]:
180181
181182
Returns list of tuples or None.
182183
"""
183-
warnings = []
184+
warns = []
184185
try:
185186
# force freeing result
186187
self._cnx.consume_results()
187188
_ = self._cnx.cmd_query("SHOW WARNINGS")
188-
warnings = self._cnx.get_rows()[0]
189+
warns = self._cnx.get_rows()[0]
189190
self._cnx.consume_results()
190191
except MySQLInterfaceError as err:
191192
raise get_mysql_exception(
@@ -194,16 +195,31 @@ def _fetch_warnings(self) -> Optional[List[WarningType]]:
194195
except Exception as err:
195196
raise InterfaceError(f"Failed getting warnings; {err}") from None
196197

197-
if warnings:
198-
return warnings
198+
if warns:
199+
return warns
199200

200201
return None
201202

202203
def _handle_warnings(self) -> None:
203-
"""Handle possible warnings after all results are consumed"""
204-
if self._cnx.get_warnings is True and self._warning_count:
204+
"""Handle possible warnings after all results are consumed.
205+
206+
Raises:
207+
Error: Also raises exceptions if raise_on_warnings is set.
208+
"""
209+
if self._cnx.get_warnings and self._warning_count:
205210
self._warnings = self._fetch_warnings()
206211

212+
if not self._warnings:
213+
return
214+
215+
err = get_mysql_exception(
216+
*self._warnings[0][1:3], warning=not self._cnx.raise_on_warnings
217+
)
218+
if self._cnx.raise_on_warnings:
219+
raise err
220+
221+
warnings.warn(err, stacklevel=4)
222+
207223
def _handle_result(self, result: Union[CextEofPacketType, CextResultType]) -> None:
208224
"""Handles the result after statement execution"""
209225
if "columns" in result:
@@ -216,8 +232,6 @@ def _handle_result(self, result: Union[CextEofPacketType, CextResultType]) -> No
216232
self._affected_rows = result["affected_rows"]
217233
self._rowcount = -1
218234
self._handle_warnings()
219-
if self._cnx.raise_on_warnings is True and self._warnings:
220-
raise get_mysql_exception(*self._warnings[0][1:3])
221235

222236
def _handle_resultset(self) -> None:
223237
"""Handle a result set"""
@@ -229,9 +243,6 @@ def _handle_eof(self) -> None:
229243
"""
230244
self._warning_count = self._cnx.warning_count
231245
self._handle_warnings()
232-
if self._cnx.raise_on_warnings is True and self._warnings:
233-
raise get_mysql_exception(*self._warnings[0][1:3])
234-
235246
if not self._cnx.more_results:
236247
self._cnx.free_result()
237248

@@ -1011,8 +1022,6 @@ def _handle_eof(self) -> None:
10111022
"""Handle EOF packet"""
10121023
self._nextrow = (None, None)
10131024
self._handle_warnings()
1014-
if self._cnx.raise_on_warnings is True and self._warnings:
1015-
raise get_mysql_exception(self._warnings[0][1], self._warnings[0][2])
10161025

10171026
def _fetch_row(self, raw: bool = False) -> Optional[RowType]:
10181027
"""Returns the next row in the result set

‎lib/mysql/connector/errors.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ def __str__(self) -> str:
7575
return self._full_msg
7676

7777

78-
class Warning(Exception): # pylint: disable=redefined-builtin
78+
class Warning(Warning, Exception): # pylint: disable=redefined-builtin
7979
"""Exception for important warnings"""
8080

8181

@@ -213,7 +213,10 @@ def custom_error_exception(
213213

214214

215215
def get_mysql_exception(
216-
errno: int, msg: Optional[str] = None, sqlstate: Optional[str] = None
216+
errno: int,
217+
msg: Optional[str] = None,
218+
sqlstate: Optional[str] = None,
219+
warning: Optional[bool] = False,
217220
) -> ErrorTypes:
218221
"""Get the exception matching the MySQL error
219222
@@ -238,6 +241,8 @@ def get_mysql_exception(
238241
pass
239242

240243
if not sqlstate:
244+
if warning:
245+
return Warning(errno, msg)
241246
return DatabaseError(msg=msg, errno=errno)
242247

243248
try:

0 commit comments

Comments
 (0)