Fix truncate behavior #16
Conversation
| std::unique_ptr<duckdb::Connection> con = | ||
| get_connection(request->configuration(), db_name); | ||
|
|
||
| if (table_exists(*con, table_name)) { | ||
| truncate_table(*con, table_name); | ||
| std::chrono::nanoseconds delete_before_ts = | ||
| std::chrono::seconds(request->utc_delete_before().seconds()) + |
There was a problem hiding this comment.
Shouldn't it be converted to ns?
| std::chrono::seconds(request->utc_delete_before().seconds()) + | |
| std::chrono::seconds(request->utc_delete_before().seconds()) * 1e9 + |
But also, doesn't std::chrono::nanoseconds(request->utc_delete_before().nanos()) already give you what you need? Does it just give you the fractional seconds part?
There was a problem hiding this comment.
Adding seconds to nanoseconds gets seconds converted automatically (docs)
When two duration objects of different types are involved, the one with the longest period (as determined by common_type) is converted before the operation.
nanos() only contain the fractional part -- proto docs.
There was a problem hiding this comment.
A bit too magical for me, but I guess I'm not on the C++ design committee :-)
You got to love this radically opposite approach between C++ stdlib that makes it as nice and expressive as possible vs the proto which complicate things and wants to split seconds and ns!
There was a problem hiding this comment.
I was actually grateful that for once C++ did not make me invent every basic utility from first principles, but yeah, it's not super intuitive either.
And I have no idea what proto designers were thinking here. I guess it's space saving for usecases where only second granularity is needed? But who uses second granularity anymore -- it's not 1999.
| const std::string absolute_table_name = table.to_string(); | ||
| std::ostringstream sql; | ||
| sql << "DELETE FROM " + absolute_table_name; | ||
|
|
||
| sql << "UPDATE " << absolute_table_name << " SET " |
There was a problem hiding this comment.
(sorry if I already asked that before...) we can't quote the table name?
There was a problem hiding this comment.
table_def::to_string() quotes all parts
There was a problem hiding this comment.
perhaps I should rename it to to_quoted_string
There was a problem hiding this comment.
renamed to to_escaped_string
Two fixes for truncate behavior:
utc_delete_before(timestamp prior to which to delete rows).Fixes #15
Fixes #9