* Add (commented out) linters that we should introduce
I went through the available linters and added (commented out) the ones
I think we should work on in the near term. In other words, fix them,
then uncomment them so they are enabled in CI.
* linter: errchkjson
* linter: ineffassign
* linter: gosimple
* linter: errname
FB-1897
The specs for the datetimepart function are, as far as i can tell,
identical to the existing datepart function. Per Pat, replaced the
datepart function with datetimepart rather than just adding
datetimepart as an alias. Made sure existing tests that were using
datepart got switched over.
Second go at this after sorting out weirdness with git.
FB-1896
Added the datetimename function, which returns parts of a timestamp
as strings. Month and day of the week are named ("January", "Monday")
while others are returned as a string of digits ("2023").
Added tests to the test definitions.
* first time quantum queries working
* implement select from timequantum columns
* skip a dax test
* make linter happy
* addressed review feedback
* reverted over eager test elimination
* correct reference for `having count(*)`
It turns out that `having count(*) ...` was always treating
the count(*) as exactly 1. After studying this a lot, I noticed
that in fact, we correctly handle other counts. The reason is
that there's already code to recognize aggregates in `having`
clauses as matching aggregates that are being computed -- but
it only covers the other aggregate clause types, not the newly
added `countStarPlanExpression` from making `count(*)` work even
if there's no `_id` field.
We add several corresponding test cases.
* fix sum(a_decimal) type conversion
Added a test case for this, and also added a fix for it.
Underlying issue: qualifiedRefPlanExpression could end up
producing an int64 instead of a pql.Decimal, even though it
had expected type Decimal.
Originally this worked by politely converting an int64 to
a pql.Decimal in the Evaluate phase, but this was not ideal;
the real question is why it was coming out as an int64 at
that step. Showed this to Pat, who spent a while studying it
and produced a better fix.
* temporarily comment out test which fails in DAX
This forward-ports a number of tests from the previous SQL
implementation. The porting is approximate in a number of ways,
and not all tests are implemented/tested yet.
In particular, several tests are currently disabled because
we don't support `limit n` constructs.
The tests that were primarily tests of the parser have been
brought forward as parser tests. One of them has been altered
to add parentheses, because our parser interprets
fld1 between 1 and 3 and fld2 = 2
as:
fld1 between (1 and 3) and (fld2 = 2)
which is invalid, while the old parser apparently interpreted it
as:
(fld1 between 1 and 3) and (fld2 = 2)
We have not yet verified the SQL spec's requirements here, but
sqlite agrees with our old parser, not our new parser, so this
may be a regression.
The old tests expected an INNER JOIN to suppress duplicate
values. Our new code does not, which is consistent with other
SQL implementations. This is a change, but the old behavior
appears to have been wrong. (You can still suppress duplicate
values by specifying DISTINCT.)
In the previous implementations, a value like `count(*)` had
`count(*)` as its column name. In the new implementation,
it has an empty string as its column name.
Related to this, the prior implementation allowed you to
write
select age, count(*) from grouper group by age having count > 1
but the new implementationt requires that to be spelled as
having count(*) > 1
This is consistent with other SQL implementations, so I think
the new behavior is correct.
The behavior of SHOW COLUMNS and SHOW TABLES has changed, in
that the specific results returned are significantly different.
Perhaps more significantly, the old system spelled the former
query as SHOW FIELDS, rather than SHOW COLUMNS. This may be
considered a regression, in that `SHOW FIELDS` no longer works,
and we should consider whether any hypothetical users might
have been relying on the output of either of these. (I hope
not, the new output is much better.)
Some of the old tests (the ones in handler_test) were accommodated
by adding a couple of specific test cases to existing tests,
specifically:
* handling timestamp values with `Z` rather than `+00:00`
* a join with a WHERE clause referring to fields in both
source tables
We introduce a new "partial" comparison type, because there's
no way for a test of `SHOW TABLES` to contain a correct table
row, because `SHOW TABLES` includes timestamps from when tables
were created. I'm not sure this is the right way to do this.
We add corresponding changes to dax_test, because the DAX tree
tests against the SQL tests.
We change the returned types of field names and field types to
plain strings, ironically because DAX needs this -- the test code
in the DAX tree is getting them back as plain strings, rather
than as dax.FieldName and dax.BaseType.
The tests using `having` are commented out because they don't
seem to be working, a ticket has been filed for this.
Two of the tests that should return strings are instead returning
untranslated integer IDs, but only for DAX, not for the regular
SQL tests, and the `delete` test has been commented out for
DAX-specific errors. If we merge this, the next step is to
ticket those and address them separately.
* Removed support for EPOCH column constraint from TIMESTAMP SQL data type.
* Implicit conversion of integers to timestamp will treat the integer value as seconds since unix epoch.
* Add new ToTimeStamp(num, timeunit) SQL scalar function to help convert integer values to timestamp.
* 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
Note: We now skip a test because we can't pass it but fixing
it is presently beyond my understanding. In parser_test.go,
we skip
SELECT * FROM X INNER JOIN Y ON true INNER JOIN Z ON false
because if we stringify it, we put the "ON true" in the wrong
place, and end up with something we can't parse.
The main change here is modifying AssertParseStatement and
AssertStatementStringer (and the corresponding Expression
functions) to also verify that they can clone and round-trip,
and that we can walk expressions. This gives us a ton
more coverage of Clone and conversions to string, and caught
a number of subtle typos and missing type switch cases.
Some of the changes are mostly cosmetic, such as only
including optional words when stringifying expressions
or statements if those optional words were present originally,
as shown by the Pos value stored for those words.
We also drop a lot of trailing apostrophes from some of the
parse test cases, which appear to be harmless but won't be
reproduced when converting back to strings.
We also add a number of additional test cases, or add
clauses to existing test cases, to improve coverage of a
lot of error testing. For instance, we added a decimal
field to the tests of show table, and added cases
using KEYPARTITIONS. (Although it doesn't *do* anything.)
Similarly, whenever we create a statement, we check
the behavior of requesting a list of sources from it, to
verify that source finding code at least runs.
* 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
* Support Drop Table in serverless (include Snapshotter, Writelogger)
* Finish Database methods
Things like:
- `Databases`
- `DatabaseByID`
- `DatabaseByName`
- `DropDatabase`
* Change Poller to use NodeService instead of its own map
Instead of the Poller maintaining its own map of Addresses to poll, this
commit changes the Poller to use the NodeService interface to get all
known nodes from the Controller.
The next commit needs to:
Next, the logic in the boltdb NodeService implementation was moved to
the boltdb Balancer implementation. That way, the Balancer can be the
source of truth for all things nodes/workers/jobs.
* Move NodeService from Controller to Balancer
This commit moves the implementation of the NodeService into the
Balancer, and aligns `Balancer.AddWorker` with `NodeService.CreateNode`
so that they stay in sync. (Same for `Balancer.RemoveWorker` and
`NodeService.DeleteNode`).
* fix import of private repo
* Fix go vet issues
* Fix bug in DeregisterNode
We need to remove the node from the NodeService even if it's not
assigned to a database. The logic had a bug in it.
This also adds some no-op implementations for SnapshotService and
WriteloggerService. If a directory was not configured for that, then the
computer node would panic on trying to read from the Snapshotter upon
receiving a Directive.
* queryer response content-type: json
* Add support for NULL to WriteloggerDir and SnapshotterDir configs
This commit changes the way WriteLoggerDir and SnapshotterDir are
handled.
If value is empty `""`, an error will be returned on computer startup.
If value is `"NULL"`, a no-op implementation of the service will be
used. This would be for a case that wanted to run serverless on-prem
with no durable storage.
Finally, any other value will be used as the directory to use.
Some things which aren't considered here and may result in unexpected
behavior:
- a value with spaces `" "`
- any "null" which is not "NULL"... like lowercase.
* Finish the DropTable test
* Change "disable service" value to case-insensitive "off"
This commit also removes an unnecessary sleep in the tests.
* Fix docker-compose variables for IDK test
Co-authored-by: Matthew Jaffee <jaffee@pilosa.com>
* implement CREATE/ALTER/DROP VIEW
* fixed failing test
* another failing test
* fixed some broken serverless tests
(cherry picked from commit c620aae350)
* handle sum aggregates with ints; handle escaped quotes in blob literals
* added test ceoverage
* skip subquery test for dax
(cherry picked from commit af475a27f2)
* implemented distinct
* implemented distinct
* uses first cut of a buffer pool, and extendible hashing with thresholded spill to disk
* tests
* cleaned up some stuff around query plan output to make developing tooling easier
* added optimization to call PQL Distinct()
* fixed test
* fix for passing wrong index name in orchestrator
* back out change to DistinctTimestamp
* fix other instance of wrong table name being passed
* use full index name instead of abbreviated one for translation. sigh.
* removed some unused code
Co-authored-by: Matthew Jaffee <jaffee@pilosa.com>
(cherry picked from commit f030d58d95)
* delete implementation with test coverage
* optimize IN expressions; stop linter complaining
* fixed some uncovered query cases
* skip test in DAX for now
(cherry picked from commit 021219935f)
* Fix PQL distinct in dax
When issuing a PQL Distinct() call (or any other call with a "index=" arg),
this commit will attempt to convert the value in the index arg with a
TableKeyer.
* Apply change to call.Children as well
* Add some PQL Distinct (join) test coverage
(cherry picked from commit 4e8fe488de)
