Actually use indexes #73
Conversation
|
This could be rewritten a few different ways, but mostly wanted to get eyes on it and know if there should be some other abstraction around the indexes, Tyler. |
| // We have to index_seek in both the committed state and the current tx state. | ||
| // First, we will check modifications in the current tx. It may be that the table | ||
| // has not been modified yet in the current tx, in which case we will only search | ||
| // the committed state. Finally, the table may not be indexed at all, in which case | ||
| // we fall back to iterating the entire table. | ||
|
|
||
| // We need to check the tx_state first. In particular, it may be that the index | ||
| // was only added in the current transaction. | ||
| // TODO(george): It's unclear that we truly support dynamically creating an index | ||
| // yet. In particular, I don't know if creating an index in a transaction and | ||
| // rolling it back will leave the index in place. |
| // Either the current transaction has not modified this table, or the table is not | ||
| // indexed. |
There was a problem hiding this comment.
| // Either the current transaction has not modified this table, or the table is not | |
| // indexed. | |
| // Either the current transaction has modified this table, or the table is indexed. |
Not sure if this is better.
| pub enum IterByColEq<'a> { | ||
| Scan(ScanIterByColEq<'a>), | ||
| Index(IndexIterByColEq<'a>), | ||
| CommittedIndex(CommittedIndexIterByColEq<'a>), | ||
| } |
There was a problem hiding this comment.
Docs on these would probably aid quicker understanding.
There was a problem hiding this comment.
Actually good question for you:
In this common pattern of
enum Foo {
X(X),
Y(Y),
}
struct X {}
struct Y {}
would you prefer the more substantial doc comments on the structs X and Y, or on the enum arms X and Y?
There was a problem hiding this comment.
Hmm that's a tough one. I think it depends on how often one has the Foo version vs. the X/Y versions. I think I tend to prefer more of the docs on the enum variants as they tend to be used like so Foo::X(X { the fields }) and it provides more context in relation to the other variants. That said, you can always back-link and such, so it's not a big deal either way.
| !self | ||
| .tx_state | ||
| .delete_tables | ||
| .get(&self.table_id) | ||
| .map_or(false, |table| table.contains(row_id)) |
There was a problem hiding this comment.
This here two I think could help readability if it was made into a function with docs explaining the why.
jdetter
force-pushed
the
george/fix-indexes
branch
from
2445b7b
to
f2cdd7f
Compare
kulakowski
force-pushed
the
george/fix-indexes
branch
from
f2cdd7f
to
6e9bf4b
Compare
kulakowski
force-pushed
the
george/fix-indexes
branch
from
6e9bf4b
to
f039536
Compare
* Actually use indexes * Get rid of unneeded references
* Actually use indexes * Get rid of unneeded references
Description of Changes
API
If the API is breaking, please state below what will break