search

ekinoguz / sprout-database

Database project of a graduate class.
0 stars 0 forks source link

Write more tests for varchar #17

Closed diedthreetimes closed 11 years ago

diedthreetimes commented 11 years ago

This should include large amounts of data, and all different types of scans (also scan and delete).

diedthreetimes commented 11 years ago

We need a test for scan in which the lowKey Is not present in the index.

Something like

insert 50000 records. with keys ranging from 1-10k and then 20k-60k

Scan for records from 15k - 25k, and verify output is correct.

Our current implementation is probably correct, but I noticed weird behavior when our index is not in the correct state.

cesarghali commented 11 years ago

testCase_O6 in branch ix simulates the previous scenario and It's working from my side.

ekinoguz commented 11 years ago

I also added "high-key" is not present version, both looks fine.

diedthreetimes commented 11 years ago

We still need tests for the original issue. Mostly I think we are lacking tests for exclusive/inclusive keys in scan when using VarChar data.

ekinoguz commented 11 years ago

I added a test case for this: @3bb5a8664fecd29038f25d5d0495f2cd52fa08b6 it looks like working fine

diedthreetimes commented 11 years ago

These all say true, there should be a mix of true and false.

ekinoguz commented 11 years ago

Why should it say mixture of true and false?

diedthreetimes commented 11 years ago

It only tests inclusive (true) we should test exclusive also (false) On May 24, 2013 4:52 PM, "ekin oguz" notifications@github.com wrote:

Why should it say true or false?

— Reply to this email directly or view it on GitHubhttps://github.com/ekinoguz/cs222-database/issues/17#issuecomment-18434337 .

ekinoguz commented 11 years ago

ok I am on it

ekinoguz commented 11 years ago

@ccfa8f0ecda5095a6808d14bbd5c71b369174633 added exclusive one as well. It looks fine however if I increase number of records by 10, it does not pass.

diedthreetimes commented 11 years ago

Commit the failing test and I'll fix it tonight. On May 24, 2013 5:45 PM, "ekin oguz" notifications@github.com wrote:

@ccfa8f0https://github.com/ekinoguz/cs222-database/commit/ccfa8f0ecda5095a6808d14bbd5c71b369174633added exclusive one as well. It looks fine however if I increase number of records by 10, it does not pass.

— Reply to this email directly or view it on GitHubhttps://github.com/ekinoguz/cs222-database/issues/17#issuecomment-18435802 .

ekinoguz commented 11 years ago

I commited. Note that I am adding 400,000 records which might me unnecessarily a lot...