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

Allow selection of constraint conflict resolution algorithm in SQLite.load! #302

Merged
merged 4 commits into from
Oct 7, 2022

Conversation

Seitzal
Copy link
Contributor

@Seitzal Seitzal commented Sep 22, 2022

#263 allows the choice between INSERT or REPLACE for the load! function. However, there are several other constraint conflict resolution algorithms listed in the SQLite specification, which could also be supported.
This pull request adds another keyword argument or::Union{String, Nothing} to the load! function, which is set to nothing by default. Valid values include nothing, "ROLLBACK", "ABORT", "FAIL", "IGNORE" and "REPLACE". For backwards compatibility, the replace keyword argument is left intact. If the new argument is set to its default value of nothing, the old keyword arbitrates between INSERT and REPLACE.

@codecov
Copy link

codecov bot commented Sep 22, 2022

Codecov Report

Merging #302 (e10f4ce) into master (ade828a) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #302   +/-   ##
=======================================
  Coverage   94.64%   94.64%           
=======================================
  Files           4        4           
  Lines         560      560           
=======================================
  Hits          530      530           
  Misses         30       30           
Impacted Files Coverage Δ
src/tables.jl 99.34% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@metab0t
Copy link
Collaborator

metab0t commented Sep 22, 2022

  1. Comments should add a link to constraint conflict resolution algorithms
  2. Comments should point out the relationship between conflict arg and existing arg replace.
  3. or is a little obscure. Maybe we can call it on_conflict?
  4. It will be better if you can add more tests to validate the result of several conflict algorithms.

@Seitzal
Copy link
Contributor Author

Seitzal commented Sep 27, 2022

@metab0t
Thanks for the feedback! I just added two commits to address your points.

src/tables.jl Outdated
@@ -214,14 +214,15 @@ function tableinfo(db::DB, name::AbstractString)
end

"""
source |> SQLite.load!(db::SQLite.DB, tablename::String; temp::Bool=false, ifnotexists::Bool=false, replace::Bool=false, analyze::Bool=false)
SQLite.load!(source, db, tablename; temp=false, ifnotexists=false, replace::Bool=false, analyze::Bool=false)
source |> SQLite.load!(db::SQLite.DB, tablename::String; temp::Bool=false, ifnotexists::Bool=false, replace::Bool=false, or::Union{String, Nothing} = nothing, analyze::Bool=false)
Copy link
Member

Choose a reason for hiding this comment

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

What is or here? Should this be on_conflict instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, seems I forgot to update that.

@Seitzal Seitzal requested a review from quinnj October 7, 2022 10:27
Copy link
Member

@quinnj quinnj left a comment

Choose a reason for hiding this comment

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

Thanks for this!

@quinnj quinnj merged commit f48f526 into JuliaDatabases:master Oct 7, 2022
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.

3 participants