Skip to content

Implement the RowsColumnTypeScanType interface in the go sql driver#12007

Merged
frouioui merged 1 commit intovitessio:mainfrom
slackhq:johan_driver_types
Jan 25, 2023
Merged

Implement the RowsColumnTypeScanType interface in the go sql driver#12007
frouioui merged 1 commit intovitessio:mainfrom
slackhq:johan_driver_types

Conversation

@johanoskarsson
Copy link
Contributor

@johanoskarsson johanoskarsson commented Dec 21, 2022

Description

I attempted to use https://github.com/jimsmart/schema to introspect the vitess schema via the vitessdriver. Much to my surprise all of the column types were returned as interface{}. I dug deeper and found that the Go sql code attempts to cast the row to RowsColumnTypeScanType and if that fails it will not be able to return the types.

RowsColumnTypeScanType contains just one function that I've implemented below. Example of how the mysql drivers implemented it: https://github.com/go-sql-driver/mysql/blob/4591e42e65cf483147a7c7a4f4cfeac81b21c917/fields.go#L140

Overall it's fairly straight forward I have tested it against our vitess locally. However a few of the types are unfamiliar to me, such as HEXNUM so any extra eyes would be appreciated.

Related Issue(s)

Adding the vitess dialect to the schema project here jimsmart/schema#21

Checklist

  • "Backport to:" labels have been added if this change should be back-ported
  • Tests were added or are not required
  • Documentation was added or is not required

Deployment Notes

Should not require any particular deployment notes.

@jimsmart
Copy link

jimsmart commented Jan 5, 2023

Just to add to the conversation here...

When I initially wrote https://github.com/jimsmart/schema some 5 years ago, I realised that the various database drivers out there actually have very different support for what is returned as sql.ColumType metadata.

I tried to catalogue some of that info, in a separate project — originally with the intent of further looking into the differences, and submitting issues / patches, and perhaps even opening an issue on the main Go repo to discuss / track some of the issues I found. Unfortunately, to date, I never got around to any of that.

But here is the project I built to catalogue the driver capabilities WRT sql.ColumnType — the table in the main README is manually composed, but in the subfolders all of the READMEs are automatically generated (and it is from these that the main README is summarised). These individual reports can be accessed via the 'View' link in the Report column in the project's top-level README. I only test common column types.

Note that this project, and its reports, have not been updated in ~5yrs.

https://github.com/jimsmart/drivercaps

The long and short of it, and why I mention it here, is that support for .ScanType() varies greatly, with some drivers returning their own types for certain column types — browsing the individual reports for what other db drivers return for common column types may be helpful to you as a point of reference.

@systay
Copy link
Collaborator

systay commented Jan 10, 2023

@johanoskarsson So, the vitess-driver is not actively worked on. We usually suggest that users use the standard mysql connection to vitess, or alternatively connect using grpc. Have you tried connection using the normal mysql connection protocol?

I'll look over your changes as well, just wanted to share this piece of info first.

@systay systay added Type: Enhancement Logical improvement (somewhere between a bug and feature) Component: Driver labels Jan 11, 2023
Copy link
Member

@frouioui frouioui left a comment

Choose a reason for hiding this comment

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

Everything looks good to me, thank you for your contribution!

@johanoskarsson
Copy link
Contributor Author

I assume the test failure is unrelated to this PR?

@harshit-gangal
Copy link
Member

can you do a merge from the main and push?

@johanoskarsson
Copy link
Contributor Author

Done, thanks!

…or Vitess in order to get the column types

Signed-off-by: Johan Oskarsson <joskarsson@slack-corp.com>
@frouioui frouioui merged commit 5bb59b7 into vitessio:main Jan 25, 2023
@timvaillancourt timvaillancourt deleted the johan_driver_types branch May 21, 2024 20:12
timvaillancourt pushed a commit to slackhq/vitess that referenced this pull request May 21, 2024
…or Vitess in order to get the column types (vitessio#12007)

Signed-off-by: Johan Oskarsson <joskarsson@slack-corp.com>

Signed-off-by: Johan Oskarsson <joskarsson@slack-corp.com>
Co-authored-by: Johan Oskarsson <joskarsson@slack-corp.com>
timvaillancourt added a commit to slackhq/vitess that referenced this pull request May 22, 2024
* Add `VStreamerCount` stat to `vttablet` (vitessio#11978)

* Add `VStreamersActive` stat to `vttablet`

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* Improve desc

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* Add PR suggestions

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* Move to *stats.Gauge

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* Single defer

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* Add `Uptime` metric (vitessio#12712)

* Add `Uptime` metric to `vtgate`+`vttablet`

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* move to go/vt/servenv/status.go

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* Use nanoseconds for uptime

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* Move Uptime metrics to servenv.go, remove dupe start time.Time

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* Use serverStart time.Time

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

---------

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* Implement the RowsColumnTypeScanType interface in the go sql driver for Vitess in order to get the column types (vitessio#12007)

Signed-off-by: Johan Oskarsson <joskarsson@slack-corp.com>

Signed-off-by: Johan Oskarsson <joskarsson@slack-corp.com>
Co-authored-by: Johan Oskarsson <joskarsson@slack-corp.com>

* Add vstream metrics to vtgate (vitessio#13098)

* Add vstream metrics to vtgate

Signed-off-by: twthorn <thomaswilliamthornton@gmail.com>

* Update unit test name and use cell variable

Signed-off-by: twthorn <thomaswilliamthornton@gmail.com>

* Reset metrics for TestVStreamsCreatedAndLagMetrics, fix data race issue

Signed-off-by: twthorn <thomaswilliamthornton@gmail.com>

---------

Signed-off-by: twthorn <thomaswilliamthornton@gmail.com>

* add lock to flaky TestValidateVersionShard test

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* typo

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

---------

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Johan Oskarsson <joskarsson@slack-corp.com>
Signed-off-by: twthorn <thomaswilliamthornton@gmail.com>
Co-authored-by: Johan Oskarsson <johan@oskarsson.nu>
Co-authored-by: Johan Oskarsson <joskarsson@slack-corp.com>
Co-authored-by: Thomas Thornton <thomaswilliamthornton@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Driver Type: Enhancement Logical improvement (somewhere between a bug and feature)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants