Simple aggregates again #584
Conversation
rnewman
force-pushed
the
rnewman/simple-aggregates-3
branch
from
56be003
to
aff2758
Compare
|
This is ready for you to look at, @fluffyemily. |
rnewman
force-pushed
the
rnewman/simple-aggregates-3
branch
from
aff2758
to
15a42e6
Compare
This lets us make reference to columns by name, rather than only pointing to qualified aliases.
rnewman
force-pushed
the
rnewman/simple-aggregates-3
branch
from
15a42e6
to
c6a5464
Compare
| @@ -173,6 +173,13 @@ impl Store { | |||
| sqlite: connection, | |||
| }) | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Really kinda surprised we didn't already have this.
query-algebrizer/src/clauses/mod.rs
Outdated
| self.value_bindings.contains_key(var) | ||
| } | ||
|
|
||
| pub fn value_bindings(&self, variables: &BTreeSet<Variable>) -> VariableBindings { | ||
| self.value_bindings.with_intersected_keys(variables) | ||
| } | ||
|
|
||
| /// Return an interator over the variables externally bound to values. |
query-algebrizer/src/types.rs
Outdated
| @@ -283,6 +283,9 @@ pub enum Inequality { | |||
| GreaterThan, | |||
| GreaterThanOrEquals, | |||
| NotEquals, | |||
|
|
|||
| // Ref operators. | |||
| Differ, | |||
There was a problem hiding this comment.
can we have a differ test?
tests/query.rs
Outdated
|
|
||
| ;; Alice danced with Beli. | ||
| [:db/add "a" :foo/dance "ab"] | ||
| [:db/add "b" :foo/dance "ab"] |
There was a problem hiding this comment.
what would happen if I did [:db/add "b" :foo/dance "ba"] here? Does this rely upon the 'dance' ordering being identical for unpermute to work?
There was a problem hiding this comment.
That would add a second dance for Beli, one in which she doesn't dance with anyone else. The only set of bindings for ?left and ?right would be her, and naturally she doesn't unpermute with herself, so that solo dance wouldn't contribute anything to the output.
If you were to add both Alice and Beli to that second dance, the output would be unchanged: the inner query would generate [Alice, Beli] [Alice, Alice] [Beli, Alice] [Beli, Beli] twice, once for each of the two dances; the unpermute step would remove [x, x] pairs and one half of the orders; and then the internal DISTINCT would remove duplicates.
I've added this to the test.
| @@ -134,14 +134,21 @@ impl ConjoiningClauses { | |||
| // We expect the intersection to be Long, Long+Double, Double, or Instant. | |||
| let left_v; | |||
| let right_v; | |||
| if shared_types.is_empty() { | |||
There was a problem hiding this comment.
We already check shared_types.is_empty() in line 115. The result of that check, if the set is empty, is to mark_known_empty and return Ok(()). As we do nothing else between that check and this, this code will never succeed. Do we really need it?
There was a problem hiding this comment.
Thanks! Probably a screwup when rebasing a very old branch.
This allows for a corresponding value to be returned when a query includes one 'min' or 'max' aggregate.
Obsoletes #498. Rebased on top of current master.
Still to do:
(count ?x)where?xis bound ➡️1.tests/query.rs.:withcontrols the projection in the inner query. The simplest case is where the outer projection list is the same as the inner, and neither features aggregation — that's what we already do! — and aggregation means slightly untangling that.