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

New DataStreams framework implementation for SQLite.jl #79

Closed
wants to merge 4 commits into from

Conversation

quinnj
Copy link
Member

@quinnj quinnj commented Oct 20, 2015

@Sean1708, it's finally ready :) (hopefully)

I still need to update the README, but I've put in the new 0.4 docs inline with the code. There's almost no change in the UDF functionality/usage. The core types/methods have been refactored slightly (SQLiteDB => SQLite.DB), plus slightly more idiomatic usage of the ! operator.

The biggest changes are in the removal of the create/append methods (which I can't remember if they're even on master or were just on jq/updates). These have been replace by the more robust and broadly applicable DataStreams framework by having the SQLite.Source and SQLite.Sink types and the appropriate Data.stream! methods defined between them and Data.Table, CSV.Source, and CSV.Sink.

I believe the deprecations have all been setup correctly, but I'd appreciate any poking around here. Basically, all the deprecated types/methods are in old_ui.jl and the old tests still exist at test/old_runtests.jl.

@@ -70,54 +65,75 @@ function SQLiteDB(file::AbstractString="";UTF16::Bool=false)
else # error
sqlite3_close(handle[1])
sqliteerror()
>>>>>>> d86fcf628105a178ae6fc0128740e4ec2df8b2e7
Copy link
Contributor

Choose a reason for hiding this comment

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

Merge conflict, I think you meant to remove this function?

Copy link
Member Author

Choose a reason for hiding this comment

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

drat. i think things got borked somehow. I'll do a clean force push

@Sean1708
Copy link
Contributor

I've given it a quick once over but I'm not gonna have time to play with it properly until the weekend. Looking good though!

@quinnj
Copy link
Member Author

quinnj commented Oct 20, 2015

Thanks for the review so far. I cleaned up the git history quite a bit and just decided to open a new PR at #80. I'll keep this up through this weekend. I'll probably plan on merging and tagging sometime this weekend (along with a nice blog post mentioned some of the higher-level changes).

sqlitetype{T<:Integer}(::Type{T}) = SQLITE_INTEGER
sqlitetype{T<:AbstractFloat}(::Type{T}) = SQLITE_FLOAT
sqlitetype{T<:AbstractString}(::Type{T}) = SQLITE_TEXT
sqlitetype(x) = SQLITE_BLOB
Copy link
Contributor

Choose a reason for hiding this comment

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

Does there also need to be a method for NullType here?

@quinnj quinnj closed this Oct 25, 2015
@quinnj quinnj deleted the jq/datastreams branch October 25, 2015 03:09
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 this pull request may close these issues.

2 participants