Picosecond support#1365
Conversation
e2fb822 to
589051a
Compare
589051a to
1852d5f
Compare
|
|
||
| void InsertTimestampData(std::shared_ptr<ODBCHandles> const& conn, | ||
| std::vector<SQL_TIMESTAMP_STRUCT> rows, | ||
| std::vector<std::string> rows, |
There was a problem hiding this comment.
why is this modified?
There was a problem hiding this comment.
Timestamp struct only contains fractions upto 9 digits. We are utilising this func in picoseconds as well.Hence updated it.
1852d5f to
4a7a338
Compare
| << (row.minute < 10 ? "0" : "") << row.minute << ":" | ||
| << (row.second < 10 ? "0" : "") << row.second << "." | ||
| << row.fraction << "'"; | ||
| // if (row.year != 0) { |
There was a problem hiding this comment.
there are so many of these unnecessary code, remove it if not needed
| std::string insert_stmt_str = insert_stmt.str(); | ||
| SQLRETURN status; | ||
|
|
||
| std::cout << "Insert Timestamp Statement: " << insert_stmt_str << std::endl; |
There was a problem hiding this comment.
please remove these prints
64602a3 to
41805f5
Compare
| *res_len = kTimestampBinaryLength; | ||
| } | ||
| timestamp_src_struct.fraction = timestamp_src_struct.fraction * 1000; | ||
| timestamp_src_struct.fraction = timestamp_src_struct.fraction; |
There was a problem hiding this comment.
why we removed this 1000 factor here?
There was a problem hiding this comment.
Because it was always converting to nanoseconds. even when not needed.
| &ts.month, &ts.day, &ts.hour, &ts.minute, &ts.second, µ); | ||
| } | ||
|
|
||
| if (matched < 6) { |
There was a problem hiding this comment.
how is this working? we have duplicate blocks of if condition and if needed see if can be combined
|
@Khushikathuria008 also add full test cases output |
5b972cb to
17776b4
Compare
|
|
||
| DSRow CreateDSRowFromTypeInfo(TypeInfoRow const& type_info) { | ||
| bool IsTimeRelated(TypeInfoRow const& type_info) { | ||
| return type_info.sql_data_type == SQL_TYPE_TIMESTAMP || |
There was a problem hiding this comment.
have we verified Date also has picoseconds support?
There was a problem hiding this comment.
Yes. date doesnt support it.
17776b4 to
e0750c8
Compare
e0750c8 to
12494e8
Compare
9e4d292 to
12494e8
Compare
|
@Khushikathuria008 Please rebase this branch from main and resolve the merge conflicts. |
12494e8 to
2258ecd
Compare
|
@Khushikathuria008 Can you please refactor this PR to have only the REST flow. Let's get it merged before we get into the HTAPI flow. |
43658b1 to
c5cd918
Compare
c5cd918 to
53cd400
Compare
| TEST(DataTranslationTest, From_SQL_Timestamp_to_all_picosecond_support) { | ||
| auto conn = std::make_shared<ODBCHandles>(); | ||
| std::string connection_string = | ||
| kDefaultConnectionString + ";TimestampOutputFormat=ISO8601_STRING;"; |
There was a problem hiding this comment.
what is this new param 'TimestampOutputFormat' and why is this needed? don't think there was a requirement for this
There was a problem hiding this comment.
The TimestampOutputFormat parameter controls the format in which timestamp values are returned by the driver. To have the driver return timestamps as ISO-8601 strings and preserve picosecond precision, set TimestampOutputFormat=ISO8601_STRING.
There was a problem hiding this comment.
can keep a similar bool useFullPrecisionTimestamp true or false instead. @sachinpro can confirm?
| status_record = | ||
| StatusRecord{SQLStates::k_22003(), "Buffer length is insufficient"}; | ||
| // --- Check for ISO string with 'T' and long fraction --- | ||
| auto t_pos = str_val.find('T'); |
There was a problem hiding this comment.
can we move this logic to another function and document the same
| std::vector<ConnectionProperty> connection_properties; | ||
| std::uint32_t row_fetched_per_block = 100000; | ||
| std::uint32_t default_string_column_length = 16384; | ||
| google::cloud::bigquery_v2_minimal_internal::DataFormatOptions format_options; |
There was a problem hiding this comment.
remove if not needed
There was a problem hiding this comment.
Its needed for setting the values in DSN.
| return status_record; | ||
| } | ||
|
|
||
| odbc_internal::StatusRecord ConvertTimestampStringToChar( |
There was a problem hiding this comment.
please write unit test cases as well
53cd400 to
ef2e961
Compare
ef2e961 to
6cf0daf
Compare
0d1a3ad to
c4f7ba0
Compare
c4f7ba0 to
ea071ad
Compare
This is a test PR created to validate picosecond support in the driver.
The following changes were made as part of this validation:
Create callback:
Insert Callback:
Select Callback:
Drop callback:
Full test-suite run on local:
