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.

Sign up for GitHub

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

Updated getSymbols.MySQL SELECT statement to include dbname variable … #88

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Updated getSymbols.MySQL SELECT statement to include dbname variable … #88

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions R/getSymbols.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,7 @@ function(Symbols,env,return.class='xts',
db.fields=c('date','o','h','l','c','v','a'),
field.names = NULL,
user=NULL,password=NULL,dbname=NULL,host='localhost',port=3306,
...) {
from=NULL, to=NULL, ...) {
Copy link
Owner

Choose a reason for hiding this comment

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

These new arguments need to be documented in the corresponding .Rd file. You can use man/getSymbols.yahoo.Rd as a template. Though I'm not sure the defaults for getSymbols.yahoo make sense here.

importDefaults("getSymbols.MySQL")
this.env <- environment()
for(var in names(list(...))) {
Expand Down Expand Up @@ -631,7 +631,13 @@ function(Symbols,env,return.class='xts',
if(verbose) {
cat(paste('Loading ',Symbols[[i]],paste(rep('.',10-nchar(Symbols[[i]])),collapse=''),sep=''))
}
query <- paste("SELECT ",paste(db.fields,collapse=',')," FROM ",Symbols[[i]]," ORDER BY date")

Copy link
Owner

@joshuaulrich joshuaulrich Jul 19, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Adding the backticks would have been cleaner as:

query <- paste("SELECT ",paste(db.fields,collapse=',')," FROM `",Symbols[[i]],"` ORDER BY date",sep="")

It would also be good to separate this change from the change that adds the from and to arguments by putting them in different commits.

if (!is.null(from) & !is.null(to)) {
Copy link
Owner

Choose a reason for hiding this comment

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

This implies that the user must specify both from and to (i.e. only specifying one will cause all data to be returned). It would be better if they could specify none, one, or both.

query <- paste("SELECT ", paste(db.fields, collapse = ","), " FROM ", paste("`", Symbols[[i]], "`", sep = ""), " WHERE date >= '", from, "' AND date <= '", to, "' ORDER BY date")
} else {
query <- paste("SELECT ", paste(db.fields, collapse = ","), " FROM ", paste("`", Symbols[[i]], "`", sep = ""), " ORDER BY date")
}

rs <- DBI::dbSendQuery(con, query)
fr <- DBI::fetch(rs, n=-1)
#fr <- data.frame(fr[,-1],row.names=fr[,1])
Expand Down