Displayer for QueryRowsResult #1138
Conversation
wprzytula
changed the title
Michu1596
force-pushed
the
query_rows_result_displayer
branch
from
c8ad86d to
ddf3d5d
Compare
|
note the difference between the way |
Michu1596
force-pushed
the
query_rows_result_displayer
branch
from
3a3f682 to
e15634e
Compare
Michu1596
force-pushed
the
query_rows_result_displayer
branch
from
e15634e to
e822850
Compare
|
|
Michu1596
force-pushed
the
query_rows_result_displayer
branch
4 times, most recently
from
54ad40d to
7b10b8b
Compare
There was a problem hiding this comment.
💯 This is a really great job done! In the aspect of usability, I'm very satisfied.
🔧 There are a few items to be addressed, though, mainly about:
- efficiency: I believe we can get rid of String allocations on the way to display,
- more idiomatic code,
- division into modules.
scylla/src/response/query_result.rs
Outdated
| } | ||
| } | ||
|
|
||
| /// A utility struct for displaying rows received from the database in a [`QueryRowsResult`]. |
There was a problem hiding this comment.
📌 As I noted elsewhere, we should consider making this work both with QueryRowsResult and with TypedRowStream.
Michu1596
force-pushed
the
query_rows_result_displayer
branch
from
7b10b8b to
7687302
Compare
github-actions
bot
added
the
semver-checks-breaking
Michu1596
force-pushed
the
query_rows_result_displayer
branch
from
222febc to
549fd82
Compare
github-actions
bot
removed
the
semver-checks-breaking
| impl fmt::Display for WrapperDisplay<'_, CqlDate> { | ||
| fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
| // example of formating date 2021-12-31 | ||
| let magic_constant = 2055453495; // it is number of days from -5877641-06-23 to -250000-01-01 | ||
| // around -250000-01-01 is the limit of naive date | ||
|
|
There was a problem hiding this comment.
❓ Please explain in detail what the "magic constant" is and why it is correct in your opinion.
There was a problem hiding this comment.
I believe it is correct because the dates printed by the displayer are the same as those inserted into the database, this constant allows shifting the date calculation to a valid range, thus preventing from_ymd_opt() from returning None. The date -5877641-06-23 is the reference point for CqlDate.
…qlsh` and examples of usage
…r.rs, made code more idiomatic
Michu1596
force-pushed
the
query_rows_result_displayer
branch
from
549fd82 to
9b05d0e
Compare
|
rebased on main |
…g for rows_displayer, made code more idiomatic
|
addressed comments |
This PR adds Displayer for QueryRowsResult trying to mimic
cqlshway of displaying tables, but there are some differences:cqlshdisplays this type in an unclear way,for more details look in the comments.list,map,set,tuple,udt), I use quotes only fortextas I think putting other types in quotes might be confusing.Fixes: #999
Pre-review checklist
./docs/source/.Fixes:annotations to PR description.