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

Connector/NET: A warning causes stack overflow #8440

Closed
tkovacs-dev opened this issue Oct 11, 2024 · 5 comments
Closed

Connector/NET: A warning causes stack overflow #8440

tkovacs-dev opened this issue Oct 11, 2024 · 5 comments
Assignees
Labels
bug Something isn't working customer issue

Comments

@tkovacs-dev
Copy link

If the statement you execute issues a warning (like division by zero, or running DROP DATABASE IF EXISTS nonexistent_db) a stack overflow error occurs in the Connector/NET code.

How to reproduce:

  • start dolt (1.43.3) with dolt sql-server
  • run this code with .net 8.0, MySql.Data version 9.0.0 (or 8.0.33):
using MySql.Data.MySqlClient;

public static class DoltTest
{
    public static void Main()
    {
        using var conn = new MySqlConnection("server=127.0.0.1;port=3306;sslmode=none;user=root;password=;CharSet=utf8;database=dolt-db");
        conn.Open();
        MySqlHelper.ExecuteScalar(conn, "select 1/0");
    }
}
@timsehn
Copy link
Contributor

timsehn commented Oct 11, 2024

Thanks for the issue. @jycor or @fulghum (our .net expert) will look at this today.

@timsehn timsehn added the bug Something isn't working label Oct 11, 2024
@fulghum
Copy link
Contributor

fulghum commented Oct 11, 2024

Hi @tkovacs-dev, thanks for reporting this issue to us. I've been able to repro this behavior and look at the wire logs. This appears to happen because Dolt is not clearing the session warnings after a call to SHOW WARNINGS. From the wire logs, I can see that the .NET app is calling SHOW WARNINGS many hundreds of times, until it overflows its stack.

From a SQL shell connected to Dolt, you can see that when we call SHOW WARNINGS after your example query, a warning is still being reported, so the .NET framework keeps calling SHOW WARNINGS over and over until it pops its stack.

mysql> select 1/0;
+-----+
| 1/0 |
+-----+
| NULL |
+-----+
1 row in set, 1 warning (0.00 sec)

mysql> show warnings;
+---------+------+---------------+
| Level   | Code | Message       |
+---------+------+---------------+
| Warning | 1365 | Division by 0 |
+---------+------+---------------+
1 row in set, 1 warning (0.01 sec)

mysql> show warnings;
+---------+------+---------------+
| Level   | Code | Message       |
+---------+------+---------------+
| Warning | 1365 | Division by 0 |
+---------+------+---------------+
1 row in set, 1 warning (0.00 sec)

mysql> show warnings;
+---------+------+---------------+
| Level   | Code | Message       |
+---------+------+---------------+
| Warning | 1365 | Division by 0 |
+---------+------+---------------+
1 row in set, 1 warning (0.01 sec)

The issue seems to be that the output of SHOW WARNINGS on Dolt is reporting that there is 1 new warning (i.e. "1 row in set, 1 warning"), while on MySQL, SHOW WARNING continues to display the current warning, but does not send back metadata saying there is a new warning.

@fulghum
Copy link
Contributor

fulghum commented Oct 11, 2024

We dug into this some more and figured out what's going on. Our logic that manages clearing sessions warnings runs at the end of analysis, but these types of warnings only happen during the execution/evaluation phase, which is after analysis. That causes the warning status to be slightly out of sync and to not get cleared correctly until two statements have been executed successfully – they should be cleared on the first statement that is executed successfully.

@jycor is going to take it from here to look at refactoring that logic in the analyzer.

@jycor
Copy link
Contributor

jycor commented Oct 11, 2024

Hey @tkovacs-dev, thanks for reporting this issue.
The fix for this has been merged to dolt main, and a release will be coming out shortly.

Let us know if it works for you!

@tkovacs-dev
Copy link
Author

@jycor Works great, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working customer issue
Projects
None yet
Development

No branches or pull requests

5 participants