fix: include unique in dimensions #558
Conversation
nathanielc
temporarily deployed
to
github-tests-2024
GitHub Actions
Inactive
| +-------+------------+---------------------------------------------------------------+-------------+----------------------------------------------------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+---------------------------------------------------------------+--------------------------------------------+----------------------------------------------------------------------+ | ||
| | index | event_type | stream_cid | stream_type | controller | dimensions | event_cid | data | previous | | ||
| +-------+------------+---------------------------------------------------------------+-------------+----------------------------------------------------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+---------------------------------------------------------------+--------------------------------------------+----------------------------------------------------------------------+ | ||
| | 0 | Data | bagcqcerahx5i27vqxigdq3xulceu5qv6yzdvxzamfueubsyxam5kmjcpp45q | 3 | did:key:z6Mkk3rtfoKDMMG4zyarNGwCQs44GSQ49pcYKQspHJPXSnVw | [model: "ce0102015512204bf5122f344554c53bde2ebb8cd2b7e3d1600ad631c385a5d7cce23c7785459a", controller: "6469643a6b65793a7a364d6b6b337274666f4b444d4d47347a7961724e4777435173343447535134397063594b517370484a5058536e5677", context: ""] | bagcqcerahx5i27vqxigdq3xulceu5qv6yzdvxzamfueubsyxam5kmjcpp45q | 6e756c6c | [] | |
There was a problem hiding this comment.
Can we have index starting with 1 here?
There was a problem hiding this comment.
@samika98 The test does not control the index. It simply inserts the events into the store and then queries them backout. Has your change to start from 1 merged? If so then we likely still have a bug somewhere.
There was a problem hiding this comment.
yes we can do this in a separate PR. We should only return fields strictly greater than the high-water-mark. We need to think about how the highwatermark relates to the index
https://linear.app/3boxlabs/issue/AES-403/index-should-start-from-1 : Created a linear issue for this, we can probably pick this up after start from 1 merges
| .unwrap_or_default(), | ||
| ), | ||
| ( | ||
| "unique".to_string(), |
There was a problem hiding this comment.
nit: Should we have a test that shows that we can query over unique?
There was a problem hiding this comment.
nit: move unique to consts?
nathanielc
force-pushed
the
fix/mid-unique
branch
from
a48fd02 to
ffd80f8
Compare
nathanielc
temporarily deployed
to
github-tests-2024
GitHub Actions
Inactive
|
@stbrody Brought up a good point that the value of the unique dimension is already inside the content (i.e. data ) of the event. So the question is do we need both the dimenstion and the raw data? Th dimension makes is straightforward to query over the unique value, which is the purpose of the dimensions? I don't see much down side to including it and since its valuable to include it I think we should. Thoughts? |
|
my inclination would be to leave it out until/unless someone asks for it. Always easier to add in later than to remove. For now I'd err on the side of keeping the API clean and avoid noise if we don't need it. |
|
but I also don't feel that strongly and could easily be swayed |
|
After some discussion we decided its not immediately useful to be able to query the unique dimension. We will need a system to be able to query any kind of relation and only set account relations are queryable via the unique dimension. As such exposing unique to be a dimension creates a different path for querying one of the types of relations and not the others. Therefore we are not going to expose unique dimensions until we have a clear use case for it and have designed how relations generally will be queried. |
Turns out the unique field can contain useful information to query for example a stream id or a did. This change exposes it in dimensions.
Closes AES-402