Skip to content
This repository has been archived by the owner on Jan 28, 2021. It is now read-only.

sql: fix how nulls are sent to clients #783

Merged
merged 1 commit into from
Jul 4, 2019

Conversation

erizocosmico
Copy link
Contributor

Fixes #779

Fixing this, connecting from MySQL client broke. After much debugging, turns out it was because @@version and @@version_comment didn't exist (and thus, they were null), but the client was expecting something there. I just added both, but left them empty and the problem goes away.

@erizocosmico erizocosmico requested a review from a team July 3, 2019 14:48
@@ -175,6 +175,8 @@ func DefaultSessionConfig() map[string]TypedValue {
"ndbinfo_version": TypedValue{Text, ""},
"sql_select_limit": TypedValue{Int32, math.MaxInt32},
"transaction_isolation": TypedValue{Text, "READ UNCOMMITTED"},
"version": TypedValue{Text, ""},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can add the real version here to maintain consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What version should we put? A fake mysql version? It might not be consistent with VERSION() though

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we cannot use the same param as VERSION() is using here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the thing, VERSION() doesn't even exist in plain go-mysql-server. It's registered in gitbase with a specific version.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, right, I mix both. Sorry for the noise.

Signed-off-by: Miguel Molina <miguel@erizocosmi.co>
@ajnavarro ajnavarro merged commit 7f8224b into src-d:master Jul 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NULL is not printed in the resulting table
4 participants