database: Replace with new version.

[davecgh]

Requires #380, #491, and #650.

This pull request removes the old database package, moves the new package into its place, and updates all imports accordingly.

[davecgh]

Rebased.

[davecgh]

Rebased.

[davecgh]

Rebased to latest master (on top of blockchain_database2 branch).

---

Comments from the review on Reviewable.io

[davecgh]

Rebased to latest master (on top of blockchain_database2 branch).

---

Comments from the review on Reviewable.io

[davecgh]

Rebased to latest master (on top of blockchain_database2 branch).

[davecgh]

Rebased to latest master (on top of blockchain_database2 branch).

[jongillham]

Reviewed 9 of 97 files at r1, 5 of 134 files at r2, 6 of 12 files at r3, 3 of 7 files at r4, 119 of 125 files at r5.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

database/interface.go, line 22 [r5] (raw file):
Most of the Cursor methods don't return errors. What would be the best way to expose errors in the underlying storage implementation without these methods returning errors?

---

database/interface.go, line 177 [r5] (raw file):
Would it make sense for this method to also return an error such as Get(key []byte) ([]byte, error) to signify errors in the underlying storage?

---

Comments from the review on Reviewable.io

[jongillham]

Review status: 5 of 110 files reviewed at latest revision, 3 unresolved discussions.

---

database/interface.go, line 104 [r7] (raw file):
It seems implicit that DeleteBucket should also recursively delete all nested buckets otherwise you would have nested buckets without any ability to access them. Maybe this should be explicitly stated?

---

Comments from the review on Reviewable.io

[davecgh]

Rebased to the latest master.

---

Reviewed 1 of 134 files at r2, 38 of 125 files at r5, 8 of 143 files at r6, 106 of 125 files at r7, 29 of 29 files at r8.
Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks broke.

---

database/interface.go, line 22 [r5] (raw file):
Yeah, after doing a bit more research, I agree that you're correct. I've mainly dealt with cursors on memory-mapped databases and hence the data is effectively in memory. I was modelling it after that given that ultimately I would like to see a memory-mapped database backend. I'm sure you've noticed several of the comments make reference to that.

However, since the interface is intended to be generic enough to support any database backend, the assumptions do not hold.

That said, I don't want to make the change in this pull request because it is only intended to delete the old database and move the new database2 package (which is already in master) in its place.

I think opening up a separate issue and creating a separate PR after the chain PR #491 and this one are in to expose errors is a fine idea.

---

database/interface.go, line 177 [r5] (raw file):
It is actually possible to test for existence because the interface guarantees that an empty byte slice will be returned for keys that exist as opposed to nil for those that don't. As a result you can do the following:

value := bucket.Get(key)
if value == nil {
    fmt.Println("key does not exist")
} else if len(value) == 0 {
    fmt.Println("key exists but has no value")
}

I should also note that another nice property is taking the len of nil or an empty slice both result in 0, so in many cases where you don't care if it exists and simply that it's not empty, the following works:

value := bucket.Get(key)
if len(value) == 0 {
    fmt.Println("key either does not exist or exists but has no value")
}

---

database/interface.go, line 104 [r7] (raw file):
Done.

---

Comments from the review on Reviewable.io

[jongillham]

Review status: 108 of 110 files reviewed at latest revision, 1 unresolved discussion.

---

database/interface.go, line 177 [r5] (raw file):
My specific issue was if there's an underlying database error when bucket.Get is called. value might return nil but you wouldn't be able to distinguish whether that was because the key really doesn't exist or just temporarily couldn't be fetched from the database due to an underlying error.

On a separate but slightly related note, I have attempted to implement database.Interface to a couple of NoSQL databases and had limited success with the Bucket and Cursor interface mainly due to the nesting functionality. However, I imagine that these interface issues could be tackled in subsequent pull requests once your awesome work on database, blockchain and the ffldb implementation have been merged into master. (I feel that there should be some orthogonality between the Tx, blockchain storage functions and Bucket/Cursor interfaces as well as the complete removal of non-managed transaction options. Although I haven't reviewed all your code to understand why the design decisions are they way they are and I know I am late in the game to comment on such drastic changes.)

---

Comments from the review on Reviewable.io

[davecgh]

Reviewed 1 of 125 files at r5, 1 of 125 files at r7, 1 of 29 files at r8, 2 of 2 files at r9.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

database/interface.go, line 177 [r5] (raw file):
Agreed. The interface will likely need some tweaks. I had it up on github as a PR for something like 6 months and didn't ever really got a lot of feedback on the interface itself, so I'm glad to read you've been taking a serious look at it to identify these types of issues. The speed improvements with all of the new PRs you mentioned versus the current code in master are huge, so I'm definitely focused on getting it finished up and merged to master before focusing on interface changes at this point. However, after all of that code makes it in to master, we'll definitely want to address any interface issues so other backends can become a reality

As far as the buckets, you'll probably have to implement them similar to how I did it in ffldb. In particular it keeps a unique bucket counter that gives every new bucket (nested or not) a unique value. Then, it maintains a bucket index that consists of keys which are a prefix followed by the bucket's parent id followed by its name and the value is the bucket's unique id. With that, all keys are stored prefixed with the unique bucket id. In that way, you can easily create a cursor over all items in a bucket by doing a prefix scan for the unique bucket id. As far as checking for nested buckets, that too is easily done by a prefix scan for the bucket index prefix + parent bucket id.

---

Comments from the review on Reviewable.io

[davecgh]

Rebased to latest master (on top of blockchain_database2_index branch).

---

Reviewed 3 of 136 files at r1, 1 of 134 files at r2, 2 of 12 files at r3, 38 of 125 files at r5, 38 of 125 files at r7, 7 of 137 files at r10, 130 of 130 files at r11.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from the review on Reviewable.io

[davecgh]

Reviewed 1 of 134 files at r2, 38 of 125 files at r5, 38 of 125 files at r7, 38 of 130 files at r11, 130 of 130 files at r13.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from the review on Reviewable.io

[davecgh]

Rebased to the latest master (on top of blockchain_database2_index branch).

---

Reviewed 1 of 134 files at r2, 38 of 125 files at r5, 38 of 125 files at r7, 38 of 130 files at r11, 38 of 130 files at r13, 2 of 132 files at r14, 130 of 130 files at r15.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[davecgh]

Rebased to the latest master (on top of blockchain_database2_index branch).

---

Reviewed 1 of 134 files at r2, 38 of 125 files at r5, 38 of 125 files at r7, 38 of 130 files at r11, 38 of 130 files at r13, 38 of 130 files at r15, 1 of 131 files at r16, 130 of 130 files at r17.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[davecgh]

Rebased to the latest master (on top of blockchain_database2_index branch).

---

Reviewed 1 of 134 files at r2, 38 of 125 files at r5, 38 of 125 files at r7, 38 of 130 files at r11, 38 of 130 files at r13, 38 of 130 files at r15, 38 of 130 files at r17, 1 of 131 files at r18, 130 of 130 files at r19.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[dajohi]

OK

[jrick]

ok