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

build_db removes table (and indexes and types etc) when it should just truncate #109

Closed
tanho63 opened this issue Sep 29, 2020 · 4 comments · Fixed by #111
Closed

build_db removes table (and indexes and types etc) when it should just truncate #109

tanho63 opened this issue Sep 29, 2020 · 4 comments · Fixed by #111

Comments

@tanho63
Copy link
Member

tanho63 commented Sep 29, 2020

The code from here: https://github.com/mrcaseb/nflfastR/blob/master/R/helper_database_functions.R#L119:L123 removes the table (I think so that it's more efficient to send whole tables of data) - it unfortunately also removes any column types and indexes, which was giving me trouble yesterday since it was specifying odd column types.

I believe replacing

dbRemoveTable(db_conn, tblname)

with

DBI::dbExecute(db_conn, glue::glue("DELETE FROM {tblname}"))

would resolve the issue - this only removes rows and not the actual table/indexes/columntypes.

@mrcaseb
Copy link
Member

mrcaseb commented Sep 29, 2020

Thanks for the suggestion @tanho63! I will most likely implement this. However, it can take a few days because I think this is also a solution for another problem I was thinking about.

@mrcaseb mrcaseb linked a pull request Sep 29, 2020 that will close this issue
@mrcaseb
Copy link
Member

mrcaseb commented Sep 29, 2020

@tanho63 I just linked a PR in which I have implemented two things:

  1. No longer remove the tables but only the rows within the tables
  2. Based on 1 it should now be possible to rebuild specified seasons

I'd be very thankful if you could check if this stuff is working in your db as we are only using SQLite dbs (and I hope it is working there, lol).

@tanho63
Copy link
Member Author

tanho63 commented Oct 1, 2020

Will tinker this weekend :)

@mrcaseb
Copy link
Member

mrcaseb commented Oct 1, 2020

No worries. I've merged it in because we want to use the function but I still have in mind we might have to fix some stuff.

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

Successfully merging a pull request may close this issue.

2 participants