fluree / core

Fluree releases and public bug reports
0 stars 0 forks source link

Time Travel Queries Only Return Current Data State #50

Closed aaj3f closed 8 months ago

aaj3f commented 8 months ago

Description

This is observed when running server on feature/where-syntax-update (commit SHA 7f125ccae182d4df5b1850ee64301291cf414954)

Whether using ISO strings or commit integer values, time travel queries with the t keyword currently only return current data state.

The following two queries should throw an error for using an invalid time (e.g. no commit exists on the ledger in 1988 or at commit number 9999999999999)

{
    "from": "cookbook/base",
    "select": {
        "ex:andrew": ["*"]
    },
    "t": "1988-05-30T12:40:44.823Z"
}

...

{
    "from": "cookbook/base",
    "select": {
        "ex:andrew": ["*"]
    },
    "t": 9999999999999
}

Instead, both queries return data state as of latest commit.

Even a query with a valid t value (i.e. within the scope of the ledger's time/commit history) only returns data state as of latest commit

mpoffald commented 8 months ago

This was happening because the new federated queries (which is what this branch ofserver uses for all queries) were not passing along t values yet. I've got a fix in the works, but have questions:

  1. If a federated time-travel query supplies multiple ledgers, but the t is only valid for some of the ledgers, what should happen?

Right now I have a branch where this is an error, and the query only returns data if the t is valid for all the supplied ledgers. This was the most straightforward thing to implement, is this good enough for now?

Alternately I could just PR single-ledger time travel and we can punt on the trickier cross-ledger questions for the time being.

  1. About this query
    {
    "from": "cookbook/base",
    "select": {
        "ex:andrew": ["*"]
    },
    "t": 9999999999999
    }

For datetime ts, we use the "latest t value that is not more recent than that datetime". So if you use a datetime that is in the future relative to the db, you'll get data from the latest t. It seems that this behavior for numeric t values is congruent with that -- if you don't yet have 9999999999999 commits, then you'll get results for your latest t. So I want to double-check that this needs to change.

mpoffald commented 8 months ago

Also want to note that what I'm implementing is the ability to supply a t that is used globally for all ledgers in the query. I found a TODO in our code that I believe refers to supporting individual t's for each data source, but that would be a separate feature rather than part of this bugfix

aaj3f commented 8 months ago

This all makes sense @mpoffald -- the following are opinionated answers to your questions but I'm happy to discuss them more broadly in #fluree-core

If a federated time-travel query supplies multiple ledgers, but the t is only valid for some of the ledgers, what should happen? Right now I have a branch where this is an error, and the query only returns data if the t is valid for all the supplied ledgers. This was the most straightforward thing to implement, is this good enough for now?

I think this is good enough for now, yes

For datetime ts, we use the "latest t value that is not more recent than that datetime". So if you use a datetime that is in the future relative to the db, you'll get data from the latest t. It seems that this behavior for numeric t values is congruent with that -- if you don't yet have 9999999999999 commits, then you'll get results for your latest t. So I want to double-check that this needs to change.

I think this is also fine and good