Skip to content

Commit cd0d58c

Browse files
fix: correct date/time type format for postgresql (#1001)
* fix: correct date/time type format for postgresql * fix: tests for timestamp * refactor: use Utc datetime for timestamp::to_chrono_datetime * Update src/servers/Cargo.toml Co-authored-by: LFC <bayinamine@gmail.com> --------- Co-authored-by: LFC <bayinamine@gmail.com>
1 parent 8b86964 commit cd0d58c

File tree

6 files changed

+74
-14
lines changed

6 files changed

+74
-14
lines changed

Cargo.lock

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/common/time/src/date.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,10 @@ impl Date {
7171
pub fn val(&self) -> i32 {
7272
self.0
7373
}
74+
75+
pub fn to_chrono_date(&self) -> Option<NaiveDate> {
76+
NaiveDate::from_num_days_from_ce_opt(UNIX_EPOCH_FROM_CE + self.0)
77+
}
7478
}
7579

7680
#[cfg(test)]

src/common/time/src/datetime.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use crate::error::{Error, ParseDateStrSnafu, Result};
2323

2424
const DATETIME_FORMAT: &str = "%F %T";
2525

26-
/// [DateTime] represents the **seconds elapsed since "1970-01-01 00:00:00 UTC" (UNIX Epoch)**.
26+
/// [DateTime] represents the **seconds elapsed since "1970-01-01 00:00:00 UTC" (UNIX Epoch)**.
2727
#[derive(
2828
Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Default, Serialize, Deserialize,
2929
)]
@@ -69,6 +69,10 @@ impl DateTime {
6969
pub fn val(&self) -> i64 {
7070
self.0
7171
}
72+
73+
pub fn to_chrono_datetime(&self) -> Option<NaiveDateTime> {
74+
NaiveDateTime::from_timestamp_millis(self.0)
75+
}
7276
}
7377

7478
#[cfg(test)]

src/common/time/src/timestamp.rs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -114,20 +114,23 @@ impl Timestamp {
114114
/// Format timestamp to ISO8601 string. If the timestamp exceeds what chrono timestamp can
115115
/// represent, this function simply print the timestamp unit and value in plain string.
116116
pub fn to_iso8601_string(&self) -> String {
117-
let nano_factor = TimeUnit::Second.factor() / TimeUnit::Nanosecond.factor();
117+
if let LocalResult::Single(datetime) = self.to_chrono_datetime() {
118+
format!("{}", datetime.format("%Y-%m-%d %H:%M:%S%.f%z"))
119+
} else {
120+
format!("[Timestamp{}: {}]", self.unit, self.value)
121+
}
122+
}
118123

124+
pub fn to_chrono_datetime(&self) -> LocalResult<DateTime<Utc>> {
125+
let nano_factor = TimeUnit::Second.factor() / TimeUnit::Nanosecond.factor();
119126
let (mut secs, mut nsecs) = self.split();
120127

121128
if nsecs < 0 {
122129
secs -= 1;
123130
nsecs += nano_factor;
124131
}
125132

126-
if let LocalResult::Single(datetime) = Utc.timestamp_opt(secs, nsecs as u32) {
127-
format!("{}", datetime.format("%Y-%m-%d %H:%M:%S%.f%z"))
128-
} else {
129-
format!("[Timestamp{}: {}]", self.unit, self.value)
130-
}
133+
Utc.timestamp_opt(secs, nsecs as u32)
131134
}
132135
}
133136

src/servers/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ axum-macros = "0.3"
1414
base64 = "0.13"
1515
bytes = "1.2"
1616
catalog = { path = "../catalog" }
17+
chrono.workspace = true
1718
common-base = { path = "../common/base" }
1819
common-catalog = { path = "../common/catalog" }
1920
common-error = { path = "../common/error" }
@@ -40,6 +41,7 @@ openmetrics-parser = "0.4"
4041
opensrv-mysql = { git = "https://github.com/datafuselabs/opensrv", rev = "b44c9d1360da297b305abf33aecfa94888e1554c" }
4142
pgwire = "0.10"
4243
pin-project = "1.0"
44+
postgres-types = { version = "0.2", features = ["with-chrono-0_4"] }
4345
prost.workspace = true
4446
query = { path = "../query" }
4547
rand = "0.8"

src/servers/src/postgres/handler.rs

Lines changed: 51 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use std::sync::Arc;
1717
use std::time::Duration;
1818

1919
use async_trait::async_trait;
20+
use chrono::LocalResult;
2021
use common_query::Output;
2122
use common_recordbatch::error::Result as RecordBatchResult;
2223
use common_recordbatch::RecordBatch;
@@ -171,9 +172,37 @@ fn encode_text_value(value: &Value, builder: &mut DataRowEncoder) -> PgWireResul
171172
Value::Float64(v) => builder.encode_text_format_field(Some(&v.0)),
172173
Value::String(v) => builder.encode_text_format_field(Some(&v.as_utf8())),
173174
Value::Binary(v) => builder.encode_text_format_field(Some(&hex::encode(v.deref()))),
174-
Value::Date(v) => builder.encode_text_format_field(Some(&v.to_string())),
175-
Value::DateTime(v) => builder.encode_text_format_field(Some(&v.to_string())),
176-
Value::Timestamp(v) => builder.encode_text_format_field(Some(&v.to_iso8601_string())),
175+
Value::Date(v) => {
176+
if let Some(date) = v.to_chrono_date() {
177+
builder.encode_text_format_field(Some(&date.format("%Y-%m-%d").to_string()))
178+
} else {
179+
Err(PgWireError::ApiError(Box::new(Error::Internal {
180+
err_msg: format!("Failed to convert date to postgres type {v:?}",),
181+
})))
182+
}
183+
}
184+
Value::DateTime(v) => {
185+
if let Some(datetime) = v.to_chrono_datetime() {
186+
builder.encode_text_format_field(Some(
187+
&datetime.format("%Y-%m-%d %H:%M:%S%.6f").to_string(),
188+
))
189+
} else {
190+
Err(PgWireError::ApiError(Box::new(Error::Internal {
191+
err_msg: format!("Failed to convert date to postgres type {v:?}",),
192+
})))
193+
}
194+
}
195+
Value::Timestamp(v) => {
196+
if let LocalResult::Single(datetime) = v.to_chrono_datetime() {
197+
builder.encode_text_format_field(Some(
198+
&datetime.format("%Y-%m-%d %H:%M:%S%.6f").to_string(),
199+
))
200+
} else {
201+
Err(PgWireError::ApiError(Box::new(Error::Internal {
202+
err_msg: format!("Failed to convert date to postgres type {v:?}",),
203+
})))
204+
}
205+
}
177206
Value::List(_) => Err(PgWireError::ApiError(Box::new(Error::Internal {
178207
err_msg: format!(
179208
"cannot write value {:?} in postgres protocol: unimplemented",
@@ -203,17 +232,32 @@ fn encode_binary_value(
203232
Value::Float64(v) => builder.encode_binary_format_field(&v.0, datatype),
204233
Value::String(v) => builder.encode_binary_format_field(&v.as_utf8(), datatype),
205234
Value::Binary(v) => builder.encode_binary_format_field(&v.deref(), datatype),
206-
// TODO(sunng87): correct date/time types encoding
207-
Value::Date(v) => builder.encode_binary_format_field(&v.to_string(), datatype),
208-
Value::DateTime(v) => builder.encode_binary_format_field(&v.to_string(), datatype),
235+
Value::Date(v) => {
236+
if let Some(date) = v.to_chrono_date() {
237+
builder.encode_binary_format_field(&date, datatype)
238+
} else {
239+
Err(PgWireError::ApiError(Box::new(Error::Internal {
240+
err_msg: format!("Failed to convert date to postgres type {v:?}",),
241+
})))
242+
}
243+
}
244+
Value::DateTime(v) => {
245+
if let Some(datetime) = v.to_chrono_datetime() {
246+
builder.encode_binary_format_field(&datetime, datatype)
247+
} else {
248+
Err(PgWireError::ApiError(Box::new(Error::Internal {
249+
err_msg: format!("Failed to convert datetime to postgres type {v:?}",),
250+
})))
251+
}
252+
}
209253
Value::Timestamp(v) => {
210254
// convert timestamp to SystemTime
211255
if let Some(ts) = v.convert_to(TimeUnit::Microsecond) {
212256
let sys_time = std::time::UNIX_EPOCH + Duration::from_micros(ts.value() as u64);
213257
builder.encode_binary_format_field(&sys_time, datatype)
214258
} else {
215259
Err(PgWireError::ApiError(Box::new(Error::Internal {
216-
err_msg: format!("Failed to conver timestamp to postgres type {v:?}",),
260+
err_msg: format!("Failed to convert timestamp to postgres type {v:?}",),
217261
})))
218262
}
219263
}

0 commit comments

Comments
 (0)