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

MySqlDataReader.GetSchemaTable() behavior change in 0.57.0 #722

Closed
MaceWindu opened this issue Oct 20, 2019 · 7 comments
Closed

MySqlDataReader.GetSchemaTable() behavior change in 0.57.0 #722

MaceWindu opened this issue Oct 20, 2019 · 7 comments
Assignees
Labels

Comments

@MaceWindu
Copy link
Contributor

MaceWindu commented Oct 20, 2019

in linq2db we use following code to read procedure results schema to be able to generate mappings from existing database:

cmd.CommandText = "<procedure_name>";
cmd.CommandType = CommandType.StoredProcedure;
//cmd.Parameters.Add(...);
var table = cmd.ExecuteReader(CommandBehavior.SchemaOnly).GetSchemaTable();
// reverse resultset schema from table

For MySqlConnector it always returned null for procedures, that doesn't return any resultsets but have output parameters.
Starting from version 0.57 for such procedures it started to return schema table with:

  • row with name "\ue001\b\v" (probably came from MySql itself?)
  • rows for each output parameter

Now is question - is it expected breaking change or bug? From API point of view I would say it's a bug, because there is no resultsets in SQL terms. BUT. MySql.Data was doing same thing for a long time and we even have detection logic for it:
https://github.com/linq2db/linq2db/blob/master/Source/LinqToDB/DataProvider/MySql/MySqlSchemaProvider.cs#L279-L283

Anyway I will update this condition in our code to check if schema table have columns with "\ue001\b\v" name.

@bgrainger
Copy link
Member

bgrainger commented Oct 20, 2019

This is almost certainly related to 8220c3e (see also 77ea90b).

This was a deliberate change in 0.57.0 (to retrieve the stored procedure results and its output parameters in one statement), but was not intended that the resultset for the output parameters would be visible to user code. (It's an implementation detail of MySQL that the output parameters have to be SELECTed like any regular result set. Connector/NET must have taken a similar approach.)

This should be fixed, but if it is, you might run into #678. (I'm open to reverting that change.)

#723 might be a solution here; even if not, it would be worth doing for efficiency.

Do you have sample Linq2Db code that reproduces this problem?

@bgrainger bgrainger added the bug label Oct 20, 2019
@MaceWindu
Copy link
Contributor Author

MaceWindu commented Oct 20, 2019

I cannot say for anyone else who can use GetSchemaTable for procedure calls, but for linq2db specifically - we don't mind this new behavior, we just need to be sure that we can detect it. And judging from code you linked, marker column name is fixed. Still I think it makes sense to document it in xml-docs for this method probably if this logic remains.

Also we are fine if it is fixed and start throwing exception - we already catching them, because this is what some other providers do.

Not sure what type of code you need, in our case following code (test) started to fail (this line specifically) because we stated to get schema for this stored procedure. For test you can just use code from initial message with provided procedure - this is essentially what linq2db does.

Actually I don't think it impacted any of linq2db users yet, as we use 0.56.0 still in our model generator, so only people who use linqpad plugin or call our schema api directly could be affected.

@bgrainger
Copy link
Member

bgrainger commented Oct 20, 2019

This is not intentional that GetSchemaTable returns the schema of the output parameters. If you need to put in a workaround for users who have MySqlConnector 0.57.0-0.60.0 installed, I understand, but I'd also like to fix it in the next release.

I'm able to reproduce the problem by using that example test code.

@bgrainger
Copy link
Member

bgrainger commented Oct 20, 2019

This is symptomatic of a larger issue: GetFieldType, GetName, etc. will also return values for the pseudo-result-set that's created for the output parameters.

I've filed a bug on Connector/NET that gives examples of this incorrect behaviour: https://bugs.mysql.com/bug.php?id=97300

(@MaceWindu you could subscribe to that bug if you want to be alerted if Oracle ever fixes the behaviour you're working around.)

All these methods need to be addressed in MySqlConnector, not just GetSchemaTable.

@bgrainger bgrainger self-assigned this Oct 20, 2019
@bgrainger
Copy link
Member

bgrainger commented Oct 20, 2019

Fixed in 0.60.1.

@MaceWindu
Copy link
Contributor Author

MaceWindu commented Oct 21, 2019

if Oracle ever fixes

From looking at their history of fixing issues (or even accepting fixes) in provider I suspect that ever will be never 😢

@kostebudinoski
Copy link

kostebudinoski commented Oct 21, 2019

Fixed in 0.60.1.

There are cases when we check the data reader FieldCount property, in order to know if we should read the data reader. With the latest update, all these checks on the data reader are throwing exceptions, when the reader is empty and only output parameters are returned.

This breaks some of our property checks on the data reader, where until now no exception was expected.

Also, executing a query that, by its nature, does not return rows (such as a DELETE query), should set FieldCount to 0. What about the HasRows property?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants