If we can't successfully generate a PQL Percentile call, error
out rather than implementing an actual Percentile function in SQL.
This can be revisited if anyone needs it.
So there's a lot going on here.
Percentile just did not work, even a little, with decimals.
In theory we try to make the int val part of ValCount work, in
ValCountize, but you can't actually use that for everything because
it unconditionally adds bsig.Base even when it shouldn't. But it
doesn't matter that we were returning those values from, say,
(Field).MinForShard, because ValCount.Smaller was not preserving them
when identifying the smaller of two Decimal ValCounts anyway.
And even if it did, the logic in Percentile wouldn't have worked
with passing the raw unscaled integer in as a value to compare
against.
But that's fine because the logic was also more generally wrong.
According to the existing logic, a value is the median value if
exactly as many values are less than it as are greater than it.
This is... not actually very accurate to what we usually mean by
"median". Because some values are *equal* to a given value. So
for instance, say you have the values {1, 1, 1, [a million 2s], 3}.
Our logic would regard 2 as being too high to be the median, because
3 times as many values are lower as are higher.
New interpretation: Imagine a sorted list of all your values, with
N entries. You want the Nth percentile, which is to say, you want N%
of values to be less than the vale you pick, and (100-N)% to be greater.
You can round both of these down. So for instance, if you have 6 values,
and want the median, you want 3 values greater, and 3 values less. To
be picky, we could demand the average of those middle two values, but
we're not in a good position to do that in this implementation.
If the number of desired things less than, or greater than, a target
is 0, we can short-circuit to the minimum or maximum value. This can
happen when nth is close to an end and the number of things is small,
not just at nth=0/nth=100.
So we rework this, and we rework the tests for this behavior to reflect
that logic.
We change executePercentile to be able to return a nil rather than
a weird ValCount in cases where there's no result, such as when
there's no values to compute a percentile of.
We also change the SQL tests to match the new behavior, since some
of them were expecting everything done on a decimal field with values
10-13 to come back as 10.00 as a decimal because that is what the
code returned.
We also propagate these changes to DAX, and along the way, fix up a
TODO item in the DAX copy, and stop skipping the test that was
failing because of that TODO item.
FB-2045
aggregate{Avg,Min,Max}->Update now tested for DataTypeDecimal.
{avg,min,max}PlanExpression->WithChildren now tested.
percentilePlanExpression->{Evaluate,Plan,WithChildren} is not
tested because percentile gets sent directly to PQL rather than
getting planned and evaluated in SQL.
aggregateLast->everything is not tested because Last is not yet
completely implemented.
* make nodeid come from the correct table
* refactored aggregates; added ability to aggregate on expressions not just references
* addressed feedback
* now with the compiler errors fixed after rebase
* fb-1940 re-implemented some changes that got missed private-public
* fb-1939 fixes to between + decimals
* fb-1935 - avg() on and id type + fixed some tests
* fb-1953 add min/max for string types
* fb-1938 - remove internal_type column from show columns
* fb-1964 - fix space_used in fb_cluster_nodes to be int
* fb-1996 - make sure all Idents that are being used as object references to schema objects are lowercased
* fixed failing test
* added some missed changes
* fb-1969 found another case issue with identifier used for column idents
* Fix formatting in CLI results with custom SQLResonse.UnmarshalJSON
When I started this, it was meant to be a quick fix to address the confusing
result formats we were seeing in the CLI. For example, all large integer values
were displayed in scientifc notation. This is because we were passing the result
types from JSON (in this case, float64) into pretty print. Similarly, `IDSets`
and `StringSets` where being printed using the default go Stringer for the types
[]int64 and []string respectively.
I started by writing a customer UnmarshalJSON() method for the `SQLResponse`
type. Part of this (the part which converts data types based on header types)
was already being used in dax tests, so this just formalizes that logic as part
of the `SQLResponse` type.
Then I realized that the sql3 tests (run against the `sql3` package) were
failing because sql3 is not actually returning the `IDSets` and `StringSets`
types. A future task is to formalize return types, define them, and modify sql3
to return them. Once that is done, we can remove the "typed" switch in the
`SQLResponse` json unmarshaller.
Another significant change is the modification to the `ExprDataType` interface:
```
type ExprDataType interface {
exprDataType()
TypeName() string
TypeDescription() string
TypeInfo() map[string]interface{}
}
```
I added two more methods in order to distinguish between a type (`DECIMAL`), its
description (`DECIMAL(2)`), and its type info (`"scale": int64(2)`). Currently,
the description can be used as the field definition in a CREATE TABLE statement,
but we may want to re-think that. Also, Decimal is the only type currently using
TypeInfo.
Finally, I tried to consilidate things around `dax.FieldType` instead of
comparing against parser types outside of sql3. We still have some sql3 parser
and planner types lurking about, but we can address those in future commits.
* Add some test coverage
* smoke test expected INT, now int
* minor fixes
* Introduce WireQueryResponse and related types
This also changes dax.FieldType to dax.BaseType.
* Populate WireQueryResponse correctly
Currently this is in the http handler, and in the queryer.
* Convert sql3 and dax tests to expect pilosa.WireQueryField in results
* fix PQL tests in the SQL defs
* Address a few of the skipped sql tests in dax
(cherry picked from commit f4385df2cf)