If you're wondering how something that simple gets a commit
message this long, sit down, because you are in for a ride.
The Row, Rows, TopK, and GroupBy(Rows...) commands had three
different sets of semantics for from/to ranges. We unify these.
Sounds easy, right?
The original purpose of this was to address a bug in GroupBy
where, if you had multiple queries only one of which used time,
we could end up silently returning no results because we tried to
do a time query against a non-time field. This was easy to
fix; just move a boolean flag from outside a loop to inside
the loop so it resets to false on each pass.
In the process of trying to test that, I discovered that
specifying `from=...` without `to=...` in a Rows in a GroupBy
didn't work. Searching around, I discovered that we had three
different answers:
GroupBy, TopK: unspecified 'to=' is 0
Row: unspecified to is tomorrow
Rows: unspecified to is the max time quantum in the field
(A time value of 0 is apparently interpreted as January 1st,
0001.) Note that "GroupBy" is really referring to a Rows()
command in a GroupBy, it's just that this uses completely different
code (because it has to be computing rows potentially matching or
restricted to a filter, or provide the rows it generated so
they can be used to filter something else).
So we fixed that, and made a field method for finding the min/max
values (as done in a Rows command that *isn't* in a GroupBy),
and tried to use that with viewsByTimeRange. Then I tried to write
documentation for this, but the documentation was unclear, and
I tried to clear it up. Which caused me to discover that these
four different places ALSO differed in when or whether they'd
replace a broad query with "just the standard view".
So. Round two of the fix: We create a `field.viewsByTimeRange`,
which tries to fall back to a standard view when one exists
and the specified range covers everything, and treats zero
values as non-restrictive, but also picks a narrow range that
is actually related to the range of dates in the field. This
matters because viewsByTimeRange generates the entire set of
views it would need *even if those views don't exist*.
We drop one test that was testing Rows specifically to verify
that, if you omitted To, we acted as though you'd specified a date
two days in the future. That behavior is not now intended, so
we drop the test that tries to verify it.
Thing that might make this better: Figuring out a way to generate the
list of views more cheaply. Right now, we're redoing all the view
computation, including producing a sorted list of view names, for
every shard. This is excessive, but hard to fix.
In particular, there is no trivial way to generate a sorting such
that you can take slices of it and have them be the right slices,
because we want to skip smaller time quanta when an entire larger
parent quantum is included. e.g., if we're including all of
April 2022, we don't want to include any of the days for April of
2022, but if we're doing up through April 15th, we want to include
the first 15 days of April, but NOT include the whole-month quantum.
And so on. Fixing this cleanly is hard and would require a
significant design effort.
When you backup a cluster, we call /schema which marshals timestamp field options to
json, and restores the fields with those options. If the min and max are missing,
they are set to 0 on restore, which causes an issue on subsequent ingest.
Fixes [SUP-213](https://molecula.atlassian.net/browse/SUP-213) and
[FB-1332](https://molecula.atlassian.net/browse/FB-1332)
During backups on a multi-node cluster, TranslateData was unconditionally redirecting to the primary, regardless of the primary’s status. This is less than ideal. If the primary is down, the backup will fail.
As a consequence of this fix, we will also no longer needlessly redirect to ourselves on a single node cluster. This is a great optimization win!!!!
Fixes # FB-1324 SUP-209
Co-authored-by: tgruben <tgruben@gmail.com>
The ingest API tried to do a Qcx operation that needs a write Tx after
requesting a write Tx. This doesn't work. The ingest API is the only caller
of clearExistenceColumns, so it's easy to just make it work for a given
shard using a provided Tx. This isn't especially clean, but a lot of this
is due for an overhaul anyway because the Qcx/Tx stuff is sort of broken.
Also, add any test case at all for this, since we didn't have one, and
also fix the fact that the test case failed because the test harness
didn't allow empty result sets.
This commit addresses a bug in https://molecula.atlassian.net/browse/SUP-200 where
the Authorization header was not being set correctly when the token was passed via
"userinfo" in the context and not "token".
Now, we prefix the token with "Bearer " when the token comes from userinfo.
This commit also adds a unit test for this function, and simplifies logic. It also
fixes a test that didn't quite test the behavior we wanted.
this stems from https://molecula.atlassian.net/browse/SUP-194 where the string
"standard" was being passed to viewTimePart, which output "standard" as the result.
this is not a valid time string and was causing confusing errors. now it simply
doesn't do that
this commit also adds regression testing framework and a regression test for fb-1287
queries can become arbitrarily long when variables are used.
This is especially the case when a variable is defined as
'select distinct field from table'
and a user wants to use all the values in a Row call
(which is effectively disabling any condition on the field).
This change allows users to select no values for a variable
associated with a Row call to disable the condition. If that
variable is the only condition (query expands to nothing)
then it interprets it as an All call.
Prior to this, we locked the index-wide i.mu around writes to i.fields.
But now the entire open process is holding that lock, so we couldn't
lock it here (that'd deadlock) and didn't think we needed to (because
it was held). But in fact that means that multiple fields being opened
at once can concurrently write to the map. Conveniently, we *already*
pass a shared mutex into openField() to prevent exactly this sort of
problem; we just need to actually use it when doing the write.
