Skip to content

Commit 0af0b11

Browse files
authored
Enable session time zone override for MySQL (#387)
1 parent d2cfe46 commit 0af0b11

6 files changed

Lines changed: 117 additions & 18 deletions

File tree

src/duckdb/creator.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,7 @@ impl TableManager {
297297

298298
/// Inserts data from this table into the target table.
299299
#[tracing::instrument(level = "debug", skip_all)]
300+
#[allow(dead_code)]
300301
pub(crate) fn insert_into(
301302
&self,
302303
table: &TableManager,

src/sql/arrow_sql_gen/mysql.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ use chrono::{NaiveDate, NaiveTime, Timelike};
1515
use mysql_async::{consts::ColumnFlags, consts::ColumnType, FromValueError, Row, Value};
1616
use snafu::{ResultExt, Snafu};
1717
use std::{convert, sync::Arc};
18-
use time::PrimitiveDateTime;
1918

2019
#[derive(Debug, Snafu)]
2120
pub enum Error {
@@ -527,7 +526,7 @@ pub fn rows_to_arrow(rows: &[Row], projected_schema: &Option<SchemaRef>) -> Resu
527526
.fail();
528527
};
529528
let v = match handle_null_error(
530-
row.get_opt::<PrimitiveDateTime, usize>(i).transpose(),
529+
row.get_opt::<chrono::NaiveDateTime, usize>(i).transpose(),
531530
) {
532531
Ok(v) => v,
533532
Err(err) => {
@@ -546,10 +545,7 @@ pub fn rows_to_arrow(rows: &[Row], projected_schema: &Option<SchemaRef>) -> Resu
546545

547546
match v {
548547
Some(v) => {
549-
#[allow(clippy::cast_possible_truncation)]
550-
let timestamp_micros =
551-
(v.assume_utc().unix_timestamp_nanos() / 1_000) as i64;
552-
builder.append_value(timestamp_micros);
548+
builder.append_value(v.and_utc().timestamp_micros());
553549
}
554550
None => builder.append_null(),
555551
}

src/sql/db_connection_pool/mysqlpool.rs

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,16 @@ pub struct MySQLConnectionPool {
5757
join_push_down: JoinPushDown,
5858
}
5959

60-
const SETUP_QUERIES: [&str; 4] = [
61-
"SET time_zone = '+00:00'",
62-
"SET character_set_results = 'utf8mb4'",
63-
"SET character_set_client = 'utf8mb4'",
64-
"SET character_set_connection = 'utf8mb4'",
65-
];
60+
/// Returns the setup queries for the MySQL connection, optionally overriding default time zone (UTC).
61+
fn get_setup_queries(time_zone: Option<&str>) -> Vec<String> {
62+
let tz = time_zone.unwrap_or("+00:00");
63+
vec![
64+
format!("SET time_zone = '{tz}'"),
65+
"SET character_set_results = 'utf8mb4'".to_string(),
66+
"SET character_set_client = 'utf8mb4'".to_string(),
67+
"SET character_set_connection = 'utf8mb4'".to_string(),
68+
]
69+
}
6670

6771
impl MySQLConnectionPool {
6872
/// Creates a new instance of `MySQLConnectionPool`.
@@ -80,6 +84,7 @@ impl MySQLConnectionPool {
8084
/// * `sslrootcert` - The path to the root certificate to use when connecting to the MySQL database.
8185
/// * `pool_min` - The minimum number of connections to keep open in the pool, lazily created when requested.
8286
/// * `pool_max` - The maximum number of connections to allow in the pool.
87+
/// * `time_zone` - The time zone to use for the MySQL connection (e.g., "+2:00", "UTC", etc.). Default is "+00:00" (UTC).
8388
///
8489
/// # Errors
8590
///
@@ -167,7 +172,9 @@ impl MySQLConnectionPool {
167172

168173
connection_string = connection_string.ssl_opts(ssl_opts);
169174

170-
connection_string = connection_string.setup(SETUP_QUERIES.to_vec());
175+
connection_string = connection_string.setup(get_setup_queries(
176+
params.get("time_zone").map(SecretBox::expose_secret),
177+
));
171178

172179
let opts = mysql_async::Opts::from(connection_string);
173180

tests/docker/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ impl<'a> ContainerRunner<'a> {
134134
format!("{container_port}/tcp"),
135135
Some(vec![PortBinding {
136136
host_ip: Some("127.0.0.1".to_string()),
137-
host_port: Some(format!("{host_port}/tcp")),
137+
host_port: Some(format!("{host_port}")),
138138
}]),
139139
);
140140
}

tests/mysql/common.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use crate::{
1212
const MYSQL_ROOT_PASSWORD: &str = "integration-test-pw";
1313
const MYSQL_DOCKER_CONTAINER: &str = "runtime-integration-test-mysql";
1414

15-
fn get_mysql_params(port: usize) -> HashMap<String, SecretString> {
15+
fn get_mysql_params(port: usize, time_zone: Option<&str>) -> HashMap<String, SecretString> {
1616
let mut params = HashMap::new();
1717
params.insert(
1818
"mysql_host".to_string(),
@@ -46,6 +46,9 @@ fn get_mysql_params(port: usize) -> HashMap<String, SecretString> {
4646
"mysql_pool_max".to_string(),
4747
SecretString::from("10".to_string()),
4848
);
49+
if let Some(tz) = time_zone {
50+
params.insert("mysql_time_zone".to_string(), SecretString::from(tz));
51+
}
4952
params
5053
}
5154

@@ -87,8 +90,9 @@ pub async fn start_mysql_docker_container(port: usize) -> Result<RunningContaine
8790
#[instrument]
8891
pub(super) async fn get_mysql_connection_pool(
8992
port: usize,
93+
time_zone: Option<&str>,
9094
) -> Result<MySQLConnectionPool, anyhow::Error> {
91-
let mysql_pool = MySQLConnectionPool::new(get_mysql_params(port))
95+
let mysql_pool = MySQLConnectionPool::new(get_mysql_params(port, time_zone))
9296
.await
9397
.expect("Failed to create MySQL Connection Pool");
9498

tests/mysql/mod.rs

Lines changed: 93 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,75 @@ VALUES
109109
create_table_stmt,
110110
insert_table_stmt,
111111
expected_record,
112+
None,
113+
)
114+
.await;
115+
}
116+
117+
/// Tests the MySQL TIMESTAMP with time zone override.
118+
/// The test verifies that the TIMESTAMP type correctly adjusts to the specified time zone when retrieved from the database.
119+
/// `TIMESTAMP` columns should be automatically converted to the specified time zone,
120+
/// while `DATETIME` columns should remain unchanged.
121+
async fn test_mysql_timestamp_tz_override(port: usize) {
122+
let create_table_stmt = "
123+
CREATE TABLE timestamp_tz_table (
124+
ts TIMESTAMP,
125+
dt DATETIME
126+
);
127+
";
128+
// values will be inserted in UTC
129+
let insert_table_stmt = "
130+
INSERT INTO timestamp_tz_table (ts, dt)
131+
VALUES
132+
('2024-09-12 10:00:00', '2024-09-12 10:00:00');
133+
";
134+
135+
let schema = Arc::new(Schema::new(vec![
136+
Field::new("ts", DataType::Timestamp(TimeUnit::Microsecond, None), true),
137+
Field::new("dt", DataType::Timestamp(TimeUnit::Microsecond, None), true),
138+
]));
139+
140+
// Both columns should remain unchanged as target time zone is UTC (same as insert time zone)
141+
let expected_utc = RecordBatch::try_new(
142+
Arc::clone(&schema),
143+
vec![
144+
Arc::new(TimestampMicrosecondArray::from(vec![1_726_135_200_000_000])),
145+
Arc::new(TimestampMicrosecondArray::from(vec![1_726_135_200_000_000])),
146+
],
147+
)
148+
.expect("Failed to created arrow record batch");
149+
150+
arrow_mysql_one_way(
151+
port,
152+
"timestamp_tz_table",
153+
create_table_stmt,
154+
insert_table_stmt,
155+
expected_utc,
156+
Some("UTC"),
157+
)
158+
.await;
159+
160+
// "+02:00"
161+
let expected_custom_tz = RecordBatch::try_new(
162+
Arc::clone(&schema),
163+
vec![
164+
// ts: TIMESTAMP column, should be shifted +2 hours (timezone override)
165+
Arc::new(TimestampMicrosecondArray::from(vec![
166+
1_726_135_200_000_000 + 2 * 60 * 60 * 1_000_000,
167+
])),
168+
// dt: DATETIME column, should remain unchanged
169+
Arc::new(TimestampMicrosecondArray::from(vec![1_726_135_200_000_000])),
170+
],
171+
)
172+
.expect("Failed to created arrow record batch");
173+
174+
arrow_mysql_one_way(
175+
port,
176+
"timestamp_tz_table",
177+
create_table_stmt,
178+
insert_table_stmt,
179+
expected_custom_tz,
180+
Some("+02:00"), // Override time zone to +02:00
112181
)
113182
.await;
114183
}
@@ -197,6 +266,7 @@ VALUES (
197266
create_table_stmt,
198267
insert_table_stmt,
199268
expected_record,
269+
None,
200270
)
201271
.await;
202272
}
@@ -269,6 +339,7 @@ VALUES
269339
create_table_stmt,
270340
insert_table_stmt,
271341
expected_record,
342+
None,
272343
)
273344
.await;
274345
}
@@ -315,6 +386,7 @@ VALUES
315386
create_table_stmt,
316387
insert_table_stmt,
317388
expected_record,
389+
None,
318390
)
319391
.await;
320392
}
@@ -377,6 +449,7 @@ VALUES
377449
create_table_stmt,
378450
insert_table_stmt,
379451
expected_record,
452+
None,
380453
)
381454
.await;
382455
}
@@ -429,6 +502,7 @@ VALUES
429502
create_table_stmt,
430503
insert_table_stmt,
431504
expected_record,
505+
None,
432506
)
433507
.await;
434508
}
@@ -484,6 +558,7 @@ INSERT INTO high_precision_decimal (decimal_values) VALUES
484558
create_table_stmt,
485559
insert_table_stmt,
486560
expected_record,
561+
None,
487562
)
488563
.await;
489564
}
@@ -559,6 +634,7 @@ async fn test_mysql_zero_date_type(port: usize) {
559634
create_table_stmt,
560635
insert_table_stmt,
561636
expected_record,
637+
None,
562638
)
563639
.await;
564640
}
@@ -593,6 +669,7 @@ async fn test_mysql_decimal_types_to_decimal128(port: usize) {
593669
create_table_stmt,
594670
insert_table_stmt,
595671
expected_record,
672+
None,
596673
)
597674
.await;
598675
}
@@ -603,13 +680,15 @@ async fn arrow_mysql_one_way(
603680
create_table_stmt: &str,
604681
insert_table_stmt: &str,
605682
expected_record: RecordBatch,
683+
test_query_tz: Option<&str>,
606684
) -> Vec<RecordBatch> {
607685
tracing::debug!("Running tests on {table_name}");
608686

609687
let ctx = SessionContext::new();
610-
let pool = common::get_mysql_connection_pool(port)
688+
// For dataset initialization we always use UTC (default) timezone
689+
let pool = common::get_mysql_connection_pool(port, None)
611690
.await
612-
.expect("MySQL connection pool should be created");
691+
.expect("MySQL connection pool for test table creation should be created");
613692

614693
let db_conn = pool
615694
.connect_direct()
@@ -625,6 +704,12 @@ async fn arrow_mysql_one_way(
625704
.await
626705
.expect("SQL mode should be adjusted");
627706

707+
// Drop table if already exists
708+
let _ = db_conn
709+
.execute(format!("DROP TABLE IF EXISTS {table_name}").as_str(), &[])
710+
.await
711+
.expect("MySQL table should be dropped if exists");
712+
628713
// Create table and insert data into mysql test_table
629714
let _ = db_conn
630715
.execute(create_table_stmt, &[])
@@ -636,6 +721,11 @@ async fn arrow_mysql_one_way(
636721
.await
637722
.expect("MySQL table data should be inserted");
638723

724+
// For the test query, use a new connection pool with optional time zone override
725+
let pool = common::get_mysql_connection_pool(port, test_query_tz)
726+
.await
727+
.expect("MySQL connection pool for test query should be created");
728+
639729
// Register datafusion table, test mysql row -> arrow conversion
640730
let sqltable_pool: Arc<
641731
dyn DbConnectionPool<mysql_async::Conn, &'static (dyn ToValue + Sync)>
@@ -685,6 +775,7 @@ async fn test_mysql_arrow_oneway() {
685775
let mysql_container = start_mysql_container(port).await;
686776

687777
test_mysql_timestamp_types(port).await;
778+
test_mysql_timestamp_tz_override(port).await;
688779
test_mysql_datetime_types(port).await;
689780
test_mysql_time_types(port).await;
690781
test_mysql_enum_types(port).await;

0 commit comments

Comments
 (0)