Skip to content
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 warnings count for non-DML/DLL queries like SELECT #2219

Closed
wants to merge 0 commits into from

Conversation

KunZhou-at
Copy link
Contributor

Turns out SELECT queries can also produce warnings, for example:

mysql> SELECT STR_TO_DATE("asdfasdfasdf", "%m-%d-%Y %H:%i:%s");
+--------------------------------------------------+
| STR_TO_DATE("asdfasdfasdf", "%m-%d-%Y %H:%i:%s") |
+--------------------------------------------------+
| NULL                                             |
+--------------------------------------------------+
1 row in set, 1 warning (0.00 sec)

mysql> show warnings;
+---------+------+-------------------------------------------------------------------+
| Level   | Code | Message                                                           |
+---------+------+-------------------------------------------------------------------+
| Warning | 1411 | Incorrect datetime value: 'asdfasdfasdf' for function str_to_date |
+---------+------+-------------------------------------------------------------------+
1 row in set (0.00 sec)

This change adds the warnings count from the second EOF packet for a SELECT query (reference here) because I don't think it's possible for the first EOF packet, the one following FIELDS, to have warnings.

@wellwelwel
Copy link
Collaborator

wellwelwel commented Sep 21, 2023

@KunZhou-at, using this approach in practice with promise, would it be something like [rows, fields, warnings] = await conn.query(...)?

@KunZhou-at
Copy link
Contributor Author

@wellwelwel Yeah, in terms of the API it might be a little confusing that DMLs have their warning counts as the first returned value but it's the last returned value for SELECTs.

@wellwelwel
Copy link
Collaborator

Thanks, @KunZhou-at. In this case, I'm only focusing on typings 🙋🏻‍♂️

You've changed the query callback types:

- | ((err: QueryError | null, result: T, fields: FieldPacket[]) => any)
+ | ((err: QueryError | null, result: T, fields: FieldPacket[], warningCount: number | number[]) => any)

So I wonder if it can also be done in execute.

Also, for promises:

- Promise<[T, FieldPacket[]]>;
+ Promise<[T, FieldPacket[], warningCount: number | number[]]>;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs tests needs typings Typings for new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants