Skip to content
This repository was archived by the owner on Sep 12, 2018. It is now read-only.

Support transacting :db/fulltext true attributes. Fixes #189.#375

Merged
ncalexan merged 1 commit into
mozilla:rustfrom
ncalexan:fts
Mar 21, 2017
Merged

Support transacting :db/fulltext true attributes. Fixes #189.#375
ncalexan merged 1 commit into
mozilla:rustfrom
ncalexan:fts

Conversation

@ncalexan
Copy link
Copy Markdown
Member

This builds on #370, so be sure to look at that first.

@ncalexan
Copy link
Copy Markdown
Member Author

This will address #189. @rnewman, this is a bit slap-dash but good enough for now.

Copy link
Copy Markdown
Collaborator

@rnewman rnewman left a comment

Choose a reason for hiding this comment

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

Pleasantly formulaic.

Comment thread db/src/db.rs

/// Insert search rows into temporary search tables.
///
/// Eventually, the details of this approach will be captured in
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

File issue?

Comment thread db/src/db.rs

let chunks: itertools::IntoChunks<_> = entities.into_iter().chunks(max_vars / bindings_per_statement);

// We'd like to flat_map here, but it's not obvious how to flat_map across Result.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think collect has automatic conversions for Vec<Result<T>> to Result<Vec<T>>, if that helps.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't think it does. We want to flat_map a bunch of Result<Iter> into Result<Iter>. I guess it's a fold with .chain(), but that's no more clear than this pattern.

Comment thread db/src/db.rs
// We must keep these computed values somewhere to reference them later, so we can't
// combine this map and the subsequent flat_map.
// (e0, a0, v0, value_type_tag0, added0, flags0)
let block: Result<Vec<(i64 /* e */,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Entid? Or are you sticking to raw types to emphasize SQLness?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I guess I am.

Comment thread db/src/db.rs Outdated
u8 /* flags0 */,
i64 /* searchid */)>> = chunk.map(|&(e, a, ref attribute, ref typed_value, added)| {
if typed_value.value_type() != ValueType::String {
bail!("TODO");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

unimplemented!()?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh! This was just overlooked; I'll add an error message.

Comment thread db/src/debug.rs
use tabwriter::TabWriter;

use bootstrap;
use db::TypedSQLValue;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think you've interleaved modules and crates here…

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No, these are all modules. Or do you want all use module; before any use module::symbol? I've been interleaving them.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I mean: you have use mentat_core::… below here, followed by use types::Schema. Rather than:

extern crate …;

use third_party_library::…;

use mentat_foo::…;
use mentat_bar::…;

use samecratemodule_a::…;
use samecratemodule_b::…;

you've interleaved them.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I haven't been separating out mentat_* from samecratemodule_*. I can start doing that.

These tests are direct translations of the Clojure tests.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants