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

feat: Use string as default for unknown column type #260

Merged
merged 9 commits into from
Oct 6, 2023

Conversation

LeeMendelowitz
Copy link
Contributor

PR for Issue #259.

Querying a JSON column on MySQL 8.0.26 returns a column of type 245, which is not handled by RMariaDB. This PR converts column of unknown type to string and issues a warning, which is similar to the RMySQL behavior.

@LeeMendelowitz
Copy link
Contributor Author

@krlmlr I'm curious to hear what you think about this proposed solution whenever you have a chance.

Copy link
Member

@krlmlr krlmlr left a comment

Choose a reason for hiding this comment

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

Thanks, looks great!

DESCRIPTION Outdated Show resolved Hide resolved
tests/testthat/test-dbWriteTable.R Outdated Show resolved Hide resolved
@MichaelSchatz
Copy link

@krlmlr This would be a really helpful feature to have integrated into the package for a project I'm currently working on. Is there any chance of getting it merged into the next release candidate?

src/MariaTypes.cpp Outdated Show resolved Hide resolved
@krlmlr
Copy link
Member

krlmlr commented Mar 16, 2023

@LeeMendelowitz: warnings don't seem to be raised for MariaDB. What do you see when you run the following:

DBI::dbGetInfo(RMariaDB::mariadbDefault())$db.version
#> [1] "10.10.2-MariaDB"

Created on 2023-03-16 with reprex v2.0.2

? I'd like to stabilize the tests before merging.

@krlmlr
Copy link
Member

krlmlr commented Mar 23, 2023

I added MySQL 8.0 configurations to main and merged it here, let's see what the tests say.

@LeeMendelowitz: can you please help by reporting what the following code shows on your system:

DBI::dbGetInfo(RMariaDB::mariadbDefault())$db.version
#> [1] "10.10.2-MariaDB"

@LeeMendelowitz
Copy link
Contributor Author

@krlmlr Sorry for the delay. I get "5.5.5-10.11.2-MariaDB-1:10.11.2+maria~ubu2204".

For some reason RMariaDB::mariadbDefault()was not working for me when running off of the LeeMendelowitz:mysql8_json_support branch. I would get:

RMariaDB::mariadbDefault()
Error: Reason: Test database not available

It didn't seem to be using the [rs-dbi] group in my ~/.my.cnf, so I explicitly provided username + password + host + port instead.

@krlmlr krlmlr changed the title Use string as default for unknown column type feat: Use string as default for unknown column type Oct 6, 2023
@krlmlr krlmlr merged commit 647fd2b into r-dbi:main Oct 6, 2023
19 checks passed
@krlmlr
Copy link
Member

krlmlr commented Oct 6, 2023

Thanks!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 6, 2024
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.

3 participants