Implement Table.order_by for SQLite and the common scaffolding for all backends
#3502
Conversation
radeusgd
force-pushed
the
wip/radeusgd/database-table-order-by-182195405
branch
3 times, most recently
from
3285734
to
4d6cf9b
Compare
radeusgd
changed the title
Table.order_by methodTable.order_by for SQLite and the common scaffolding for all backends
TODO: actual ordering tests
radeusgd
force-pushed
the
wip/radeusgd/database-table-order-by-182195405
branch
from
8b9c576
to
2497df5
Compare
| Checks if the provided value does not have any attached problems. | ||
| assume_no_problems result = | ||
| result.is_error.should_be_false | ||
| warnings = Warning.get_all result . map .value |
There was a problem hiding this comment.
| warnings = Warning.get_all result . map .value | |
| Warning.get_all result . length . should_equal 0 |
There was a problem hiding this comment.
I deliberately did it this way.
If we did as you suggest, if the test fails it will display 1 did not equal 0 or something like that, not too informative.
But with the original approach I did, we'd get [Some_Warning] did not equal [] - so you immediately see what is the unexpected warning that got reported and IMO this helps with debugging the tests.
| - If two name matchers match the same column, a | ||
| `Column_Matched_By_Multiple_Selectors`. | ||
| - If no valid columns are selected, a `No_Input_Columns_Selected`. | ||
| - If values do not implement an ordering, an |
There was a problem hiding this comment.
don't think this can occur here - only true for in-memory I think
There was a problem hiding this comment.
True, I kept this to have the documentation consistent between variants. I thought we want the docstrings to be the same, but I'm not sure, happy to change it if you think slight differences are ok.
My line of thinking is just that it may be confusing to the users because we try to make the differences between inmem and db rather opaque and so the user may read the doc for one and thing it applies in both variants.
OTOH there are some differences and so they may be worth highlighting.
Maybe some compromise would be to keep the docstring the same but highlight differences? For example I can add (only applicable in the In_Memory backend which can hold custom types) here. What do you think?
There was a problem hiding this comment.
no happy to leave as is for that reason - worse case they add error handling for an impossible scenario but basically harmless
|
|
||
| In case the selectors have differing metadata and the error does not prevent | ||
| the operation from continuing, the first selector on the list is used. | ||
| type Column_Matched_By_Multiple_Selectors (column_name : Text) (selectors : [Any]) |
There was a problem hiding this comment.
We should be consistent on [Any] versus Vector Any
There was a problem hiding this comment.
As discussed, we'll make a separate task to align these, as this applies to the whole codebase.
Pull Request Description
Implements the common and SQLite parts of https://www.pivotaltracker.com/story/show/182195405
Important Notes
Checklist
Please include the following checklist in your PR:
Scala,
Java,
and
Rust
style guides.
./run ide distand./run ide watch.