From e7e1f9b1c6624880252b8a98a6dddd3bef41728e Mon Sep 17 00:00:00 2001 From: shbhmrzd Date: Sun, 28 Jun 2026 20:03:23 +0530 Subject: [PATCH 01/11] feat(connectors): add generic JDBC source connector Adds a JDBC source connector that polls a configured SQL query against any JDBC-compliant database (PostgreSQL, MySQL, Oracle, SQL Server, H2) through an embedded JVM and produces each row as a JSON message. Supports bulk and incremental (offset-tracked) modes with persisted offset state. Also adds the shared connector integration-test harness and a small TCP listener change so that harness can read the server's written runtime config when bound to a fixed port. The matching JDBC sink connector follows in a separate PR. --- .config/nextest.toml | 12 + .github/workflows/_build_rust_artifacts.yml | 2 +- .github/workflows/edge-release.yml | 1 + Cargo.lock | 55 +- Cargo.toml | 1 + core/connectors/README.md | 1 + .../connectors/jdbc_bulk_mode.toml | 81 + .../example_config/connectors/jdbc_h2.toml | 60 + .../example_config/connectors/jdbc_mysql.toml | 85 + .../connectors/jdbc_oracle.toml | 82 + .../connectors/jdbc_sqlserver.toml | 66 + .../connectors/test_jdbc_h2.toml | 45 + core/connectors/sources/README.md | 1 + .../connectors/sources/jdbc_source/Cargo.toml | 67 + core/connectors/sources/jdbc_source/README.md | 539 ++++ .../sources/jdbc_source/config.toml | 50 + .../connectors/sources/jdbc_source/src/lib.rs | 2843 +++++++++++++++++ .../connectors/jdbc/config_postgres.toml | 22 + .../connectors_config_postgres/jdbc_pg.toml | 49 + core/integration/tests/connectors/jdbc/mod.rs | 21 + .../connectors/jdbc/test_with_postgres.rs | 624 ++++ core/integration/tests/connectors/mod.rs | 190 +- core/server/src/tcp/tcp_listener.rs | 8 +- 23 files changed, 4898 insertions(+), 7 deletions(-) create mode 100644 core/connectors/runtime/example_config/connectors/jdbc_bulk_mode.toml create mode 100644 core/connectors/runtime/example_config/connectors/jdbc_h2.toml create mode 100644 core/connectors/runtime/example_config/connectors/jdbc_mysql.toml create mode 100644 core/connectors/runtime/example_config/connectors/jdbc_oracle.toml create mode 100644 core/connectors/runtime/example_config/connectors/jdbc_sqlserver.toml create mode 100644 core/connectors/runtime/example_config/connectors/test_jdbc_h2.toml create mode 100644 core/connectors/sources/jdbc_source/Cargo.toml create mode 100644 core/connectors/sources/jdbc_source/README.md create mode 100644 core/connectors/sources/jdbc_source/config.toml create mode 100644 core/connectors/sources/jdbc_source/src/lib.rs create mode 100644 core/integration/tests/connectors/jdbc/config_postgres.toml create mode 100644 core/integration/tests/connectors/jdbc/connectors_config_postgres/jdbc_pg.toml create mode 100644 core/integration/tests/connectors/jdbc/mod.rs create mode 100644 core/integration/tests/connectors/jdbc/test_with_postgres.rs diff --git a/.config/nextest.toml b/.config/nextest.toml index 3070df8bdc..916c27ecd0 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -48,6 +48,18 @@ max-threads = 1 filter = 'package(integration) and test(/connectors::elasticsearch::/)' test-group = "elasticsearch" +# JDBC tests each start their own iggy-server plus a Postgres testcontainer and +# an embedded JVM, and they share a JDBC driver JAR downloaded to a single path +# on first run. nextest runs each test in its own process, so the in-source +# `#[serial]` guard does not serialize them here; `max-threads = 1` does, which +# avoids resource contention and a race to download the same driver JAR. +[test-groups.jdbc] +max-threads = 1 + +[[profile.default.overrides]] +filter = 'package(integration) and test(/connectors::jdbc::/)' +test-group = "jdbc" + [profile.default] slow-timeout = { period = "60s", terminate-after = 5 } diff --git a/.github/workflows/_build_rust_artifacts.yml b/.github/workflows/_build_rust_artifacts.yml index 5dfe055149..316bfc2ade 100644 --- a/.github/workflows/_build_rust_artifacts.yml +++ b/.github/workflows/_build_rust_artifacts.yml @@ -46,7 +46,7 @@ on: connector_plugins: type: string required: false - default: "iggy_connector_elasticsearch_sink,iggy_connector_elasticsearch_source,iggy_connector_iceberg_sink,iggy_connector_postgres_sink,iggy_connector_postgres_source,iggy_connector_quickwit_sink,iggy_connector_random_source,iggy_connector_s3_sink,iggy_connector_stdout_sink,iggy_connector_surrealdb_sink" + default: "iggy_connector_elasticsearch_sink,iggy_connector_elasticsearch_source,iggy_connector_iceberg_sink,iggy_connector_jdbc_source,iggy_connector_postgres_sink,iggy_connector_postgres_source,iggy_connector_quickwit_sink,iggy_connector_random_source,iggy_connector_s3_sink,iggy_connector_stdout_sink,iggy_connector_surrealdb_sink" description: "Comma-separated list of connector plugin crates to build as shared libraries" outputs: artifact_name: diff --git a/.github/workflows/edge-release.yml b/.github/workflows/edge-release.yml index 87b7eb3b84..613d6a4b95 100644 --- a/.github/workflows/edge-release.yml +++ b/.github/workflows/edge-release.yml @@ -104,6 +104,7 @@ jobs: - `iggy_connector_elasticsearch_sink` - `iggy_connector_elasticsearch_source` - `iggy_connector_iceberg_sink` + - `iggy_connector_jdbc_source` - `iggy_connector_postgres_sink` - `iggy_connector_postgres_source` - `iggy_connector_quickwit_sink` diff --git a/Cargo.lock b/Cargo.lock index 22cbf3cfe2..e274442948 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2742,7 +2742,7 @@ checksum = "0b023947811758c97c59bf9d1c188fd619ad4718dcaa767947df1cadb14f39f4" dependencies = [ "glob", "libc", - "libloading", + "libloading 0.8.9", ] [[package]] @@ -6203,6 +6203,16 @@ version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "135b12329e5e3ce057a9f972339ea52bc954fe1e9358ef27f95e89716fbc5424" +[[package]] +name = "humantime-serde" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "57a3db5ea5923d99402c94e9feb261dc5ee9b4efa158b0315f788cf549cc200c" +dependencies = [ + "humantime", + "serde", +] + [[package]] name = "hwlocality" version = "1.0.0-alpha.12" @@ -7024,6 +7034,28 @@ dependencies = [ "uuid", ] +[[package]] +name = "iggy_connector_jdbc_source" +version = "0.1.0" +dependencies = [ + "async-trait", + "base64", + "chrono", + "dashmap", + "humantime-serde", + "iggy_common", + "iggy_connector_sdk", + "jni 0.21.1", + "regex", + "secrecy", + "serde", + "serde_json", + "tokio", + "toml 1.1.2+spec-1.1.0", + "tracing", + "uuid", +] + [[package]] name = "iggy_connector_mongodb_sink" version = "0.4.1-edge.1" @@ -7526,6 +7558,15 @@ version = "1.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f42a60cbdf9a97f5d2305f08a87dc4e09308d1276d28c869c684d7777685682" +[[package]] +name = "java-locator" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09c46c1fe465c59b1474e665e85e1256c3893dd00927b8d55f63b09044c1e64f" +dependencies = [ + "glob", +] + [[package]] name = "jiff" version = "0.2.29" @@ -7577,7 +7618,9 @@ dependencies = [ "cesu8", "cfg-if", "combine", + "java-locator", "jni-sys 0.3.1", + "libloading 0.7.4", "log", "thiserror 1.0.69", "walkdir", @@ -7906,6 +7949,16 @@ dependencies = [ "pkg-config", ] +[[package]] +name = "libloading" +version = "0.7.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b67380fd3b2fbe7527a606e18729d21c6f3951633d0500574c4dc22d2d638b9f" +dependencies = [ + "cfg-if", + "winapi", +] + [[package]] name = "libloading" version = "0.8.9" diff --git a/Cargo.toml b/Cargo.toml index 87a96add3e..543b15ae83 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,6 +48,7 @@ members = [ "core/connectors/sinks/surrealdb_sink", "core/connectors/sources/elasticsearch_source", "core/connectors/sources/influxdb_source", + "core/connectors/sources/jdbc_source", "core/connectors/sources/postgres_source", "core/connectors/sources/random_source", "core/consensus", diff --git a/core/connectors/README.md b/core/connectors/README.md index 64bfc9a1f8..d93d22d99e 100644 --- a/core/connectors/README.md +++ b/core/connectors/README.md @@ -98,6 +98,7 @@ Please refer to the **[Source documentation](https://github.com/apache/iggy/tree ### Available Sources - **Elasticsearch Source** - polls documents from Elasticsearch indices +- **JDBC Source** - reads rows from any JDBC-compliant database (PostgreSQL, MySQL, Oracle, SQL Server, H2) via an embedded JVM; bulk and incremental modes - **PostgreSQL Source** - reads rows from PostgreSQL tables with multiple consumption strategies (delete after read, mark as processed, timestamp tracking) - **Random Source** - generates random test messages (useful for testing/development) diff --git a/core/connectors/runtime/example_config/connectors/jdbc_bulk_mode.toml b/core/connectors/runtime/example_config/connectors/jdbc_bulk_mode.toml new file mode 100644 index 0000000000..25e2b458ae --- /dev/null +++ b/core/connectors/runtime/example_config/connectors/jdbc_bulk_mode.toml @@ -0,0 +1,81 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Example JDBC Source Connector Configuration - BULK MODE +# Bulk mode works with ALL JDBC databases without any special requirements +# No tracking column needed - just executes your query and fetches results + +type = "source" +key = "jdbc_bulk_example" +enabled = true +version = 0 +name = "JDBC Bulk Mode Source" +path = "target/release/libiggy_connector_jdbc_source" +plugin_config_format = "toml" + +[plugin_config] +# This example uses PostgreSQL, but bulk mode works identically with: +# MySQL, Oracle, SQL Server, H2, Derby, DB2, etc. +jdbc_url = "jdbc:postgresql://localhost:5432/warehouse" + +# Database credentials can be in URL or separate +# jdbc_url = "jdbc:postgresql://localhost:5432/warehouse?user=myuser&password=mypass" +username = "warehouse_user" +password = "secret" + +driver_class = "org.postgresql.Driver" +driver_jar_path = "/opt/jdbc-drivers/postgresql-42.6.0.jar" + +# Bulk mode: Any valid SELECT query +# Can include JOINs, aggregations, complex WHERE clauses, etc. +query = """ +SELECT + p.product_id,\ + p.product_name, + p.category, + p.price, + COUNT(o.order_id) as total_orders, + SUM(o.quantity) as total_quantity +FROM products p +LEFT JOIN orders o ON p.product_id = o.product_id +GROUP BY p.product_id, p.product_name, p.category, p.price +""" + +# Poll once per hour for daily snapshots +poll_interval = "1h" + +# Large batch size for full table scans +batch_size = 10000 + +# BULK MODE - no tracking column needed! +mode = "bulk" + +# Bulk mode benefits: +# - No tracking column required +# - Works with any SELECT query +# - Supports complex queries (JOINs, aggregations, window functions) +# - Perfect for periodic snapshots +# - Universal compatibility with all databases + +snake_case_columns = true +include_metadata = false + +[[streams]] +stream = "warehouse" +topic = "product_summary" +partition_id = 1 +schema = "json" diff --git a/core/connectors/runtime/example_config/connectors/jdbc_h2.toml b/core/connectors/runtime/example_config/connectors/jdbc_h2.toml new file mode 100644 index 0000000000..82ee459ff5 --- /dev/null +++ b/core/connectors/runtime/example_config/connectors/jdbc_h2.toml @@ -0,0 +1,60 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Example JDBC Source Connector Configuration for H2 Database +# H2 is useful for testing and development as it's an embedded Java database + +type = "source" +key = "jdbc_h2_example" +enabled = true +version = 0 +name = "JDBC H2 Source" +path = "target/release/libiggy_connector_jdbc_source" +plugin_config_format = "toml" + +[plugin_config] +# H2 connection URL (in-memory database) +jdbc_url = "jdbc:h2:mem:testdb;DB_CLOSE_DELAY=-1" + +# H2 JDBC driver +driver_class = "org.h2.Driver" + +# Path to H2 driver JAR +# Download from: https://repo1.maven.org/maven2/com/h2database/h2/2.2.224/h2-2.2.224.jar +# Note: Update this path to match where you downloaded the JAR file +driver_jar_path = "/tmp/jdbc-drivers/h2-2.2.224.jar" + +# H2 credentials (default) +username = "sa" +password = "" + +# Simple query for testing +query = "SELECT * FROM users WHERE id > {last_offset} ORDER BY id" + +poll_interval = "10s" +batch_size = 100 +tracking_column = "id" +initial_offset = "0" +mode = "incremental" +snake_case_columns = false +include_metadata = true + +[[streams]] +stream = "test" +topic = "users" +partition_id = 1 +schema = "json" diff --git a/core/connectors/runtime/example_config/connectors/jdbc_mysql.toml b/core/connectors/runtime/example_config/connectors/jdbc_mysql.toml new file mode 100644 index 0000000000..fd2ed5fc48 --- /dev/null +++ b/core/connectors/runtime/example_config/connectors/jdbc_mysql.toml @@ -0,0 +1,85 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Example JDBC Source Connector Configuration for MySQL +# This file demonstrates how to configure the JDBC source connector +# to read data from a MySQL database and publish to Iggy streams. + +type = "source" +key = "jdbc_mysql_example" +enabled = true +version = 0 +name = "JDBC MySQL Source" +path = "target/release/libiggy_connector_jdbc_source" +plugin_config_format = "toml" + +[plugin_config] +# JDBC connection URL +# Option 1: Separate credentials (recommended) +jdbc_url = "jdbc:mysql://localhost:3306/ecommerce?useSSL=false&serverTimezone=UTC" + +# Option 2: Embedded credentials in URL (alternative) +# jdbc_url = "jdbc:mysql://iggy_user:iggy_password@localhost:3306/ecommerce?useSSL=false&serverTimezone=UTC" + +# JDBC driver class name +driver_class = "com.mysql.cj.jdbc.Driver" + +# Path to JDBC driver JAR file +# Download from: https://repo1.maven.org/maven2/com/mysql/mysql-connector-j/8.0.33/mysql-connector-j-8.0.33.jar +driver_jar_path = "/opt/jdbc-drivers/mysql-connector-j-8.0.33.jar" + +# Database credentials (optional if included in jdbc_url) +username = "iggy_user" +password = "iggy_password" + +# SQL query to execute +# Use {last_offset} placeholder for incremental reads +query = "SELECT * FROM orders WHERE updated_at > {last_offset} ORDER BY updated_at ASC" + +# How often to poll the database +poll_interval = "30s" + +# Maximum number of rows to fetch per poll +batch_size = 1000 + +# Column to track for incremental reads (must be in query result) +tracking_column = "updated_at" + +# Initial offset value for the first poll +initial_offset = "2024-01-01 00:00:00" + +# Source mode: "incremental" or "bulk" +# Note: Both modes work with ALL JDBC databases (MySQL, Oracle, PostgreSQL, etc.) +# - incremental: Tracks last offset, avoids duplicate reads +# - bulk: Full table scan, no offset tracking +mode = "incremental" + +# Convert column names to snake_case (e.g., OrderDate -> order_date) +snake_case_columns = true + +# Include metadata wrapper in output messages +include_metadata = true + +# Custom JVM options (optional) +jvm_options = ["-Xmx512m", "-Xms128m"] + +# Target Iggy stream and topic +[[streams]] +stream = "ecommerce" +topic = "orders" +partition_id = 1 +schema = "json" diff --git a/core/connectors/runtime/example_config/connectors/jdbc_oracle.toml b/core/connectors/runtime/example_config/connectors/jdbc_oracle.toml new file mode 100644 index 0000000000..04478bf4ab --- /dev/null +++ b/core/connectors/runtime/example_config/connectors/jdbc_oracle.toml @@ -0,0 +1,82 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Example JDBC Source Connector Configuration for Oracle Database + +type = "source" +key = "jdbc_oracle_example" +enabled = true +version = 0 +name = "JDBC Oracle Source" +path = "target/release/libiggy_connector_jdbc_source" +plugin_config_format = "toml" + +[plugin_config] +# JDBC connection URL +# Option 1: Separate credentials +jdbc_url = "jdbc:oracle:thin:@localhost:1521:XE" + +# Option 2: Embedded credentials in URL (Oracle uses / separator) +# jdbc_url = "jdbc:oracle:thin:system/oracle@localhost:1521:XE" + +# JDBC driver class name +driver_class = "oracle.jdbc.OracleDriver" + +# Path to JDBC driver JAR file +# Download from: https://www.oracle.com/database/technologies/appdev/jdbc-downloads.html +driver_jar_path = "/opt/jdbc-drivers/ojdbc11.jar" + +# Database credentials (optional if included in jdbc_url) +username = "system" +password = "oracle" + +# SQL query to execute +# Oracle example with ROWNUM or use a numeric/timestamp column +query = "SELECT * FROM CUSTOMERS WHERE ID > {last_offset} ORDER BY ID" + +# How often to poll the database +poll_interval = "1m" + +# Maximum number of rows to fetch per poll +batch_size = 500 + +# Column to track for incremental reads (must be in query result) +tracking_column = "ID" + +# Initial offset value for the first poll +initial_offset = "0" + +# Source mode: "incremental" or "bulk" +# Works with ALL JDBC databases universally +mode = "incremental" + +# Convert column names to snake_case (e.g., OrderDate -> order_date) +snake_case_columns = true + +# Include metadata wrapper in output messages +include_metadata = true + + +# Custom JVM options (optional) +jvm_options = ["-Xmx512m", "-Xms256m"] + +# Target Iggy stream and topic +[[streams]] +stream = "crm" +topic = "customers" +partition_id = 1 +schema = "json" diff --git a/core/connectors/runtime/example_config/connectors/jdbc_sqlserver.toml b/core/connectors/runtime/example_config/connectors/jdbc_sqlserver.toml new file mode 100644 index 0000000000..9e28d08a03 --- /dev/null +++ b/core/connectors/runtime/example_config/connectors/jdbc_sqlserver.toml @@ -0,0 +1,66 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Example JDBC Source Connector Configuration for Microsoft SQL Server + +type = "source" +key = "jdbc_sqlserver_example" +enabled = true +version = 0 +name = "JDBC SQL Server Source" +path = "target/release/libiggy_connector_jdbc_source" +plugin_config_format = "toml" + +[plugin_config] +# JDBC connection URL +# Option 1: Separate credentials +jdbc_url = "jdbc:sqlserver://localhost:1433;databaseName=Sales;encrypt=false" + +# Option 2: Embedded credentials in URL +# jdbc_url = "jdbc:sqlserver://localhost:1433;databaseName=Sales;user=sa;password=YourPassword123;encrypt=false" + +# JDBC driver class name +driver_class = "com.microsoft.sqlserver.jdbc.SQLServerDriver" + +# Path to JDBC driver JAR file +# Download from: https://repo1.maven.org/maven2/com/microsoft/sqlserver/mssql-jdbc/ +driver_jar_path = "/opt/jdbc-drivers/mssql-jdbc-12.4.1.jre11.jar" + +# Database credentials (optional if included in jdbc_url) +username = "sa" +password = "YourPassword123" + +# SQL query to execute +query = "SELECT * FROM Orders WHERE OrderDate > {last_offset} ORDER BY OrderDate" + +poll_interval = "15s" +batch_size = 2000 +tracking_column = "OrderDate" +initial_offset = "2024-01-01" +mode = "incremental" + +# Convert SQL Server naming to snake_case +snake_case_columns = true +include_metadata = true + +connection_timeout_ms = 30000 + +[[streams]] +stream = "sales" +topic = "orders" +partition_id = 1 +schema = "json" diff --git a/core/connectors/runtime/example_config/connectors/test_jdbc_h2.toml b/core/connectors/runtime/example_config/connectors/test_jdbc_h2.toml new file mode 100644 index 0000000000..06b65c21b2 --- /dev/null +++ b/core/connectors/runtime/example_config/connectors/test_jdbc_h2.toml @@ -0,0 +1,45 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +type = "source" +key = "jdbc_h2_test" +enabled = true +version = 0 +name = "JDBC H2 Test Source" +path = "target/release/libiggy_connector_jdbc_source" +plugin_config_format = "toml" + +[plugin_config] +jdbc_url = "jdbc:h2:mem:testdb;DB_CLOSE_DELAY=-1;INIT=CREATE TABLE IF NOT EXISTS users (id INT PRIMARY KEY AUTO_INCREMENT, name VARCHAR(100), email VARCHAR(100), created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP);INSERT INTO users (name, email) SELECT 'User ' || x, 'user' || x || '@test.com' FROM SYSTEM_RANGE(1, 100) WHERE NOT EXISTS (SELECT 1 FROM users);" +driver_class = "org.h2.Driver" +driver_jar_path = "/tmp/jdbc-drivers/h2-2.2.224.jar" +username = "sa" +password = "" +query = "SELECT * FROM users WHERE id > {last_offset} ORDER BY id" +poll_interval = "5s" +batch_size = 10 +tracking_column = "id" +initial_offset = "0" +mode = "incremental" +snake_case_columns = true +include_metadata = true + +[[streams]] +stream = "test" +topic = "users" +partition_id = 1 +schema = "json" diff --git a/core/connectors/sources/README.md b/core/connectors/sources/README.md index 34989aef00..cea774355a 100644 --- a/core/connectors/sources/README.md +++ b/core/connectors/sources/README.md @@ -10,6 +10,7 @@ Source connectors are responsible for ingesting data from external sources into | ------ | ----------- | | **elasticsearch_source** | Polls documents from Elasticsearch indices with timestamp-based tracking | | **influxdb_source** | Polls InfluxDB with cursor-based timestamp tracking; supports V2 (Flux, annotated CSV) and V3 (SQL, JSONL) | +| **jdbc_source** | Reads rows from any JDBC-compliant database (PostgreSQL, MySQL, Oracle, SQL Server, H2) via an embedded JVM; bulk and incremental modes | | **postgres_source** | Reads rows from PostgreSQL tables with multiple strategies: delete after read, mark as processed, or timestamp tracking | | **random_source** | Generates random test messages (useful for testing and development) | diff --git a/core/connectors/sources/jdbc_source/Cargo.toml b/core/connectors/sources/jdbc_source/Cargo.toml new file mode 100644 index 0000000000..c3eabd9e61 --- /dev/null +++ b/core/connectors/sources/jdbc_source/Cargo.toml @@ -0,0 +1,67 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +[package] +name = "iggy_connector_jdbc_source" +version = "0.1.0" +edition = "2024" +license = "Apache-2.0" +keywords = ["iggy", "messaging", "streaming", "jdbc", "source"] +categories = ["database"] +description = "Generic JDBC source connector for Iggy - supports MySQL, Oracle, SQL Server, H2, and any JDBC-compliant database" +readme = "README.md" + +[package.metadata.cargo-machete] +ignored = ["dashmap", "humantime-serde"] + +[lib] +crate-type = ["cdylib", "rlib"] + +[features] +default = [] + +[dependencies] +async-trait = { workspace = true } +base64 = { workspace = true } +chrono = { workspace = true } + +# Required by source_connector! macro +dashmap = { workspace = true } + +# For parsing duration strings +humantime-serde = "1.1" + +# Shared serde helpers (SecretString serialization) +iggy_common = { workspace = true } + +# Connector SDK +iggy_connector_sdk = { workspace = true } + +# JNI for Java interop with invocation support +jni = { version = "0.21", features = ["invocation"] } + +# For sanitizing passwords in logs +regex = { workspace = true } +secrecy = { workspace = true } +serde = { workspace = true, features = ["derive"] } +serde_json = { workspace = true } +tokio = { workspace = true, features = ["full"] } +tracing = { workspace = true } +uuid = { workspace = true, features = ["v4"] } + +[dev-dependencies] +toml = { workspace = true } diff --git a/core/connectors/sources/jdbc_source/README.md b/core/connectors/sources/jdbc_source/README.md new file mode 100644 index 0000000000..c2e4d979b4 --- /dev/null +++ b/core/connectors/sources/jdbc_source/README.md @@ -0,0 +1,539 @@ +# JDBC Source Connector + +A generic JDBC source connector for Iggy that supports any JDBC-compliant database including MySQL, PostgreSQL, Oracle, SQL Server, H2, Derby, and more. + +## Overview + +This connector reads data from relational databases using JDBC (Java Database Connectivity) and publishes it as messages to Iggy streams. It supports both bulk and incremental data synchronization modes. + +## Features + +- **Universal Database Support**: Works with any database that has a JDBC driver +- **Incremental Sync**: Track changes using timestamps or auto-increment IDs +- **Bulk Mode**: Re-runs the query each poll for snapshots (capped at `batch_size` rows; see limitations) +- **Type Mapping**: Automatic conversion of SQL types to JSON +- **Configurable Polling**: Control how frequently data is fetched +- **State Management**: Automatically tracks offsets to prevent duplicate reads +- **Flexible Queries**: Support for custom SQL queries with placeholders + +## Supported Databases + +**ALL JDBC-compliant databases are supported for both bulk and incremental modes:** + +- MySQL / MariaDB +- PostgreSQL +- Oracle Database +- Microsoft SQL Server +- H2 Database +- Apache Derby +- IBM DB2 +- SQLite (via JDBC) +- SAP HANA +- Teradata +- Snowflake +- Amazon Redshift +- Google BigQuery +- Any other JDBC-compliant database + +**Key Point:** The JDBC connector provides a **single, universal implementation** that works with all these databases. You don't need separate connectors for MySQL, Oracle, etc. Just swap the JDBC driver JAR and connection string! + +## Prerequisites + +1. **Java Runtime Environment (JRE)**: JRE 8 or later must be installed +2. **JDBC Driver**: Download the appropriate JDBC driver JAR for your database + +### Downloading JDBC Drivers + +**MySQL:** + +```bash +wget https://repo1.maven.org/maven2/com/mysql/mysql-connector-j/8.0.33/mysql-connector-j-8.0.33.jar +``` + +**PostgreSQL:** + +```bash +wget https://jdbc.postgresql.org/download/postgresql-42.6.0.jar +``` + +**Oracle:** + +- Download from [Oracle JDBC Driver Downloads](https://www.oracle.com/database/technologies/appdev/jdbc-downloads.html) + +**SQL Server:** + +```bash +wget https://repo1.maven.org/maven2/com/microsoft/sqlserver/mssql-jdbc/12.4.1.jre11/mssql-jdbc-12.4.1.jre11.jar +``` + +**H2:** + +```bash +wget https://repo1.maven.org/maven2/com/h2database/h2/2.2.224/h2-2.2.224.jar +``` + +## Configuration + +### Basic Configuration (Incremental Sync) + +```toml +type = "source" +key = "jdbc_mysql_source" +enabled = true + +[plugin_config] +jdbc_url = "jdbc:mysql://localhost:3306/ecommerce" +driver_class = "com.mysql.cj.jdbc.Driver" +driver_jar_path = "/opt/jdbc-drivers/mysql-connector-j-8.0.33.jar" +username = "iggy_user" +password = "secret_password" +query = "SELECT * FROM orders WHERE updated_at > {last_offset} ORDER BY updated_at ASC" +poll_interval = "30s" +batch_size = 1000 +tracking_column = "updated_at" +initial_offset = "2024-01-01 00:00:00" +mode = "incremental" +snake_case_columns = true +include_metadata = true + +[[streams]] +stream = "ecommerce" +topic = "orders" +partition_id = 1 +``` + +### Bulk Mode Configuration + +```toml +type = "source" +key = "jdbc_bulk_source" +enabled = true + +[plugin_config] +jdbc_url = "jdbc:postgresql://localhost:5432/warehouse" +driver_class = "org.postgresql.Driver" +driver_jar_path = "/opt/jdbc-drivers/postgresql-42.6.0.jar" +username = "warehouse_user" +password = "secret" +query = "SELECT * FROM product_catalog" +poll_interval = "1h" +batch_size = 5000 +mode = "bulk" +snake_case_columns = false +include_metadata = true + +[[streams]] +stream = "warehouse" +topic = "products" +``` + +### Oracle Database Example + +```toml +type = "source" +key = "jdbc_oracle_source" +enabled = true + +[plugin_config] +jdbc_url = "jdbc:oracle:thin:@localhost:1521:XE" +driver_class = "oracle.jdbc.OracleDriver" +driver_jar_path = "/opt/jdbc-drivers/ojdbc11.jar" +username = "system" +password = "oracle" +query = "SELECT * FROM CUSTOMERS WHERE ID > {last_offset} ORDER BY ID" +poll_interval = "1m" +batch_size = 500 +tracking_column = "ID" +initial_offset = "0" +mode = "incremental" +jvm_options = ["-Xmx256m", "-Xms128m"] + +[[streams]] +stream = "crm" +topic = "customers" +``` + +### SQL Server Example + +```toml +type = "source" +key = "jdbc_sqlserver_source" +enabled = true + +[plugin_config] +jdbc_url = "jdbc:sqlserver://localhost:1433;databaseName=Sales;encrypt=false" +driver_class = "com.microsoft.sqlserver.jdbc.SQLServerDriver" +driver_jar_path = "/opt/jdbc-drivers/mssql-jdbc-12.4.1.jre11.jar" +username = "sa" +password = "YourPassword123" +query = "SELECT * FROM Orders WHERE OrderDate > {last_offset} ORDER BY OrderDate" +poll_interval = "15s" +batch_size = 2000 +tracking_column = "OrderDate" +initial_offset = "2024-01-01" +mode = "incremental" + +[[streams]] +stream = "sales" +topic = "orders" +``` + +## Configuration Parameters + +| Parameter | Type | Required | Default | Description | +| ----------- | ------ | ---------- | --------- | ------------- | +| `jdbc_url` | string | Yes | - | JDBC connection URL (can include credentials) | +| `driver_class` | string | Yes | - | JDBC driver class name | +| `driver_jar_path` | string | Yes | - | Path to the JDBC driver JAR (checked to exist at startup; passed to the embedded JVM as `-Djava.class.path`) | +| `username` | string | No | - | Database username (optional if in jdbc_url) | +| `password` | string | No | - | Database password (optional if in jdbc_url) | +| `query` | string | Yes | - | SQL query to execute (supports `{last_offset}` and `{tracking_column}` placeholders) | +| `poll_interval` | duration | Yes | - | How often to poll (e.g., "30s", "5m", "1h") | +| `batch_size` | u32 | No | 1000 | Maximum rows to fetch per poll | +| `tracking_column` | string | Incremental | - | Column to track for incremental reads (required in incremental mode; the query must also `ORDER BY` it) | +| `initial_offset` | string | No | - | Starting offset value for first poll | +| `mode` | string | No | "incremental" | Sync mode: "incremental" or "bulk" (bulk works with ALL databases) | +| `connection_timeout_ms` | u64 | No | 30000 | Timeout (ms) for the per-poll connection liveness check | +| `jvm_options` | array | No | [] | Custom JVM options (e.g., ["-Xmx1g"]) | +| `snake_case_columns` | bool | No | false | Convert column names to snake_case | +| `include_metadata` | bool | No | true | Include metadata (table, operation, timestamp) | + +## Query Placeholders + +The `query` parameter supports placeholders for dynamic queries: + +- `{last_offset}`: Replaced with the last tracked offset value, wrapped in quotes and escaped +- `{tracking_column}`: Replaced with the configured `tracking_column` (validated as a plain SQL identifier) + +Incremental mode is validated at `open()` and enforces the following (the +connector refuses to start otherwise): + +- **`tracking_column` is required.** Without it the offset can never advance and + every poll re-reads the same rows. +- **The query must order by the tracking column, ascending, as the first + `ORDER BY` term.** Row limiting uses `setMaxRows`, so an unordered (or + otherwise-ordered) query returns an arbitrary subset; advancing the offset to + that subset's max would permanently skip the unread lower keys. The validator + therefore requires the tracking column to be the **first** ordering term of the + outer `ORDER BY` and rejects a descending (`DESC`) direction. Write either + `ORDER BY {tracking_column}` or the column name (optionally table-qualified, + e.g. `ORDER BY t.updated_at`). A composite order such as `ORDER BY other, id` + (tracking column not first) or a `DESC` order is rejected at `open()`. + +The tracking column must also be: + +- **Homogeneously typed and monotonic.** Offsets are compared as integers, then + floats, then lexicographically; a column mixing numeric-looking and + non-numeric strings can make the connector's comparison disagree with the + database's `>` and skip or re-read rows. Prefer an auto-increment ID or a + timestamp. +- **`NOT NULL`.** A NULL tracking value cannot be watermarked, so the connector + errors the poll if it reads one and keeps erroring (making no progress) until + the query is fixed. Exclude NULLs in the query, e.g. `AND {tracking_column} IS + NOT NULL`. +- **Lexicographically ordered, for timestamps.** Timestamp columns are read via + the driver's string form; ensure that form is ordered (ISO-8601 is). A locale + format such as `MM/DD/YYYY` is not monotonic as text and will misorder. + +**Example:** + +```sql +-- Configuration +tracking_column = "id" +query = "SELECT * FROM users WHERE id > {last_offset} ORDER BY id" + +-- First poll (no offset yet) +SELECT * FROM users WHERE id > '0' ORDER BY id + +-- After processing rows up to id=100 +SELECT * FROM users WHERE id > '100' ORDER BY id +``` + +## Output Format + +Each database row is converted to a JSON message: + +### With Metadata (default) + +```json +{ + "table_name": null, + "operation_type": "SELECT", + "timestamp": "2024-01-09T10:30:00Z", + "data": { + "id": 123, + "name": "John Doe", + "email": "john@example.com", + "created_at": "2024-01-08T15:20:00" + } +} +``` + +### Without Metadata + +```json +{ + "id": 123, + "name": "John Doe", + "email": "john@example.com", + "created_at": "2024-01-08T15:20:00" +} +``` + +## Type Mapping + +JDBC SQL types are automatically mapped to JSON: + +| SQL Type | JSON Type | Notes | +| ---------- | ----------- | ------- | +| BIT, BOOLEAN | boolean | - | +| TINYINT, SMALLINT, INTEGER | number | Integer | +| BIGINT | number | Long integer (values above 2^53 may lose precision in JSON consumers that parse numbers as f64) | +| FLOAT, REAL | number | Float | +| DOUBLE | number | Double | +| NUMERIC, DECIMAL | string | Emitted as a string to preserve arbitrary precision (e.g. money) | +| CHAR, VARCHAR, TEXT | string | - | +| DATE, TIME, TIMESTAMP | string | Driver string form | +| BINARY, VARBINARY, LONGVARBINARY | string | Base64 encoded | +| NULL | null | - | + +## Runtime notes & limitations + +- **Embedded JVM, one per process.** JNI permits a single `JavaVM` per OS + process. All JDBC *source* instances in the connectors runtime share one JVM + (the first instance's `jvm_options`/classpath win). A JDBC source and a JDBC + sink are separate shared libraries and **cannot both create a JVM in the same + runtime process** — run them in separate connectors-runtime processes. +- **Blocking I/O.** JDBC calls go through JNI and are synchronous. The fetch in + `poll()` (and the close in `close()`) runs under `tokio::task::block_in_place` + so it does not monopolize a shared async-runtime worker, but the work is still + blocking; size the runtime and `poll_interval`/`batch_size` accordingly. +- **Bulk mode has no pagination beyond `batch_size`, and fails closed.** Row + limiting uses JDBC `setMaxRows`. In bulk mode the connector probes with + `batch_size + 1` rows and, if the result set is larger than `batch_size`, + **errors the poll instead of syncing a truncated subset**. For tables larger + than a batch, raise `batch_size` to cover the full result, or use incremental + mode with an ordered `tracking_column`. (Full cross-database OFFSET pagination + is a planned follow-up.) +- **Delivery semantics.** State (the incremental offset) is persisted by the + runtime only after a batch is successfully sent, so offsets are **at-least-once + across restarts**: a crash or restart never skips rows. One narrower gap + remains: a *transient in-process send failure without a restart* can skip the + batch it happened on, because the in-memory offset is not rolled back. This + matches the other offset-tracking source connectors and is a runtime-level + limitation (there is no per-poll delivery ack to the connector); a stronger + guarantee is tracked as a follow-up. +- **Connection recovery.** The connection is validated with `Connection.isValid` + each poll and transparently re-established (closing the old handle) if it has + dropped. +- **`SQLState` classification is informational today.** Query failures are + classified into transient vs permanent error variants, but the runtime does + not yet apply differentiated backoff based on that distinction; it currently + shapes the error variant and the log message only. +- **Credentials reach the JVM heap unzeroed.** `password` is held as a + `SecretString` on the Rust side, but the JDBC API takes a `java.lang.String`, + so the password is copied onto the JVM heap as an ordinary (non-zeroed) string + for the lifetime of the connection. This is inherent to the JDBC surface and + is an accepted risk. + +### Credential precedence + +Provide credentials **either** via `username` + `password` **or** embedded in the +`jdbc_url`, not both: + +- `username` and `password` must be **both set** (separate-credential auth) or + **both unset** (URL-embedded credentials). A half-set pair is rejected at + `open()`. +- When both `username`/`password` and URL-embedded credentials are present, the + driver decides precedence (typically the explicit `getConnection(url, user, + pass)` arguments win). Avoid the ambiguity by using only one method. + +## Troubleshooting + +### Connection Failures + +**Error**: "Failed to create JDBC connection" + +**Solution**: + +- Verify JDBC URL format for your database +- Check username/password +- Ensure database server is accessible +- Verify firewall rules + +### Driver Not Found + +**Error**: "Failed to find driver class" + +**Solution**: + +- Verify `driver_jar_path` points to correct JAR file +- Check `driver_class` name matches your JDBC driver +- Ensure JAR file has read permissions + +### JVM Issues + +**Error**: "Failed to create JVM" + +**Solution**: + +- Ensure Java is installed: `java -version` +- Increase JVM memory: + + ```toml + jvm_options = ["-Xmx1g", "-Xms512m"] + ``` + +### No Data Being Fetched + +**Check**: + +- Verify query returns results when run directly in database +- Check `initial_offset` value +- Review connector logs for errors +- Ensure `tracking_column` exists in query result + +## Performance Tuning + +### Optimize Batch Size + +```toml +# Small batches for low latency +batch_size = 100 +poll_interval = "5s" + +# Large batches for throughput +batch_size = 10000 +poll_interval = "1m" +``` + +### JVM Memory Tuning + +```toml +jvm_options = [ + "-Xmx1g", # Maximum heap size + "-Xms512m", # Initial heap size + "-XX:+UseG1GC" # Use G1 garbage collector +] +``` + +### Query Optimization + +- Add indexes on tracking columns +- Use efficient WHERE clauses +- Avoid SELECT * in production (specify columns) +- Consider database-specific optimizations + +## Connection String Formats + +### MySQL + +```toml +# Option 1: Separate credentials +jdbc_url = "jdbc:mysql://localhost:3306/mydb" +username = "user" +password = "pass" + +# Option 2: Embedded in URL +jdbc_url = "jdbc:mysql://user:pass@localhost:3306/mydb" +``` + +### PostgreSQL + +```toml +# Option 1: Separate credentials +jdbc_url = "jdbc:postgresql://localhost:5432/mydb" +username = "user" +password = "pass" + +# Option 2: Embedded in URL +jdbc_url = "jdbc:postgresql://localhost:5432/mydb?user=myuser&password=mypass" +``` + +### Oracle + +```toml +# Option 1: Separate credentials +jdbc_url = "jdbc:oracle:thin:@localhost:1521:XE" +username = "system" +password = "oracle" + +# Option 2: Embedded in URL (Oracle uses @ for host) +jdbc_url = "jdbc:oracle:thin:system/oracle@localhost:1521:XE" +``` + +### SQL Server + +```toml +# Option 1: Separate credentials +jdbc_url = "jdbc:sqlserver://localhost:1433;databaseName=mydb" +username = "sa" +password = "YourPassword123" + +# Option 2: Embedded in URL +jdbc_url = "jdbc:sqlserver://localhost:1433;databaseName=mydb;user=sa;password=YourPassword123" +``` + +### H2 (In-Memory) + +```toml +# No credentials needed for in-memory +jdbc_url = "jdbc:h2:mem:testdb" + +# Or with file-based +jdbc_url = "jdbc:h2:file:/data/mydb;USER=sa;PASSWORD=sa" +``` + +## Mode Comparison + +### Incremental Mode (Universal) + +**Works with ALL databases** - requires only a tracking column: + +```toml +mode = "incremental" +tracking_column = "updated_at" # or "id", "created_at", etc. +query = "SELECT * FROM table WHERE {tracking_column} > {last_offset} ORDER BY {tracking_column}" +``` + +**Benefits:** + +- Prevents duplicate reads +- Tracks offset automatically +- Efficient for large tables +- Works with timestamps, IDs, or any orderable column + +**Database Examples** (the query must order by the tracking column): + +- MySQL: `WHERE updated_at > {last_offset} ORDER BY updated_at` +- Oracle: `WHERE id > {last_offset} ORDER BY id` (use a monotonic key; `ROWNUM` is not a valid tracking column) +- SQL Server: `WHERE updated_at > {last_offset} ORDER BY updated_at` +- PostgreSQL: `WHERE id > {last_offset} ORDER BY id` + +### Bulk Mode (Universal) + +**Works with ALL databases** - no special requirements: + +```toml +mode = "bulk" +query = "SELECT * FROM table" # Any valid SELECT query +``` + +**Benefits:** + +- No tracking column needed +- Works with any SELECT query +- Good for snapshots +- Supports complex queries with JOINs, aggregations, etc. + +**Limitation:** the result set is capped at `batch_size` rows (`setMaxRows`) with +no pagination beyond it. Rather than sync a truncated subset, bulk mode **fails +closed**: if the result set is larger than `batch_size` the poll errors. Raise +`batch_size` to cover the full table, or use incremental mode, for large tables. + +**Use Cases:** + +- Initial data load +- Periodic full snapshots (that fit within `batch_size`) +- Complex analytical queries +- Tables without tracking columns diff --git a/core/connectors/sources/jdbc_source/config.toml b/core/connectors/sources/jdbc_source/config.toml new file mode 100644 index 0000000000..8cb3f06441 --- /dev/null +++ b/core/connectors/sources/jdbc_source/config.toml @@ -0,0 +1,50 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +type = "source" +key = "jdbc" +enabled = true +version = 0 +name = "JDBC source" +path = "../../target/release/libiggy_connector_jdbc_source" +verbose = false + +[[streams]] +stream = "user_events" +topic = "users" +schema = "json" +batch_length = 100 + +[plugin_config] +jdbc_url = "jdbc:postgresql://localhost:5432/database" +driver_class = "org.postgresql.Driver" +driver_jar_path = "/tmp/jdbc-drivers/postgresql-42.7.1.jar" +# Credential precedence: provide credentials EITHER via username + password here +# OR embedded in jdbc_url, not both. username and password must be both set or +# both unset (a half-set pair is rejected at startup). If both this pair and +# URL-embedded credentials are present, the driver decides which wins; avoid the +# ambiguity by using only one method. +username = "postgres" +password = "postgres" +query = "SELECT * FROM users WHERE id > {last_offset} ORDER BY id" +poll_interval = "1s" +batch_size = 1000 +tracking_column = "id" +initial_offset = "0" +mode = "incremental" +snake_case_columns = false +include_metadata = true diff --git a/core/connectors/sources/jdbc_source/src/lib.rs b/core/connectors/sources/jdbc_source/src/lib.rs new file mode 100644 index 0000000000..ea19ab0916 --- /dev/null +++ b/core/connectors/sources/jdbc_source/src/lib.rs @@ -0,0 +1,2843 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use async_trait::async_trait; +use chrono::{DateTime, Utc}; +use iggy_common::serde_secret; +use iggy_connector_sdk::{ + ConnectorState, Error, ProducedMessage, ProducedMessages, Schema, Source, source_connector, +}; +use jni::objects::{GlobalRef, JByteArray, JObject, JString, JThrowable, JValue}; +use jni::{JNIEnv, JavaVM}; +use regex::Regex; +use secrecy::{ExposeSecret, SecretString}; +use serde::{Deserialize, Serialize}; +use std::sync::{Arc, Mutex, MutexGuard}; +use std::time::{Duration, Instant}; +use tracing::{debug, info, warn}; +use uuid::Uuid; + +/// Clear any pending Java exception on the current thread. The JNI spec forbids +/// making most calls while an exception is pending; doing so aborts the whole +/// embedded JVM (the entire connectors-runtime process). Every fallible JNI +/// call in this connector clears on error before returning so a thrown Java +/// exception is never left pending for the next call on this thread. No-op when +/// nothing is pending. +fn clear_pending_exception(env: &mut JNIEnv) { + let _ = env.exception_clear(); +} + +/// Best-effort `close()` on a JDBC handle used in error/cleanup paths. Clears +/// any pending exception first (`close()` is a `CallVoidMethod`, which JNI +/// forbids while an exception is pending) and again afterwards in case the +/// close itself throws. +fn best_effort_close(env: &mut JNIEnv, handle: &JObject) { + clear_pending_exception(env); + let _ = env.call_method(handle, "close", "()V", &[]); + clear_pending_exception(env); +} + +/// Evaluate a fallible JNI call; on error clear any pending Java exception and +/// return `Error::Connection` with context. Keeps a thrown Java exception from +/// being left pending for the next JNI call on this thread. +macro_rules! jni { + ($env:expr, $call:expr, $ctx:expr) => { + match $call { + Ok(value) => value, + Err(err) => { + clear_pending_exception(&mut *$env); + return Err(Error::Connection(format!("{}: {err}", $ctx))); + } + } + }; +} + +/// Like [`jni!`] but returns `Error::InitError`, for the connection-setup path. +macro_rules! jni_init { + ($env:expr, $call:expr, $ctx:expr) => { + match $call { + Ok(value) => value, + Err(err) => { + clear_pending_exception(&mut *$env); + return Err(Error::InitError(format!("{}: {err}", $ctx))); + } + } + }; +} + +/// Lock a mutex, mapping a poisoned lock to a returned `Error` instead of +/// panicking. A thread that panicked while holding the lock then surfaces as a +/// logged, recoverable failure rather than a permanent panic loop on every +/// subsequent poll. +fn lock_mutex<'a, T>(mutex: &'a Mutex, what: &str) -> Result, Error> { + mutex + .lock() + .map_err(|_| Error::Connection(format!("{what} mutex poisoned"))) +} + +/// Cached compiled regex patterns for password sanitization +static RE_USER_PASS_AT: std::sync::LazyLock = + std::sync::LazyLock::new(|| Regex::new(r"://([^:]+):([^@?;/]+)@").unwrap()); +static RE_PASSWORD_PARAM: std::sync::LazyLock = + std::sync::LazyLock::new(|| Regex::new(r"(?i)(password|pwd|pass)=([^;&\s]+)").unwrap()); +static RE_ORACLE_PASS: std::sync::LazyLock = + std::sync::LazyLock::new(|| Regex::new(r"thin:([^/]+)/([^@]+)@").unwrap()); + +/// Matches the canonical incremental predicate so it can be removed on the first +/// (no-offset) poll. Case- and whitespace-tolerant so `where {tracking_column}>{last_offset}` +/// and `WHERE {tracking_column} > {last_offset}` are all recognized, rather +/// than only one exact literal spelling. +static RE_INCREMENTAL_PREDICATE: std::sync::LazyLock = std::sync::LazyLock::new(|| { + Regex::new(r"(?i)\bWHERE\s+\{tracking_column\}\s*>\s*\{last_offset\}").unwrap() +}); + +const CONNECTOR_NAME: &str = "JDBC source"; + +/// Source mode for the JDBC connector +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq)] +#[serde(rename_all = "lowercase")] +pub enum Mode { + /// Re-run the query on every poll, capped at `batch_size` rows via JDBC + /// `setMaxRows`. There is no pagination beyond that cap, so rather than sync a + /// truncated subset, bulk mode fails closed: the poll probes with + /// `batch_size + 1` rows and errors if the result set is larger than + /// `batch_size`. Raise `batch_size` to cover the full table, or use + /// incremental mode, for tables larger than a batch. + Bulk, + /// Track the last offset and fetch only rows beyond it on each poll. + Incremental, +} + +/// Configuration for JDBC source connector +#[derive(Clone, Deserialize, Serialize)] +pub struct JdbcSourceConfig { + /// JDBC connection URL (e.g., "jdbc:mysql://localhost:3306/mydb") + /// Can include credentials: "jdbc:mysql://localhost:3306/mydb?user=root&password=secret" + #[serde(serialize_with = "serde_secret::serialize_secret")] + pub jdbc_url: SecretString, + + /// JDBC driver class name (e.g., "com.mysql.cj.jdbc.Driver") + pub driver_class: String, + + /// Path to JDBC driver JAR file + pub driver_jar_path: String, + + /// Database username (optional if included in jdbc_url) + #[serde(default)] + pub username: Option, + + /// Database password (optional if included in jdbc_url) + #[serde(default, serialize_with = "serde_secret::serialize_optional_secret")] + pub password: Option, + + /// SQL query to execute for fetching data + /// Can use {last_offset} placeholder for incremental reads + pub query: String, + + /// Polling interval (e.g., "30s", "5m", "1h") + #[serde(with = "humantime_serde")] + pub poll_interval: Duration, + + /// Batch size - maximum rows to fetch per poll + #[serde(default = "default_batch_size")] + pub batch_size: u32, + + /// Tracking column for incremental reads (e.g., "id", "updated_at") + #[serde(default)] + pub tracking_column: Option, + + /// Initial offset value for the first poll + #[serde(default)] + pub initial_offset: Option, + + /// Source mode: "bulk" (full table scan) or "incremental" (track last offset) + #[serde(default = "default_mode")] + pub mode: Mode, + + /// Convert column names to snake_case + #[serde(default)] + pub snake_case_columns: bool, + + /// Include metadata in output (table name, operation type, timestamp) + #[serde(default = "default_true")] + pub include_metadata: bool, + + /// JVM options (e.g., ["-Xmx512m", "-Xms128m"]) + #[serde(default)] + pub jvm_options: Vec, + + /// Timeout for the per-poll `Connection.isValid` liveness check (default: + /// 30000). JDBC expresses this timeout in whole seconds, so the value is + /// converted to seconds and clamped to the 1..=30s range; it does not govern + /// connection establishment. + #[serde(default = "default_connection_timeout")] + pub connection_timeout_ms: u64, +} + +fn default_connection_timeout() -> u64 { + 30000 +} + +fn default_batch_size() -> u32 { + 1000 +} + +fn default_mode() -> Mode { + Mode::Incremental +} + +fn default_true() -> bool { + true +} + +impl std::fmt::Debug for JdbcSourceConfig { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("JdbcSourceConfig") + .field( + "jdbc_url", + &sanitize_jdbc_url(self.jdbc_url.expose_secret()), + ) + .field("driver_class", &self.driver_class) + .field("driver_jar_path", &self.driver_jar_path) + .field("username", &self.username) + .field("password", &self.password.as_ref().map(|_| "***")) + .field("query", &self.query) + .field("poll_interval", &self.poll_interval) + .field("batch_size", &self.batch_size) + .field("tracking_column", &self.tracking_column) + .field("initial_offset", &self.initial_offset) + .field("mode", &self.mode) + .field("snake_case_columns", &self.snake_case_columns) + .field("include_metadata", &self.include_metadata) + .field("connection_timeout_ms", &self.connection_timeout_ms) + .finish() + } +} + +/// Internal state tracking for the JDBC source +#[derive(Debug, Clone, Serialize, Deserialize)] +struct State { + /// Last tracked offset value (for incremental mode) + last_offset: Option, + + /// Total rows processed + processed_rows: u64, + + /// Last poll timestamp + last_poll_time: DateTime, +} + +impl Default for State { + fn default() -> Self { + Self { + last_offset: None, + processed_rows: 0, + last_poll_time: Utc::now(), + } + } +} + +/// Database record structure for output messages +#[derive(Debug, Serialize, Deserialize)] +pub struct DatabaseRecord { + pub table_name: Option, + pub operation_type: String, + pub timestamp: DateTime, + pub data: serde_json::Value, +} + +/// JDBC Source Connector +#[derive(Debug)] +pub struct JdbcSource { + id: u32, + config: JdbcSourceConfig, + jvm: Option>, + // Behind a Mutex so `poll()` (&self) can transparently re-establish a dead + // direct connection without `&mut self`. + connection: Mutex>, + state: Arc>, + // Scheduled start of the next poll, used to pace polls at a fixed cadence + // that does not drift with per-poll work time. `None` until the first poll. + next_poll_at: Mutex>, +} + +/// Sanitize JDBC URL by masking passwords for logging +fn sanitize_jdbc_url(url: &str) -> String { + // Pattern 1: user:password@host format (MySQL, PostgreSQL) + let url = RE_USER_PASS_AT.replace_all(url, "://$1:***@"); + + // Pattern 2: password=value format (PostgreSQL, SQL Server, H2) + let url = RE_PASSWORD_PARAM.replace_all(&url, "$1=***"); + + // Pattern 3: Oracle user/password@host format + let url = RE_ORACLE_PASS.replace_all(&url, "thin:$1/***@"); + + url.to_string() +} + +impl JdbcSource { + /// Create a new JDBC source connector + pub fn new(id: u32, config: JdbcSourceConfig, connector_state: Option) -> Self { + // Restore state from persistent storage if available + let state = connector_state + .and_then(|cs| cs.deserialize::(CONNECTOR_NAME, id)) + .unwrap_or_else(|| { + let mut default_state = State::default(); + // Use initial_offset from config if provided + if let Some(ref initial_offset) = config.initial_offset { + default_state.last_offset = Some(initial_offset.clone()); + } + default_state + }); + + Self { + id, + config, + jvm: None, + connection: Mutex::new(None), + state: Arc::new(Mutex::new(state)), + next_poll_at: Mutex::new(None), + } + } + + /// Obtain the process-wide JVM, creating it on first use. JNI permits only a + /// single JVM per OS process, so this is shared across all JDBC connector + /// instances (see [`get_or_create_jvm`]). + fn initialize_jvm(&mut self) -> Result<(), Error> { + info!("Initializing JVM for JDBC source connector [{}]", self.id); + let jvm = get_or_create_jvm(&self.config.driver_jar_path, &self.config.jvm_options)?; + self.jvm = Some(jvm); + Ok(()) + } + + /// Load the JDBC driver and open the database connection. + fn create_connection(&mut self) -> Result<(), Error> { + let jvm = self + .jvm + .as_ref() + .ok_or_else(|| Error::InitError("JVM not initialized".to_string()))?; + + let mut env = jvm + .attach_current_thread() + .map_err(|e| Error::InitError(format!("Failed to attach thread to JVM: {}", e)))?; + + info!("Loading JDBC driver: {}", self.config.driver_class); + + // Load driver using Class.forName() which triggers static initialization + info!( + "Loading driver class via Class.forName: {}", + self.config.driver_class + ); + + let class_class = jni_init!( + env, + env.find_class("java/lang/Class"), + "Failed to find Class" + ); + + let driver_class_name = jni_init!( + env, + env.new_string(&self.config.driver_class), + "Failed to create class name string" + ); + + // Call Class.forName(className) to load and initialize the driver + jni_init!( + env, + env.call_static_method( + class_class, + "forName", + "(Ljava/lang/String;)Ljava/lang/Class;", + &[JValue::Object(&driver_class_name.into())], + ), + format!("Failed to load driver class '{}'", self.config.driver_class) + ); + + info!("JDBC driver loaded and registered successfully"); + + info!( + "Creating direct JDBC connection to: {}", + sanitize_jdbc_url(self.config.jdbc_url.expose_secret()) + ); + let conn = self.create_direct_connection_internal(&mut env)?; + *lock_mutex(&self.connection, "connection")? = Some(conn); + + Ok(()) + } + + /// Create a direct JDBC connection via DriverManager, inside its own JNI + /// local-reference frame so the transient class/loader/URL locals do not + /// accumulate on the caller's frame. The returned handle is a `GlobalRef`, so + /// it survives the frame pop. + fn create_direct_connection_internal(&self, env: &mut JNIEnv) -> Result { + env.push_local_frame(16) + .map_err(|e| Error::InitError(format!("Failed to push local frame: {e}")))?; + let result = self.create_direct_connection_inner(env); + // SAFETY: `result` holds only a global reference (or an error); no JNI + // local reference escapes the frame. + let _ = unsafe { env.pop_local_frame(&JObject::null()) }; + result + } + + fn create_direct_connection_inner(&self, env: &mut JNIEnv) -> Result { + // Set the thread context class loader to help DriverManager find the driver + let current_thread_class = jni_init!( + env, + env.find_class("java/lang/Thread"), + "Failed to find Thread class" + ); + + let current_thread = jni_init!( + env, + env.call_static_method( + current_thread_class, + "currentThread", + "()Ljava/lang/Thread;", + &[], + ) + .and_then(|v| v.l()), + "Failed to get current thread" + ); + + // Get the class loader that loaded the driver + let driver_class = jni_init!( + env, + env.find_class(self.config.driver_class.replace('.', "/")), + "Failed to find driver class" + ); + + let driver_class_loader = jni_init!( + env, + env.call_method( + &driver_class, + "getClassLoader", + "()Ljava/lang/ClassLoader;", + &[], + ) + .and_then(|v| v.l()), + "Failed to get driver class loader" + ); + + // Set the context class loader + jni_init!( + env, + env.call_method( + ¤t_thread, + "setContextClassLoader", + "(Ljava/lang/ClassLoader;)V", + &[JValue::Object(&driver_class_loader)], + ), + "Failed to set context class loader" + ); + + info!( + "Set thread context class loader for driver: {}", + self.config.driver_class + ); + + // Get connection from DriverManager + let driver_manager = jni_init!( + env, + env.find_class("java/sql/DriverManager"), + "Failed to find DriverManager" + ); + + let jdbc_url = jni_init!( + env, + env.new_string(self.config.jdbc_url.expose_secret()), + "Failed to create JDBC URL string" + ); + + // If username/password are provided separately, use 3-arg getConnection + let connection_obj = if let (Some(username), Some(password)) = + (&self.config.username, &self.config.password) + { + info!("Using separate username/password authentication"); + let username_jstring = jni_init!( + env, + env.new_string(username), + "Failed to create username string" + ); + let password_jstring = jni_init!( + env, + env.new_string(password.expose_secret()), + "Failed to create password string" + ); + + jni_init!( + env, + env.call_static_method( + driver_manager, + "getConnection", + "(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)Ljava/sql/Connection;", + &[ + JValue::Object(&jdbc_url.into()), + JValue::Object(&username_jstring.into()), + JValue::Object(&password_jstring.into()), + ], + ) + .and_then(|v| v.l()), + "Failed to create JDBC connection with credentials" + ) + } else { + info!("Using connection string with embedded credentials"); + jni_init!( + env, + env.call_static_method( + driver_manager, + "getConnection", + "(Ljava/lang/String;)Ljava/sql/Connection;", + &[JValue::Object(&jdbc_url.into())], + ) + .and_then(|v| v.l()), + "Failed to create JDBC connection from URL" + ) + }; + + let global_ref = env + .new_global_ref(connection_obj) + .map_err(|e| Error::InitError(format!("Failed to create global reference: {e}")))?; + + info!("Direct database connection established successfully"); + Ok(global_ref) + } + + /// Acquire the direct connection, transparently re-establishing it if it has + /// dropped since the last poll. + fn get_connection<'local>(&self, env: &mut JNIEnv<'local>) -> Result, Error> { + // Hold the connection lock across the whole check/close/create/store + // sequence so correctness does not depend on there being a single caller: + // a concurrent caller can no longer observe or replace the handle + // mid-reconnect. The lock is a std Mutex held only across synchronous JNI + // work (no .await), and neither `connection_is_valid` nor + // `create_direct_connection_internal` re-locks it, so this cannot deadlock. + let mut guard = lock_mutex(&self.connection, "connection")?; + + let needs_reconnect = match guard.as_ref() { + Some(conn) => !self.connection_is_valid(env, conn.as_obj()), + None => true, + }; + + if needs_reconnect { + info!("Direct JDBC connection is not valid; re-establishing"); + // Best-effort close of the old handle, then drop it before creating + // the replacement so a failed reconnect leaves no stale reference. + if let Some(old) = guard.as_ref() { + best_effort_close(env, old.as_obj()); + } + *guard = None; + *guard = Some(self.create_direct_connection_internal(env)?); + } + + let conn = guard + .as_ref() + .ok_or_else(|| Error::Connection("No connection available".to_string()))?; + let local_ref = env + .new_local_ref(conn.as_obj()) + .map_err(|e| Error::Connection(format!("Failed to create local ref: {e}")))?; + Ok(local_ref) + } + + /// Best-effort `Connection.isValid(timeout)` check. Returns false on any + /// JNI error so the caller re-establishes the connection. + fn connection_is_valid(&self, env: &mut JNIEnv, conn: &JObject) -> bool { + let timeout_secs = (self.config.connection_timeout_ms / 1000).clamp(1, 30) as i32; + match env + .call_method(conn, "isValid", "(I)Z", &[JValue::Int(timeout_secs)]) + .and_then(|v| v.z()) + { + Ok(valid) => valid, + Err(_) => { + // isValid may throw; clear so the reconnect path's next JNI call + // is not made with an exception pending. + clear_pending_exception(env); + false + } + } + } + + /// Execute query and fetch results. + /// + /// The mutex is held only briefly: once to read the current offset for + /// query building, and once after the JNI work to write the updated state. + fn execute_query(&self, env: &mut JNIEnv) -> Result, Error> { + let connection = self.get_connection(env)?; + + // Read current state snapshot (short lock) + let query = { + let state = lock_mutex(&self.state, "state")?; + self.build_query(&state) + }?; + // Logged at debug: the built query embeds the substituted offset value. + debug!("Executing query: {}", query); + + let (messages, row_count, max_offset) = + self.execute_statement_and_fetch_rows(env, &connection, &query)?; + + // Fail closed on bulk truncation. The bulk fetch probes with batch_size+1 + // rows, so seeing more than batch_size means the result set is larger than + // one batch and bulk mode (which has no pagination) would otherwise sync + // only an arbitrary subset. Erroring surfaces the misconfiguration instead + // of silently dropping rows. Full cross-database OFFSET pagination is a + // separate follow-up. + if self.config.mode == Mode::Bulk && row_count > self.config.batch_size as u64 { + return Err(Error::InvalidConfigValue(format!( + "bulk query returned more than batch_size ({}) rows; bulk mode does not paginate \ + and would sync only a truncated subset. Increase batch_size to cover the full \ + result set, or use incremental mode with an ordered tracking_column.", + self.config.batch_size + ))); + } + + // Update state with results (short lock) + { + let mut state = lock_mutex(&self.state, "state")?; + if let Some(offset) = max_offset { + state.last_offset = Some(offset); + } + state.processed_rows += row_count; + state.last_poll_time = Utc::now(); + info!( + "Fetched {} rows, total processed: {}", + row_count, state.processed_rows + ); + } + + Ok(messages) + } + + /// Prepare a JDBC statement, execute it, and read all result rows into messages. + fn execute_statement_and_fetch_rows( + &self, + env: &mut JNIEnv, + connection: &JObject, + query: &str, + ) -> Result<(Vec, u64, Option), Error> { + let query_jstring = jni!(env, env.new_string(query), "Failed to create query string"); + + let statement = match env + .call_method( + connection, + "prepareStatement", + "(Ljava/lang/String;)Ljava/sql/PreparedStatement;", + &[JValue::Object(&query_jstring.into())], + ) + .and_then(|v| v.l()) + { + Ok(s) => s, + Err(_) => return Err(classify_query_failure(env, "prepare statement")), + }; + + // Use setMaxRows for database-agnostic row limiting instead of SQL LIMIT clause. + // This works across all JDBC drivers (MySQL, Oracle, SQL Server, H2, etc.). + // In bulk mode fetch one extra row so the caller can detect an oversized + // (truncated) result set and fail closed rather than sync an arbitrary + // subset; incremental mode pages via the offset, so batch_size is the cap. + let max_rows = match self.config.mode { + Mode::Bulk => self.config.batch_size.saturating_add(1), + Mode::Incremental => self.config.batch_size, + }; + if let Err(err) = env.call_method( + &statement, + "setMaxRows", + "(I)V", + &[JValue::Int(max_rows.min(i32::MAX as u32) as i32)], + ) { + best_effort_close(env, &statement); + return Err(Error::Connection(format!("Failed to set max rows: {err}"))); + } + + let result_set = match env + .call_method(&statement, "executeQuery", "()Ljava/sql/ResultSet;", &[]) + .and_then(|v| v.l()) + { + Ok(rs) => rs, + Err(_) => { + // Read and classify the pending SQLException FIRST: this clears + // it, which then makes the statement close() safe (close() is a + // JNI call and must not run with an exception pending). + let error = classify_query_failure(env, "execute query"); + best_effort_close(env, &statement); + return Err(error); + } + }; + + // On any read error the callees clear the pending exception; close the + // statement here before propagating so it is not leaked. + let columns = match self.read_column_metadata(env, &result_set) { + Ok(columns) => columns, + Err(err) => { + best_effort_close(env, &statement); + return Err(err); + } + }; + let (messages, row_count, max_offset) = match self.read_rows(env, &result_set, &columns) { + Ok(result) => result, + Err(err) => { + best_effort_close(env, &statement); + return Err(err); + } + }; + + // Close statement (best-effort) + best_effort_close(env, &statement); + + Ok((messages, row_count, max_offset)) + } + + /// Read column names and types from the ResultSet metadata. + fn read_column_metadata( + &self, + env: &mut JNIEnv, + result_set: &JObject, + ) -> Result, Error> { + // Read all column metadata inside its own JNI local frame so the + // metadata object and per-column name references are reclaimed; a very + // wide table would otherwise accumulate one local ref per column on the + // outer frame for the whole poll. + env.push_local_frame(16) + .map_err(|e| Error::Connection(format!("Failed to push local frame: {}", e)))?; + let result = self.read_column_metadata_inner(env, result_set); + // SAFETY: the returned Vec is owned Rust data; no JNI reference escapes. + let _ = unsafe { env.pop_local_frame(&JObject::null()) }; + result + } + + fn read_column_metadata_inner( + &self, + env: &mut JNIEnv, + result_set: &JObject, + ) -> Result, Error> { + let metadata = jni!( + env, + env.call_method( + result_set, + "getMetaData", + "()Ljava/sql/ResultSetMetaData;", + &[], + ) + .and_then(|v| v.l()), + "Failed to get metadata" + ); + + let column_count = jni!( + env, + env.call_method(&metadata, "getColumnCount", "()I", &[]) + .and_then(|v| v.i()), + "Failed to get column count" + ); + + info!("Query returned {} columns", column_count); + + // Clamp a driver-supplied count before using it as an allocation size: a + // negative i32 would sign-extend to an enormous usize and abort on alloc. + let mut columns = Vec::with_capacity((column_count.max(0) as usize).min(8192)); + for i in 1..=column_count { + let col_name = self.get_column_name(env, &metadata, i)?; + let col_type = self.get_column_type(env, &metadata, i)?; + columns.push((col_name, col_type)); + } + + Ok(columns) + } + + /// Iterate over result set rows and convert each to a ProducedMessage. + fn read_rows( + &self, + env: &mut JNIEnv, + result_set: &JObject, + columns: &[(String, i32)], + ) -> Result<(Vec, u64, Option), Error> { + // setMaxRows caps the result set at batch_size, so that is the known + // upper bound; clamp the pre-allocation so an extreme batch_size cannot + // request an absurd allocation up front. + let mut messages = Vec::with_capacity((self.config.batch_size as usize).min(8192)); + let mut row_count: u64 = 0; + let mut max_offset: Option = None; + + loop { + let has_next = jni!( + env, + env.call_method(result_set, "next", "()Z", &[]) + .and_then(|v| v.z()), + "Failed to fetch next row" + ); + + if !has_next { + break; + } + + // Read each row inside its own JNI local-reference frame so the per + // -column local refs (getObject/getString/getBytes results) are + // reclaimed every iteration; otherwise a large result set would + // overflow the JNI local reference table and abort the JVM. + env.push_local_frame(32) + .map_err(|e| Error::Connection(format!("Failed to push local frame: {}", e)))?; + let row_result = self.read_single_row(env, result_set, columns); + // SAFETY: `read_single_row` returns only owned Rust data (a JSON map + // and an optional String); no JNI local reference escapes the frame. + let _ = unsafe { env.pop_local_frame(&JObject::null()) }; + let (row_data, offset) = row_result?; + + // Track the maximum tracking value across the batch rather than + // assuming the last row is the largest, so incremental mode is + // correct even if the query is not ordered by the tracking column. + if let Some(offset) = offset { + max_offset = Some(match max_offset { + Some(current) => larger_offset(current, offset), + None => offset, + }); + } + + let message = self.build_message(row_data)?; + messages.push(message); + row_count += 1; + } + + Ok((messages, row_count, max_offset)) + } + + /// Extract data from a single result set row, returning the row map and optional offset. + fn read_single_row( + &self, + env: &mut JNIEnv, + result_set: &JObject, + columns: &[(String, i32)], + ) -> Result<(serde_json::Map, Option), Error> { + let mut row_data = serde_json::Map::new(); + let mut offset = None; + + for (idx, (col_name, col_type)) in columns.iter().enumerate() { + let col_idx = (idx + 1) as i32; + let value = self.extract_column_value(env, result_set, col_idx, col_type)?; + + let final_col_name = if self.config.snake_case_columns { + to_snake_case(col_name) + } else { + col_name.clone() + }; + + // A snake_case conversion (or a query with duplicate labels) can map + // two source columns onto the same key; the later value silently wins. + // Warn so the loss is diagnosable rather than invisible. + if row_data.contains_key(&final_col_name) { + warn!( + "Column '{col_name}' maps to key '{final_col_name}', which already exists in the row; the earlier value is overwritten" + ); + } + + row_data.insert(final_col_name.clone(), value); + + // Track offset from the first column matching the tracking column + // (by driver label or normalized key, case-insensitively). Only the + // first match counts: a collapsed/duplicate label must not let a later + // column silently overwrite the offset with an unrelated value. + if offset.is_none() + && let Some(ref tracking_col) = self.config.tracking_column + && tracking_column_matches(tracking_col, col_name, &final_col_name) + { + let value = self.extract_offset_value(&row_data, &final_col_name); + offset = self.tracking_offset_or_error(value, tracking_col)?; + } + } + + Ok((row_data, offset)) + } + + /// Resolve a tracking-column value into an offset. In incremental mode a NULL + /// or empty value is a hard error: the row is emitted but the offset cannot + /// advance past NULL, so it would be re-read (and re-emitted) every poll. + fn tracking_offset_or_error( + &self, + value: Option, + tracking_col: &str, + ) -> Result, Error> { + match value { + Some(value) => Ok(Some(value)), + None if self.config.mode == Mode::Incremental => { + Err(Error::InvalidRecordValue(format!( + "tracking column '{tracking_col}' is NULL or empty in a returned row; incremental \ + mode cannot advance its offset past NULL values. Exclude them in the query, e.g. \ + add `AND {tracking_col} IS NOT NULL`." + ))) + } + None => Ok(None), + } + } + + /// Build a ProducedMessage from row data, optionally wrapping in DatabaseRecord metadata. + fn build_message( + &self, + row_data: serde_json::Map, + ) -> Result { + let now = Utc::now(); + let payload = if self.config.include_metadata { + let record = DatabaseRecord { + table_name: None, + operation_type: "SELECT".to_string(), + timestamp: now, + data: serde_json::Value::Object(row_data), + }; + serde_json::to_vec(&record) + .map_err(|e| Error::Serialization(format!("Failed to serialize record: {e}")))? + } else { + serde_json::to_vec(&serde_json::Value::Object(row_data)) + .map_err(|e| Error::Serialization(format!("Failed to serialize row data: {e}")))? + }; + + let now_ms = now.timestamp_millis() as u64; + Ok(ProducedMessage { + id: Some(Uuid::new_v4().as_u128()), + payload, + headers: None, + checksum: None, + timestamp: Some(now_ms), + origin_timestamp: Some(now_ms), + }) + } + + /// Build the query for this poll by substituting the `{tracking_column}` and + /// `{last_offset}` placeholders. Row limiting is handled via JDBC setMaxRows + /// rather than SQL LIMIT to ensure cross-database compatibility. + fn build_query(&self, state: &State) -> Result { + let mut query = self.config.query.clone(); + + if self.config.mode != Mode::Incremental { + return finalize_query(query); + } + + let offset = state + .last_offset + .as_deref() + .or(self.config.initial_offset.as_deref()); + + // Without an offset yet, drop the incremental predicate but keep the rest + // of the query (e.g. an ORDER BY) intact. + if offset.is_none() { + query = RE_INCREMENTAL_PREDICATE.replace(&query, "").into_owned(); + } + + // Substitute {tracking_column} wherever it still appears (a WHERE and/or + // an ORDER BY), validating it as a plain identifier to avoid injection. + if query.contains("{tracking_column}") { + let column = self.config.tracking_column.as_deref().ok_or_else(|| { + Error::InvalidConfigValue( + "query uses {tracking_column} but tracking_column is not set".to_string(), + ) + })?; + if !is_valid_identifier(column) { + return Err(Error::InvalidConfigValue(format!( + "tracking_column '{column}' is not a valid SQL identifier" + ))); + } + query = query.replace("{tracking_column}", column); + } + + // Substitute the offset value (quoted and escaped) when we have one. + if let Some(offset) = offset { + query = query.replace("{last_offset}", "e_sql_literal(offset)); + } + + finalize_query(query) + } + + /// Get column name from ResultSetMetaData + fn get_column_name( + &self, + env: &mut JNIEnv, + metadata: &JObject, + column_index: i32, + ) -> Result { + let col_name_obj = jni!( + env, + env.call_method( + metadata, + "getColumnName", + "(I)Ljava/lang/String;", + &[JValue::Int(column_index)], + ) + .and_then(|v| v.l()), + "Failed to get column name" + ); + + let col_name: String = jni!( + env, + env.get_string(&JString::from(col_name_obj)), + "Failed to convert column name" + ) + .into(); + + Ok(col_name) + } + + /// Get column type from ResultSetMetaData + fn get_column_type( + &self, + env: &mut JNIEnv, + metadata: &JObject, + column_index: i32, + ) -> Result { + let col_type = jni!( + env, + env.call_method( + metadata, + "getColumnType", + "(I)I", + &[JValue::Int(column_index)], + ) + .and_then(|v| v.i()), + "Failed to get column type" + ); + + Ok(col_type) + } + + /// Extract column value based on JDBC type + fn extract_column_value( + &self, + env: &mut JNIEnv, + result_set: &JObject, + column_index: i32, + sql_type: &i32, + ) -> Result { + use java::sql::Types; + + // Check if null first + let obj = jni!( + env, + env.call_method( + result_set, + "getObject", + "(I)Ljava/lang/Object;", + &[JValue::Int(column_index)], + ) + .and_then(|v| v.l()), + "Failed to get object" + ); + + if obj.is_null() { + return Ok(serde_json::Value::Null); + } + + match *sql_type { + Types::BIT | Types::BOOLEAN => { + let value = jni!( + env, + env.call_method( + result_set, + "getBoolean", + "(I)Z", + &[JValue::Int(column_index)] + ) + .and_then(|v| v.z()), + "Failed to get boolean" + ); + Ok(serde_json::Value::Bool(value)) + } + Types::TINYINT | Types::SMALLINT | Types::INTEGER => { + let value = jni!( + env, + env.call_method(result_set, "getInt", "(I)I", &[JValue::Int(column_index)]) + .and_then(|v| v.i()), + "Failed to get int" + ); + Ok(serde_json::json!(value)) + } + Types::BIGINT => { + let value = jni!( + env, + env.call_method(result_set, "getLong", "(I)J", &[JValue::Int(column_index)]) + .and_then(|v| v.j()), + "Failed to get long" + ); + Ok(serde_json::json!(value)) + } + Types::FLOAT | Types::REAL => { + let value = jni!( + env, + env.call_method(result_set, "getFloat", "(I)F", &[JValue::Int(column_index)]) + .and_then(|v| v.f()), + "Failed to get float" + ); + Ok(serde_json::json!(value)) + } + Types::DOUBLE => { + let value = jni!( + env, + env.call_method( + result_set, + "getDouble", + "(I)D", + &[JValue::Int(column_index)] + ) + .and_then(|v| v.d()), + "Failed to get double" + ); + Ok(serde_json::json!(value)) + } + // NUMERIC/DECIMAL can carry more precision than an f64 can represent + // (e.g. money/large decimals), so emit them as strings to avoid + // silent precision loss. + Types::NUMERIC | Types::DECIMAL => { + self.get_column_as_string(env, result_set, column_index) + } + // Binary columns are base64-encoded so arbitrary bytes survive the + // round-trip through JSON. + Types::BINARY | Types::VARBINARY | Types::LONGVARBINARY => { + let bytes_obj = jni!( + env, + env.call_method( + result_set, + "getBytes", + "(I)[B", + &[JValue::Int(column_index)] + ) + .and_then(|v| v.l()), + "Failed to get bytes" + ); + if bytes_obj.is_null() { + return Ok(serde_json::Value::Null); + } + let buf = jni!( + env, + env.convert_byte_array(JByteArray::from(bytes_obj)), + "Failed to convert bytes" + ); + use base64::Engine; + Ok(serde_json::Value::String( + base64::engine::general_purpose::STANDARD.encode(&buf), + )) + } + Types::TIMESTAMP | Types::DATE | Types::TIME => { + let value = jni!( + env, + env.call_method( + result_set, + "getString", + "(I)Ljava/lang/String;", + &[JValue::Int(column_index)], + ) + .and_then(|v| v.l()), + "Failed to get timestamp" + ); + let str_value: String = jni!( + env, + env.get_string(&JString::from(value)), + "Failed to convert timestamp" + ) + .into(); + Ok(serde_json::Value::String(str_value)) + } + // Default: getString for all other types (CHAR, VARCHAR, etc.) + _ => self.get_column_as_string(env, result_set, column_index), + } + } + + /// Read a column via `ResultSet.getString`, returning JSON `null` when the + /// value is SQL NULL. + fn get_column_as_string( + &self, + env: &mut JNIEnv, + result_set: &JObject, + column_index: i32, + ) -> Result { + let value = jni!( + env, + env.call_method( + result_set, + "getString", + "(I)Ljava/lang/String;", + &[JValue::Int(column_index)], + ) + .and_then(|v| v.l()), + "Failed to get string" + ); + + if value.is_null() { + Ok(serde_json::Value::Null) + } else { + let str_value: String = jni!( + env, + env.get_string(&JString::from(value)), + "Failed to convert string" + ) + .into(); + Ok(serde_json::Value::String(str_value)) + } + } + + /// Extract the tracking-column value as a string offset, or `None` when it is + /// SQL NULL / empty / not comparable. In incremental mode a `None` here is + /// turned into a hard error by [`Self::tracking_offset_or_error`] (a NULL + /// tracking value cannot be watermarked), so the tracking column must be + /// NOT NULL; see the README. + fn extract_offset_value( + &self, + row_data: &serde_json::Map, + col_name: &str, + ) -> Option { + match row_data.get(col_name) { + Some(serde_json::Value::Number(n)) => Some(n.to_string()), + Some(serde_json::Value::String(s)) if !s.is_empty() => Some(s.clone()), + _ => None, + } + } + + /// Validate configuration before touching the JVM or the database, so bad + /// config surfaces immediately at `open()` with an actionable message rather + /// than as an opaque wrapped JVM error or only after the first poll sleep. + fn validate_config(&self) -> Result<(), Error> { + // The driver JAR must exist; a missing path otherwise surfaces as an + // opaque ClassNotFound wrapped deep inside JVM startup. + if !std::path::Path::new(&self.config.driver_jar_path).exists() { + return Err(Error::InvalidConfigValue(format!( + "driver_jar_path '{}' does not exist; set it to the path of the JDBC driver JAR", + self.config.driver_jar_path + ))); + } + + // batch_size drives JDBC setMaxRows. Zero means "no limit" to JDBC, which + // would defeat both the row cap and the bulk truncation probe, so require + // at least 1. Cap below i32::MAX so the bulk `batch_size + 1` probe still + // fits in the i32 setMaxRows takes and stays distinguishable from a full + // batch. + const MAX_BATCH_SIZE: u32 = i32::MAX as u32 - 1; + if self.config.batch_size == 0 || self.config.batch_size > MAX_BATCH_SIZE { + return Err(Error::InvalidConfigValue(format!( + "batch_size must be between 1 and {MAX_BATCH_SIZE}, got {}", + self.config.batch_size + ))); + } + + // Separate-credential auth requires both username and password; a + // half-set pair would silently fall through to URL-embedded credentials. + if self.config.username.is_some() != self.config.password.is_some() { + return Err(Error::InvalidConfigValue( + "username and password must both be set (for separate authentication) or both be \ + unset (to use credentials embedded in the JDBC URL)" + .to_string(), + )); + } + + // Incremental mode invariants. Without a tracking column the offset can + // never advance, so every poll re-reads the same rows. Without ordering + // by that column, setMaxRows returns an arbitrary subset, and advancing + // the offset to its max permanently skips the unread lower keys. + if self.config.mode == Mode::Incremental { + let Some(tracking_column) = self.config.tracking_column.as_deref() else { + return Err(Error::InvalidConfigValue( + "incremental mode requires tracking_column so the offset can advance; set \ + tracking_column, or use mode = \"bulk\"" + .to_string(), + )); + }; + if !query_orders_by_tracking_column(&self.config.query, tracking_column) { + return Err(Error::InvalidConfigValue(format!( + "incremental mode requires the query to order by the tracking column so each \ + batch is a contiguous ascending range; add `ORDER BY {tracking_column}` (or \ + `ORDER BY {{tracking_column}}`) to the query" + ))); + } + } + + // Dry-run the query build so an unresolved placeholder or an invalid + // tracking_column fails now instead of after the first poll interval. + let state = lock_mutex(&self.state, "state")?; + self.build_query(&state)?; + Ok(()) + } +} + +#[async_trait] +impl Source for JdbcSource { + async fn open(&mut self) -> Result<(), Error> { + info!("Opening JDBC source connector [{}]", self.id); + info!( + "Configuration: JDBC URL={}, Driver={}, Mode={:?}", + sanitize_jdbc_url(self.config.jdbc_url.expose_secret()), + self.config.driver_class, + self.config.mode + ); + + // Fail fast on bad config before starting the JVM or opening a connection. + self.validate_config()?; + + // Initialize JVM + self.initialize_jvm()?; + + // Create database connection + self.create_connection()?; + + info!("JDBC source connector [{}] opened successfully", self.id); + Ok(()) + } + + async fn poll(&self) -> Result { + // Pace polls on a fixed cadence measured from a scheduled start instant, + // so per-poll work time does not accumulate as drift and the first poll + // is not delayed by a full interval. The schedule is clamped forward to + // `now` whenever it has fallen behind (a long pause, a poll that overran + // the interval, or the runtime re-polling immediately after an error), + // so a lagging schedule can never collapse the sleep into a busy loop + // that hammers the database. + let scheduled = { + let mut next = lock_mutex(&self.next_poll_at, "next_poll_at")?; + let scheduled = next.map_or_else(Instant::now, |planned| planned.max(Instant::now())); + *next = Some(scheduled + self.config.poll_interval); + scheduled + }; + let now = Instant::now(); + if scheduled > now { + tokio::time::sleep(scheduled - now).await; + } + + // The JDBC/JNI fetch is synchronous, blocking work; run it via + // block_in_place so it does not monopolize a shared async-runtime worker + // while other connectors need to make progress. The connectors runtime is + // multi-threaded, which block_in_place requires. + let messages = tokio::task::block_in_place(|| -> Result, Error> { + let jvm = self + .jvm + .as_ref() + .ok_or_else(|| Error::InitError("JVM not initialized".to_string()))?; + let mut env = jvm + .attach_current_thread() + .map_err(|e| Error::InitError(format!("Failed to attach thread: {e}")))?; + // Defensive: clear any exception left pending by a prior failed poll + // on this thread before issuing JNI calls. + clear_pending_exception(&mut env); + // Bound this poll's local references (the connection local ref, the + // query string, and the statement/result-set handles) to a frame + // reclaimed when the poll returns. A tokio worker thread stays + // attached to the JVM across polls (attach_current_thread returns a + // no-detach nested guard once attached), so without this frame those + // per-poll locals accumulate on the thread's top-level frame and + // eventually overflow the JNI local reference table, aborting the JVM. + env.push_local_frame(16) + .map_err(|e| Error::Connection(format!("Failed to push local frame: {e}")))?; + let result = self.execute_query(&mut env); + // SAFETY: execute_query returns only owned Rust data (messages); no + // JNI local reference escapes the frame. + let _ = unsafe { env.pop_local_frame(&JObject::null()) }; + result + })?; + + // Persist state so offsets survive connector restarts + let connector_state = { + let state = lock_mutex(&self.state, "state")?; + ConnectorState::serialize(&*state, CONNECTOR_NAME, self.id) + }; + + Ok(ProducedMessages { + schema: Schema::Json, + messages, + state: connector_state, + }) + } + + async fn close(&mut self) -> Result<(), Error> { + info!("Closing JDBC source connector [{}]", self.id); + + if self.jvm.is_some() { + // Closing the JDBC connection is blocking JNI work; run it off the + // async worker like the poll path. + tokio::task::block_in_place(|| -> Result<(), Error> { + let Some(jvm) = self.jvm.as_ref() else { + return Ok(()); + }; + let Ok(mut env) = jvm.attach_current_thread() else { + return Ok(()); + }; + let mut guard = lock_mutex(&self.connection, "connection")?; + if let Some(connection) = guard.as_ref() { + best_effort_close(&mut env, connection.as_obj()); + info!("Database connection closed"); + } + *guard = None; + Ok(()) + })?; + } + + let state = lock_mutex(&self.state, "state")?; + info!( + "JDBC source connector [{}] closed. Total rows processed: {}", + self.id, state.processed_rows + ); + + Ok(()) + } +} + +/// Convert string to snake_case +fn to_snake_case(s: &str) -> String { + let mut result = String::new(); + let mut prev_is_upper = false; + + for (i, ch) in s.chars().enumerate() { + if ch.is_uppercase() { + if i > 0 && !prev_is_upper { + result.push('_'); + } + result.extend(ch.to_lowercase()); + prev_is_upper = true; + } else { + result.push(ch); + prev_is_upper = false; + } + } + + result +} + +/// Process-wide JVM. JNI allows only one `JavaVM` per OS process, so every JDBC +/// connector instance in this dynamic library shares this one. +static GLOBAL_JVM: Mutex>> = Mutex::new(None); + +/// Return the process JVM, creating it on first use within this dynamic +/// library. The first caller's `jvm_options`/classpath win; later callers (e.g. +/// a second JDBC connector of the same type) reuse the existing VM instead of +/// failing with `JNI_EEXIST`. +/// +/// Limitation: a JDBC *source* and a JDBC *sink* are separate dynamic libraries +/// and do not share this static, so configuring both in the *same* connectors +/// runtime process is not supported (the second to start cannot create a second +/// JVM). Run them in separate runtime processes. +fn get_or_create_jvm(driver_jar_path: &str, jvm_options: &[String]) -> Result, Error> { + let mut guard = lock_mutex(&GLOBAL_JVM, "jvm")?; + if let Some(jvm) = guard.as_ref() { + info!("Reusing existing process JVM"); + return Ok(jvm.clone()); + } + + let classpath_option = format!("-Djava.class.path={driver_jar_path}"); + let mut args_builder = jni::InitArgsBuilder::new() + .version(jni::JNIVersion::V8) + .option(&classpath_option); + for option in jvm_options { + args_builder = args_builder.option(option); + } + let jvm_args = args_builder + .build() + .map_err(|e| Error::InitError(format!("Failed to build JVM arguments: {e:?}")))?; + let jvm = JavaVM::new(jvm_args) + .map_err(|e| Error::InitError(format!("Failed to create JVM: {e:?}")))?; + + info!("JVM initialized successfully (classpath: {driver_jar_path})"); + let arc = Arc::new(jvm); + *guard = Some(arc.clone()); + Ok(arc) +} + +/// Quote a value as a SQL string literal for substituting the incremental +/// `{last_offset}` value, which originates from a (DB-controlled) tracking-column +/// value. Backslashes are escaped before single quotes are doubled: doubling +/// quotes alone is insufficient under MySQL's default `sql_mode`, where a +/// backslash is an escape character and a trailing `\` could otherwise consume +/// the closing quote and break out of the literal. Binding the offset as a +/// `PreparedStatement` parameter is the DB-agnostic long-term fix (tracked as a +/// follow-up); this keeps the string-substitution path safe in the meantime. +fn quote_sql_literal(value: &str) -> String { + let escaped = value.replace('\\', "\\\\").replace('\'', "''"); + format!("'{escaped}'") +} + +/// Reject a query that still contains an unresolved placeholder, so an invalid +/// statement is never sent to the driver. This guards misconfigurations such as +/// a bulk-mode query using `{last_offset}`, or an incremental query whose +/// predicate could not be auto-removed on the first (no-offset) poll. +fn finalize_query(query: String) -> Result { + if query.contains("{tracking_column}") || query.contains("{last_offset}") { + return Err(Error::InvalidConfigValue( + "query still contains an unresolved {tracking_column}/{last_offset} placeholder; \ + placeholders are only resolved in incremental mode, and require either a persisted \ + offset, an initial_offset, or the exact 'WHERE {tracking_column} > {last_offset}' form" + .to_string(), + )); + } + Ok(query) +} + +/// Return the larger of two offset values. Integer keys are compared as `i128` +/// first (exact across the full `BIGINT` range, avoiding the f64 precision loss +/// past 2^53 that would misorder large IDs and contradict the NUMERIC/DECIMAL +/// -as-string rationale elsewhere in this file); genuinely fractional values +/// fall back to f64; non-numeric values (ISO-8601 timestamps, text keys) compare +/// lexicographically. +/// +/// This assumes the tracking column is homogeneously typed and monotonic under +/// the database's own ordering. A column that mixes numeric-looking and +/// non-numeric strings, or a timestamp whose driver string form is not +/// lexicographically ordered, can make this Rust-side max disagree with the +/// database's `>` comparison and cause rows to be skipped or re-read. See the +/// tracking-column guidance in the README. +fn larger_offset(a: String, b: String) -> String { + let b_is_larger = match (a.parse::(), b.parse::()) { + (Ok(na), Ok(nb)) => nb > na, + _ => match (a.parse::(), b.parse::()) { + (Ok(na), Ok(nb)) => nb > na, + _ => b > a, + }, + }; + if b_is_larger { b } else { a } +} + +/// Check that an incremental query's result set is ordered ascending by the +/// tracking column, so `setMaxRows` returns a contiguous ascending prefix rather +/// than an arbitrary subset (which would let the advancing offset skip unread +/// lower keys). The tracking column must be the FIRST ordering term of the outer +/// `ORDER BY`, and must not be descending. +/// +/// This is a lexical check, not a SQL parser, so it errs strict: it inspects the +/// last `ORDER BY` in the text (the outer query's, not a subquery's), takes the +/// first ordering term, and requires it to be the `{tracking_column}` placeholder +/// or an identifier whose final path segment equals the tracking column +/// (case-insensitive). A trailing `DESC` is rejected, and a composite +/// `ORDER BY other, tracking` (tracking not primary) is rejected, because +/// truncation then yields a prefix ordered by `other`. +fn query_orders_by_tracking_column(query: &str, tracking_column: &str) -> bool { + let lower = query.to_lowercase(); + let Some(pos) = lower.rfind("order by") else { + return false; + }; + let after = lower[pos + "order by".len()..].trim_start(); + let first_term = after.split(',').next().unwrap_or("").trim(); + let mut tokens = first_term.split_whitespace(); + let key = tokens.next().unwrap_or(""); + // Any explicit descending direction breaks ascending offset advancement. + if tokens.any(|token| token == "desc") { + return false; + } + if key.starts_with("{tracking_column}") { + return true; + } + // Compare the final path segment (`t.updated_at` -> `updated_at`), keeping + // only leading identifier characters so trailing punctuation is ignored. + let key_ident: String = key + .rsplit('.') + .next() + .unwrap_or(key) + .chars() + .take_while(|c| c.is_ascii_alphanumeric() || *c == '_') + .collect(); + let tracking_lower = tracking_column.to_lowercase(); + let tracking_ident = tracking_lower.rsplit('.').next().unwrap_or(&tracking_lower); + !key_ident.is_empty() && key_ident == tracking_ident +} + +/// Whether a configured `tracking_column` name refers to this result column. +/// Compares case-insensitively against both the raw driver label and the +/// normalized (snake_cased) output key: drivers fold identifier case (e.g. +/// PostgreSQL lowercases unquoted names) and snake_case output would otherwise +/// never match the configured name, silently stalling the offset. +fn tracking_column_matches(tracking_column: &str, raw_name: &str, normalized_name: &str) -> bool { + tracking_column.eq_ignore_ascii_case(raw_name) + || tracking_column.eq_ignore_ascii_case(normalized_name) +} + +/// Validate that a string is a safe SQL identifier for interpolation: ASCII +/// letters, digits, underscore, and dot (for `table.column`), starting with a +/// letter or underscore. Prevents injection through the `{tracking_column}` +/// placeholder. +fn is_valid_identifier(name: &str) -> bool { + let mut chars = name.chars(); + match chars.next() { + Some(c) if c.is_ascii_alphabetic() || c == '_' => {} + _ => return false, + } + name.chars() + .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '.') +} + +/// Classify a JDBC `SQLState` class (first 2 chars) as transient vs permanent. +/// `08` connection, `40` rollback/serialization, `53` resources, `57` operator +/// intervention, `58` system error are transient; everything else (and an +/// unknown/absent state) is permanent. +fn is_transient_sql_state(sql_state: Option<&str>) -> bool { + // Use `get` rather than slicing: the SQLState comes from the driver and is + // not guaranteed ASCII, so `&s[..2]` could panic on a multi-byte boundary. + match sql_state.and_then(|s| s.get(..2)) { + Some(class) => matches!(class, "08" | "40" | "53" | "57" | "58"), + None => false, + } +} + +/// Inspect and CLEAR the pending Java exception after a failed query JNI call, +/// returning a classified `Error`: transient SQL states (connection/resource +/// classes) map to `Error::Connection`, permanent ones (syntax/constraint) to +/// `Error::InvalidRecordValue`. Clearing is required so the next JNI call on this +/// thread is not aborted. +/// +/// NOTE: the runtime does not yet branch on this distinction (there is no +/// per-variant backoff in the poll loop today), so the classification is +/// currently informational: it shapes the error variant and log message and is +/// kept ready for when SDK-side backoff lands. Do not claim differentiated +/// runtime backoff until that exists. +fn classify_query_failure(env: &mut JNIEnv, action: &str) -> Error { + let (sql_state, message) = take_pending_sql_exception(env); + let transient = is_transient_sql_state(sql_state.as_deref()); + let state = sql_state.as_deref().unwrap_or("?"); + let msg = format!("Failed to {action} (SQLState {state}): {message}"); + if transient { + Error::Connection(msg) + } else { + Error::InvalidRecordValue(msg) + } +} + +/// Take the pending Java exception (clearing it) and return its `SQLState` (if a +/// `java.sql.SQLException`) and message. +fn take_pending_sql_exception(env: &mut JNIEnv) -> (Option, String) { + let throwable = match env.exception_occurred() { + Ok(t) if !t.is_null() => t, + _ => return (None, "unknown error".to_string()), + }; + let _ = env.exception_clear(); + + let message = throwable_string_method(env, &throwable, "getMessage") + .unwrap_or_else(|| "unknown error".to_string()); + let sql_state = if env + .is_instance_of(&throwable, "java/sql/SQLException") + .unwrap_or(false) + { + throwable_string_method(env, &throwable, "getSQLState") + } else { + None + }; + (sql_state, message) +} + +/// Call a no-arg `String`-returning method on a throwable; None on JNI error/null. +fn throwable_string_method( + env: &mut JNIEnv, + throwable: &JThrowable, + method: &str, +) -> Option { + let obj = env + .call_method(throwable, method, "()Ljava/lang/String;", &[]) + .ok()? + .l() + .ok()?; + if obj.is_null() { + return None; + } + env.get_string(&JString::from(obj)).ok().map(|s| s.into()) +} + +/// JDBC SQL Types constants +mod java { + pub mod sql { + #[allow(dead_code)] + pub struct Types; + + #[allow(dead_code)] + impl Types { + pub const BIT: i32 = -7; + pub const TINYINT: i32 = -6; + pub const SMALLINT: i32 = 5; + pub const INTEGER: i32 = 4; + pub const BIGINT: i32 = -5; + pub const FLOAT: i32 = 6; + pub const REAL: i32 = 7; + pub const DOUBLE: i32 = 8; + pub const NUMERIC: i32 = 2; + pub const DECIMAL: i32 = 3; + pub const CHAR: i32 = 1; + pub const VARCHAR: i32 = 12; + pub const LONGVARCHAR: i32 = -1; + pub const DATE: i32 = 91; + pub const TIME: i32 = 92; + pub const TIMESTAMP: i32 = 93; + pub const BINARY: i32 = -2; + pub const VARBINARY: i32 = -3; + pub const LONGVARBINARY: i32 = -4; + pub const NULL: i32 = 0; + pub const BOOLEAN: i32 = 16; + } + } +} + +// Export the connector via SDK macro +source_connector!(JdbcSource); + +#[cfg(test)] +mod tests { + use super::*; + + /// A minimal, valid bulk-mode config for tests that need a `JdbcSourceConfig`. + fn base_config() -> JdbcSourceConfig { + JdbcSourceConfig { + jdbc_url: SecretString::from("jdbc:h2:mem:test"), + driver_class: "org.h2.Driver".to_string(), + driver_jar_path: "/tmp/h2.jar".to_string(), + username: None, + password: None, + query: "SELECT 1".to_string(), + poll_interval: Duration::from_secs(10), + batch_size: 100, + tracking_column: None, + initial_offset: None, + mode: Mode::Bulk, + snake_case_columns: false, + include_metadata: true, + jvm_options: vec![], + connection_timeout_ms: 30000, + } + } + + /// Write a throwaway file to stand in for a driver JAR and return its path, + /// so `validate_config`'s existence check passes. + fn write_temp_jar(name: &str) -> String { + let path = std::env::temp_dir().join(name); + std::fs::write(&path, b"jar").expect("write temp jar"); + path.to_string_lossy().into_owned() + } + + #[test] + fn test_quote_sql_literal_escapes_backslash() { + // Backslash is doubled before quotes so a trailing backslash cannot + // consume the closing quote under MySQL's default sql_mode. + assert_eq!(quote_sql_literal(r"a\b"), r"'a\\b'"); + assert_eq!(quote_sql_literal(r"end\"), r"'end\\'"); + assert_eq!(quote_sql_literal(r"\'"), r"'\\'''"); + } + + #[test] + fn test_larger_offset_i128_precision_beyond_f64() { + // Two BIGINTs that differ only past 2^53 must still be ordered exactly; + // an f64 comparison would collapse them and misorder the offset. + let a = "9007199254740993".to_string(); // 2^53 + 1 + let b = "9007199254740992".to_string(); // 2^53 + assert_eq!(larger_offset(a.clone(), b.clone()), a); + assert_eq!(larger_offset(b, a.clone()), a); + } + + #[test] + fn test_build_query_removes_predicate_case_and_whitespace_insensitive() { + for query in [ + "select * from t where {tracking_column} > {last_offset} order by id", + "SELECT * FROM t WHERE {tracking_column} > {last_offset} ORDER BY id", + "SELECT * FROM t where {tracking_column}>{last_offset} ORDER BY id", + ] { + let mut config = base_config(); + config.query = query.to_string(); + config.mode = Mode::Incremental; + config.tracking_column = Some("id".to_string()); + let source = JdbcSource::new(1, config, None); + let state = State::default(); + let built = source.build_query(&state).expect("build query"); + assert!( + !built.contains("{last_offset}") && !built.contains("{tracking_column}"), + "predicate not removed for query variant: {query} -> {built}" + ); + assert!(built.to_lowercase().contains("order by id")); + } + } + + #[test] + fn test_validate_config_rejects_half_set_credentials() { + let jar = write_temp_jar("jdbc_validate_half_creds.jar"); + let mut config = base_config(); + config.driver_jar_path = jar; + config.username = Some("user".to_string()); + config.password = None; + let source = JdbcSource::new(1, config, None); + assert!(matches!( + source.validate_config(), + Err(Error::InvalidConfigValue(_)) + )); + } + + #[test] + fn test_validate_config_rejects_missing_driver_jar() { + let mut config = base_config(); + config.driver_jar_path = "/nonexistent/path/to/driver.jar".to_string(); + let source = JdbcSource::new(1, config, None); + let err = source.validate_config().expect_err("missing jar must fail"); + assert!(matches!(err, Error::InvalidConfigValue(msg) if msg.contains("driver_jar_path"))); + } + + #[test] + fn test_validate_config_dry_runs_query() { + let jar = write_temp_jar("jdbc_validate_dry_run.jar"); + let mut config = base_config(); + config.driver_jar_path = jar; + // Passes the incremental invariants (tracking_column set, ordered by it) + // but the non-canonical predicate leaves {last_offset} unresolved with no + // offset, so the dry-run build_query must reject it now. + config.mode = Mode::Incremental; + config.tracking_column = Some("id".to_string()); + config.query = "SELECT * FROM t WHERE x >= {last_offset} ORDER BY id".to_string(); + let source = JdbcSource::new(1, config, None); + assert!(matches!( + source.validate_config(), + Err(Error::InvalidConfigValue(_)) + )); + } + + #[test] + fn test_validate_config_accepts_valid_bulk() { + let jar = write_temp_jar("jdbc_validate_ok.jar"); + let mut config = base_config(); + config.driver_jar_path = jar; + let source = JdbcSource::new(1, config, None); + assert!(source.validate_config().is_ok()); + } + + #[test] + fn test_validate_config_accepts_valid_incremental() { + let jar = write_temp_jar("jdbc_validate_ok_incremental.jar"); + let mut config = base_config(); + config.driver_jar_path = jar; + config.mode = Mode::Incremental; + config.tracking_column = Some("id".to_string()); + // Canonical placeholder predicate so the cold-start (no offset) build + // auto-removes the WHERE; ordered by the tracking column. + config.query = + "SELECT id, name FROM t WHERE {tracking_column} > {last_offset} ORDER BY {tracking_column}" + .to_string(); + let source = JdbcSource::new(1, config, None); + assert!(source.validate_config().is_ok()); + } + + #[test] + fn test_validate_config_incremental_requires_tracking_column() { + let jar = write_temp_jar("jdbc_validate_no_tracking.jar"); + let mut config = base_config(); + config.driver_jar_path = jar; + config.mode = Mode::Incremental; + config.tracking_column = None; + config.query = "SELECT id FROM t ORDER BY id".to_string(); + let source = JdbcSource::new(1, config, None); + let err = source + .validate_config() + .expect_err("must require tracking_column"); + assert!(matches!(err, Error::InvalidConfigValue(msg) if msg.contains("tracking_column"))); + } + + #[test] + fn test_validate_config_incremental_requires_order_by_tracking_column() { + let jar = write_temp_jar("jdbc_validate_no_order.jar"); + let mut config = base_config(); + config.driver_jar_path = jar; + config.mode = Mode::Incremental; + config.tracking_column = Some("id".to_string()); + // No ORDER BY: an unordered incremental query can skip rows on truncation. + config.query = "SELECT id FROM t WHERE id > {last_offset}".to_string(); + let source = JdbcSource::new(1, config, None); + let err = source.validate_config().expect_err("must require ORDER BY"); + assert!( + matches!(err, Error::InvalidConfigValue(msg) if msg.to_lowercase().contains("order by")) + ); + } + + #[test] + fn test_query_orders_by_tracking_column_accepts_valid() { + assert!(query_orders_by_tracking_column( + "SELECT * FROM t WHERE id > {last_offset} ORDER BY id", + "id" + )); + // Placeholder form, case/whitespace variance, explicit ASC. + assert!(query_orders_by_tracking_column( + "select * from t order by {tracking_column}", + "updated_at" + )); + assert!(query_orders_by_tracking_column( + "SELECT * FROM t ORDER BY Updated_At ASC", + "updated_at" + )); + // Table-qualified column, and the outer ORDER BY after a subquery. + assert!(query_orders_by_tracking_column( + "SELECT * FROM t ORDER BY t.updated_at", + "updated_at" + )); + assert!(query_orders_by_tracking_column( + "SELECT * FROM (SELECT * FROM t ORDER BY x) s ORDER BY id", + "id" + )); + } + + #[test] + fn test_query_orders_by_tracking_column_rejects_invalid() { + // No ORDER BY. + assert!(!query_orders_by_tracking_column( + "SELECT * FROM t WHERE id > 0", + "id" + )); + // Different column. + assert!(!query_orders_by_tracking_column( + "SELECT * FROM t ORDER BY name", + "id" + )); + // Descending breaks ascending offset advancement. + assert!(!query_orders_by_tracking_column( + "SELECT * FROM t ORDER BY updated_at DESC", + "updated_at" + )); + // Substring-only match must not pass (id is a substring of valid_flag/id_backup). + assert!(!query_orders_by_tracking_column( + "SELECT * FROM t ORDER BY valid_flag", + "id" + )); + assert!(!query_orders_by_tracking_column( + "SELECT * FROM t ORDER BY id_backup", + "id" + )); + // Tracking column not the primary (first) ordering term. + assert!(!query_orders_by_tracking_column( + "SELECT * FROM t ORDER BY name, id", + "id" + )); + } + + #[test] + fn test_validate_config_rejects_bad_batch_size() { + let jar = write_temp_jar("jdbc_validate_batch_size.jar"); + for bad in [0u32, i32::MAX as u32] { + let mut config = base_config(); + config.driver_jar_path = jar.clone(); + config.batch_size = bad; + let source = JdbcSource::new(1, config, None); + let err = source + .validate_config() + .expect_err("must reject invalid batch_size"); + assert!(matches!(err, Error::InvalidConfigValue(msg) if msg.contains("batch_size"))); + } + } + + #[test] + fn test_tracking_column_matches_case_and_normalization() { + // Case-insensitive against the raw driver label (driver case-folding). + assert!(tracking_column_matches( + "OrderDate", + "orderdate", + "orderdate" + )); + // Matches the normalized (snake_cased) output key. + assert!(tracking_column_matches( + "order_date", + "OrderDate", + "order_date" + )); + // Plain lowercase match. + assert!(tracking_column_matches("id", "id", "id")); + // Genuinely different column does not match. + assert!(!tracking_column_matches("id", "name", "name")); + } + + #[test] + fn test_tracking_offset_or_error_rejects_null_in_incremental() { + let mut config = base_config(); + config.mode = Mode::Incremental; + config.tracking_column = Some("id".to_string()); + let source = JdbcSource::new(1, config, None); + // Non-null resolves; NULL/empty (None) is a hard error in incremental mode. + assert_eq!( + source + .tracking_offset_or_error(Some("42".to_string()), "id") + .unwrap(), + Some("42".to_string()) + ); + assert!(matches!( + source.tracking_offset_or_error(None, "id"), + Err(Error::InvalidRecordValue(_)) + )); + } + + #[test] + fn test_tracking_offset_or_error_allows_null_in_bulk() { + let source = JdbcSource::new(1, base_config(), None); // base_config is bulk + assert_eq!(source.tracking_offset_or_error(None, "id").unwrap(), None); + } + + #[test] + fn test_sanitize_jdbc_url_mysql_format() { + let url = "jdbc:mysql://root:SuperSecret123@localhost:3306/mydb"; + let sanitized = sanitize_jdbc_url(url); + assert_eq!(sanitized, "jdbc:mysql://root:***@localhost:3306/mydb"); + assert!(!sanitized.contains("SuperSecret123")); + } + + #[test] + fn test_sanitize_jdbc_url_postgresql_query_params() { + let url = "jdbc:postgresql://localhost:5432/mydb?user=admin&password=P@ssw0rd&ssl=true"; + let sanitized = sanitize_jdbc_url(url); + assert_eq!( + sanitized, + "jdbc:postgresql://localhost:5432/mydb?user=admin&password=***&ssl=true" + ); + assert!(!sanitized.contains("P@ssw0rd")); + } + + #[test] + fn test_sanitize_jdbc_url_oracle_format() { + let url = "jdbc:oracle:thin:system/oracle123@localhost:1521:XE"; + let sanitized = sanitize_jdbc_url(url); + assert_eq!(sanitized, "jdbc:oracle:thin:system/***@localhost:1521:XE"); + assert!(!sanitized.contains("oracle123")); + } + + #[test] + fn test_sanitize_jdbc_url_sqlserver_format() { + let url = "jdbc:sqlserver://localhost:1433;user=sa;password=MySecretPass;database=Sales"; + let sanitized = sanitize_jdbc_url(url); + assert_eq!( + sanitized, + "jdbc:sqlserver://localhost:1433;user=sa;password=***;database=Sales" + ); + assert!(!sanitized.contains("MySecretPass")); + } + + #[test] + fn test_sanitize_jdbc_url_h2_format() { + let url = "jdbc:h2:mem:testdb;USER=sa;PASSWORD=secret"; + let sanitized = sanitize_jdbc_url(url); + assert_eq!(sanitized, "jdbc:h2:mem:testdb;USER=sa;PASSWORD=***"); + assert!(!sanitized.contains("secret")); + } + + #[test] + fn test_sanitize_jdbc_url_case_insensitive() { + let url1 = "jdbc:postgresql://localhost?password=secret"; + let url2 = "jdbc:postgresql://localhost?PASSWORD=secret"; + let url3 = "jdbc:postgresql://localhost?pwd=secret"; + let url4 = "jdbc:postgresql://localhost?PWD=secret"; + + for url in [url1, url2, url3, url4] { + let sanitized = sanitize_jdbc_url(url); + assert!(!sanitized.contains("secret"), "Failed for URL: {}", url); + assert!(sanitized.contains("***")); + } + } + + #[test] + fn test_sanitize_jdbc_url_no_password() { + let url = "jdbc:h2:mem:testdb"; + let sanitized = sanitize_jdbc_url(url); + assert_eq!(sanitized, url); + } + + #[test] + fn test_sanitize_jdbc_url_multiple_passwords() { + let url = "jdbc:postgresql://localhost?password=secret1&pwd=secret2"; + let sanitized = sanitize_jdbc_url(url); + assert!(!sanitized.contains("secret1")); + assert!(!sanitized.contains("secret2")); + assert_eq!( + sanitized, + "jdbc:postgresql://localhost?password=***&pwd=***" + ); + } + + #[test] + fn test_build_query_incremental_with_offset() { + let config = JdbcSourceConfig { + jdbc_url: SecretString::from("jdbc:h2:mem:test"), + driver_class: "org.h2.Driver".to_string(), + driver_jar_path: "/tmp/h2.jar".to_string(), + username: None, + password: None, + query: "SELECT * FROM users WHERE id > {last_offset} ORDER BY id".to_string(), + poll_interval: Duration::from_secs(10), + batch_size: 100, + tracking_column: Some("id".to_string()), + initial_offset: Some("0".to_string()), + mode: Mode::Incremental, + snake_case_columns: false, + include_metadata: true, + jvm_options: vec![], + connection_timeout_ms: 30000, + }; + let source = JdbcSource::new(1, config, None); + + // With initial offset (no last_offset yet) + let state = State { + last_offset: None, + processed_rows: 0, + last_poll_time: Utc::now(), + }; + let query = source.build_query(&state).expect("build query"); + assert_eq!(query, "SELECT * FROM users WHERE id > '0' ORDER BY id"); + + // With tracked offset + let state = State { + last_offset: Some("42".to_string()), + processed_rows: 42, + last_poll_time: Utc::now(), + }; + let query = source.build_query(&state).expect("build query"); + assert_eq!(query, "SELECT * FROM users WHERE id > '42' ORDER BY id"); + } + + #[test] + fn test_build_query_substitutes_tracking_column() { + let config = JdbcSourceConfig { + jdbc_url: SecretString::from("jdbc:h2:mem:test"), + driver_class: "org.h2.Driver".to_string(), + driver_jar_path: "/tmp/h2.jar".to_string(), + username: None, + password: None, + query: + "SELECT * FROM orders WHERE {tracking_column} > {last_offset} ORDER BY {tracking_column}" + .to_string(), + poll_interval: Duration::from_secs(10), + batch_size: 100, + tracking_column: Some("updated_at".to_string()), + initial_offset: Some("2024-01-01".to_string()), + mode: Mode::Incremental, + snake_case_columns: false, + include_metadata: true, + jvm_options: vec![], + connection_timeout_ms: 30000, + }; + let source = JdbcSource::new(1, config, None); + let state = State { + last_offset: Some("2024-06-15".to_string()), + processed_rows: 0, + last_poll_time: Utc::now(), + }; + let query = source.build_query(&state).expect("build query"); + assert_eq!( + query, + "SELECT * FROM orders WHERE updated_at > '2024-06-15' ORDER BY updated_at" + ); + } + + #[test] + fn test_build_query_no_offset_substitutes_tracking_column_in_order_by() { + let config = JdbcSourceConfig { + jdbc_url: SecretString::from("jdbc:h2:mem:test"), + driver_class: "org.h2.Driver".to_string(), + driver_jar_path: "/tmp/h2.jar".to_string(), + username: None, + password: None, + query: + "SELECT * FROM orders WHERE {tracking_column} > {last_offset} ORDER BY {tracking_column}" + .to_string(), + poll_interval: Duration::from_secs(10), + batch_size: 100, + tracking_column: Some("updated_at".to_string()), + initial_offset: None, + mode: Mode::Incremental, + snake_case_columns: false, + include_metadata: true, + jvm_options: vec![], + connection_timeout_ms: 30000, + }; + let source = JdbcSource::new(1, config, None); + // No last_offset and no initial_offset: the WHERE predicate is dropped, + // but the ORDER BY {tracking_column} must still be substituted. + let query = source + .build_query(&State { + last_offset: None, + processed_rows: 0, + last_poll_time: Utc::now(), + }) + .expect("build query"); + assert!(!query.contains("{tracking_column}"), "got: {query}"); + assert!(!query.contains("{last_offset}"), "got: {query}"); + assert!(query.contains("ORDER BY updated_at"), "got: {query}"); + } + + #[test] + fn test_build_query_rejects_injection_in_tracking_column() { + let config = JdbcSourceConfig { + jdbc_url: SecretString::from("jdbc:h2:mem:test"), + driver_class: "org.h2.Driver".to_string(), + driver_jar_path: "/tmp/h2.jar".to_string(), + username: None, + password: None, + query: "SELECT * FROM t WHERE {tracking_column} > {last_offset}".to_string(), + poll_interval: Duration::from_secs(10), + batch_size: 100, + tracking_column: Some("id; DROP TABLE t".to_string()), + initial_offset: Some("0".to_string()), + mode: Mode::Incremental, + snake_case_columns: false, + include_metadata: true, + jvm_options: vec![], + connection_timeout_ms: 30000, + }; + let source = JdbcSource::new(1, config, None); + assert!(source.build_query(&State::default()).is_err()); + } + + #[test] + fn test_build_query_bulk_mode_no_limit_appended() { + let config = JdbcSourceConfig { + jdbc_url: SecretString::from("jdbc:h2:mem:test"), + driver_class: "org.h2.Driver".to_string(), + driver_jar_path: "/tmp/h2.jar".to_string(), + username: None, + password: None, + query: "SELECT * FROM products".to_string(), + poll_interval: Duration::from_secs(60), + batch_size: 5000, + tracking_column: None, + initial_offset: None, + mode: Mode::Bulk, + snake_case_columns: false, + include_metadata: false, + jvm_options: vec![], + connection_timeout_ms: 30000, + }; + let source = JdbcSource::new(1, config, None); + let state = State::default(); + let query = source.build_query(&state).expect("build query"); + // build_query should NOT append LIMIT; row limiting is done via setMaxRows + assert_eq!(query, "SELECT * FROM products"); + assert!(!query.to_uppercase().contains("LIMIT")); + } + + #[test] + fn test_state_restoration_from_connector_state() { + let original_state = State { + last_offset: Some("2024-06-15 12:00:00".to_string()), + processed_rows: 1500, + last_poll_time: Utc::now(), + }; + let connector_state = ConnectorState::serialize(&original_state, CONNECTOR_NAME, 1) + .expect("Failed to serialize state"); + + let config = JdbcSourceConfig { + jdbc_url: SecretString::from("jdbc:h2:mem:test"), + driver_class: "org.h2.Driver".to_string(), + driver_jar_path: "/tmp/h2.jar".to_string(), + username: None, + password: None, + query: "SELECT * FROM orders WHERE updated_at > {last_offset}".to_string(), + poll_interval: Duration::from_secs(30), + batch_size: 1000, + tracking_column: Some("updated_at".to_string()), + initial_offset: Some("2024-01-01 00:00:00".to_string()), + mode: Mode::Incremental, + snake_case_columns: false, + include_metadata: true, + jvm_options: vec![], + connection_timeout_ms: 30000, + }; + let source = JdbcSource::new(1, config, Some(connector_state)); + let state = source.state.lock().unwrap(); + assert_eq!(state.last_offset, Some("2024-06-15 12:00:00".to_string())); + assert_eq!(state.processed_rows, 1500); + } + + #[test] + fn test_quote_sql_literal_escapes_single_quotes() { + assert_eq!(quote_sql_literal("42"), "'42'"); + assert_eq!( + quote_sql_literal("2024-01-01 00:00:00"), + "'2024-01-01 00:00:00'" + ); + assert_eq!(quote_sql_literal("o'brien"), "'o''brien'"); + assert_eq!( + quote_sql_literal("x'; DROP TABLE t; --"), + "'x''; DROP TABLE t; --'" + ); + } + + #[test] + fn test_is_transient_sql_state() { + for s in [ + "08001", "08006", "40001", "40P01", "53300", "57P01", "58030", + ] { + assert!(is_transient_sql_state(Some(s)), "{s} should be transient"); + } + for s in ["22001", "23505", "42601", "42P01", "99999"] { + assert!(!is_transient_sql_state(Some(s)), "{s} should be permanent"); + } + assert!(!is_transient_sql_state(None)); + assert!(!is_transient_sql_state(Some(""))); + } + + #[test] + fn test_larger_offset_numeric_and_lexical() { + // Numeric comparison, not lexical: "100" > "9" numerically. + assert_eq!(larger_offset("9".into(), "100".into()), "100"); + assert_eq!(larger_offset("100".into(), "9".into()), "100"); + assert_eq!(larger_offset("10.5".into(), "9.9".into()), "10.5"); + // Non-numeric (timestamps / text) compare lexicographically. + assert_eq!( + larger_offset("2024-01-01".into(), "2024-06-15".into()), + "2024-06-15" + ); + } + + #[test] + fn test_is_valid_identifier() { + assert!(is_valid_identifier("id")); + assert!(is_valid_identifier("updated_at")); + assert!(is_valid_identifier("t.updated_at")); + assert!(!is_valid_identifier("id; DROP TABLE t")); + assert!(!is_valid_identifier("1col")); + assert!(!is_valid_identifier("col name")); + assert!(!is_valid_identifier("")); + } + + #[test] + fn test_to_snake_case() { + assert_eq!(to_snake_case("OrderDate"), "order_date"); + assert_eq!(to_snake_case("updatedAt"), "updated_at"); + assert_eq!(to_snake_case("ID"), "id"); // consecutive uppers stay together + assert_eq!(to_snake_case("already_snake"), "already_snake"); + assert_eq!(to_snake_case("simple"), "simple"); + } + + // ========================================================================= + // Config deserialization tests + // ========================================================================= + + #[test] + fn test_config_deserialization_minimal_toml() { + let toml_str = r#" + jdbc_url = "jdbc:h2:mem:test" + driver_class = "org.h2.Driver" + driver_jar_path = "/tmp/h2.jar" + query = "SELECT * FROM users" + poll_interval = "30s" + "#; + let config: JdbcSourceConfig = + toml::from_str(toml_str).expect("Failed to parse minimal TOML config"); + assert_eq!(config.driver_class, "org.h2.Driver"); + assert_eq!(config.query, "SELECT * FROM users"); + assert_eq!(config.poll_interval, Duration::from_secs(30)); + // Verify defaults are applied + assert_eq!(config.mode, Mode::Incremental); + assert_eq!(config.batch_size, 1000); + assert!(config.include_metadata); + assert!(!config.snake_case_columns); + assert_eq!(config.connection_timeout_ms, 30000); + assert!(config.username.is_none()); + assert!(config.password.is_none()); + assert!(config.tracking_column.is_none()); + assert!(config.initial_offset.is_none()); + assert!(config.jvm_options.is_empty()); + } + + #[test] + fn test_config_deserialization_full_toml() { + let toml_str = r#" + jdbc_url = "jdbc:mysql://localhost:3306/mydb" + driver_class = "com.mysql.cj.jdbc.Driver" + driver_jar_path = "/opt/drivers/mysql.jar" + username = "admin" + password = "s3cret" + query = "SELECT * FROM orders WHERE id > {last_offset} ORDER BY id" + poll_interval = "5m" + batch_size = 500 + tracking_column = "id" + initial_offset = "0" + mode = "incremental" + snake_case_columns = true + include_metadata = false + jvm_options = ["-Xmx512m", "-Xms128m"] + connection_timeout_ms = 60000 + "#; + let config: JdbcSourceConfig = + toml::from_str(toml_str).expect("Failed to parse full TOML config"); + assert_eq!(config.driver_class, "com.mysql.cj.jdbc.Driver"); + assert_eq!(config.username.as_deref(), Some("admin")); + assert!(config.password.is_some()); + assert_eq!(config.batch_size, 500); + assert_eq!(config.tracking_column.as_deref(), Some("id")); + assert_eq!(config.initial_offset.as_deref(), Some("0")); + assert_eq!(config.mode, Mode::Incremental); + assert!(config.snake_case_columns); + assert!(!config.include_metadata); + assert_eq!(config.jvm_options, vec!["-Xmx512m", "-Xms128m"]); + assert_eq!(config.connection_timeout_ms, 60000); + assert_eq!(config.poll_interval, Duration::from_secs(300)); + } + + #[test] + fn test_config_deserialization_bulk_mode() { + let toml_str = r#" + jdbc_url = "jdbc:h2:mem:test" + driver_class = "org.h2.Driver" + driver_jar_path = "/tmp/h2.jar" + query = "SELECT * FROM products" + poll_interval = "1h" + mode = "bulk" + "#; + let config: JdbcSourceConfig = + toml::from_str(toml_str).expect("Failed to parse bulk mode config"); + assert_eq!(config.mode, Mode::Bulk); + assert_eq!(config.poll_interval, Duration::from_secs(3600)); + } + + #[test] + fn test_config_deserialization_invalid_mode_fails() { + let toml_str = r#" + jdbc_url = "jdbc:h2:mem:test" + driver_class = "org.h2.Driver" + driver_jar_path = "/tmp/h2.jar" + query = "SELECT 1" + poll_interval = "1s" + mode = "invalid_mode" + "#; + let result = toml::from_str::(toml_str); + assert!( + result.is_err(), + "Expected error for invalid mode, but got: {:?}", + result + ); + } + + // ========================================================================= + // State restoration tests + // ========================================================================= + + #[test] + fn test_state_restoration_with_malformed_bytes_falls_back_to_default() { + let connector_state = ConnectorState(vec![0xFF, 0xFE, 0xFD, 0x00]); + + let config = JdbcSourceConfig { + jdbc_url: SecretString::from("jdbc:h2:mem:test"), + driver_class: "org.h2.Driver".to_string(), + driver_jar_path: "/tmp/h2.jar".to_string(), + username: None, + password: None, + query: "SELECT 1".to_string(), + poll_interval: Duration::from_secs(10), + batch_size: 100, + tracking_column: None, + initial_offset: None, + mode: Mode::Bulk, + snake_case_columns: false, + include_metadata: true, + jvm_options: vec![], + connection_timeout_ms: 30000, + }; + let source = JdbcSource::new(1, config, Some(connector_state)); + let state = source.state.lock().unwrap(); + // Should fall back to default state + assert!(state.last_offset.is_none()); + assert_eq!(state.processed_rows, 0); + } + + #[test] + fn test_state_restoration_with_empty_bytes_falls_back_to_default() { + let connector_state = ConnectorState(vec![]); + + let config = JdbcSourceConfig { + jdbc_url: SecretString::from("jdbc:h2:mem:test"), + driver_class: "org.h2.Driver".to_string(), + driver_jar_path: "/tmp/h2.jar".to_string(), + username: None, + password: None, + query: "SELECT 1".to_string(), + poll_interval: Duration::from_secs(10), + batch_size: 100, + tracking_column: None, + initial_offset: None, + mode: Mode::Bulk, + snake_case_columns: false, + include_metadata: true, + jvm_options: vec![], + connection_timeout_ms: 30000, + }; + let source = JdbcSource::new(1, config, Some(connector_state)); + let state = source.state.lock().unwrap(); + assert!(state.last_offset.is_none()); + assert_eq!(state.processed_rows, 0); + } + + #[test] + fn test_state_restoration_none_uses_initial_offset() { + let config = JdbcSourceConfig { + jdbc_url: SecretString::from("jdbc:h2:mem:test"), + driver_class: "org.h2.Driver".to_string(), + driver_jar_path: "/tmp/h2.jar".to_string(), + username: None, + password: None, + query: "SELECT * FROM orders WHERE id > {last_offset}".to_string(), + poll_interval: Duration::from_secs(10), + batch_size: 100, + tracking_column: Some("id".to_string()), + initial_offset: Some("100".to_string()), + mode: Mode::Incremental, + snake_case_columns: false, + include_metadata: true, + jvm_options: vec![], + connection_timeout_ms: 30000, + }; + let source = JdbcSource::new(1, config, None); + let state = source.state.lock().unwrap(); + assert_eq!(state.last_offset, Some("100".to_string())); + assert_eq!(state.processed_rows, 0); + } + + #[test] + fn test_state_restoration_none_without_initial_offset() { + let config = JdbcSourceConfig { + jdbc_url: SecretString::from("jdbc:h2:mem:test"), + driver_class: "org.h2.Driver".to_string(), + driver_jar_path: "/tmp/h2.jar".to_string(), + username: None, + password: None, + query: "SELECT * FROM products".to_string(), + poll_interval: Duration::from_secs(60), + batch_size: 100, + tracking_column: None, + initial_offset: None, + mode: Mode::Bulk, + snake_case_columns: false, + include_metadata: true, + jvm_options: vec![], + connection_timeout_ms: 30000, + }; + let source = JdbcSource::new(1, config, None); + let state = source.state.lock().unwrap(); + assert!(state.last_offset.is_none()); + assert_eq!(state.processed_rows, 0); + } + + // ========================================================================= + // extract_offset_value tests + // ========================================================================= + + #[test] + fn test_extract_offset_value_with_integer() { + let config = JdbcSourceConfig { + jdbc_url: SecretString::from("jdbc:h2:mem:test"), + driver_class: "org.h2.Driver".to_string(), + driver_jar_path: "/tmp/h2.jar".to_string(), + username: None, + password: None, + query: "SELECT 1".to_string(), + poll_interval: Duration::from_secs(10), + batch_size: 100, + tracking_column: None, + initial_offset: None, + mode: Mode::Bulk, + snake_case_columns: false, + include_metadata: true, + jvm_options: vec![], + connection_timeout_ms: 30000, + }; + let source = JdbcSource::new(1, config, None); + + let mut row = serde_json::Map::new(); + row.insert("id".to_string(), serde_json::json!(42)); + assert_eq!( + source.extract_offset_value(&row, "id"), + Some("42".to_string()) + ); + } + + #[test] + fn test_extract_offset_value_with_string() { + let config = JdbcSourceConfig { + jdbc_url: SecretString::from("jdbc:h2:mem:test"), + driver_class: "org.h2.Driver".to_string(), + driver_jar_path: "/tmp/h2.jar".to_string(), + username: None, + password: None, + query: "SELECT 1".to_string(), + poll_interval: Duration::from_secs(10), + batch_size: 100, + tracking_column: None, + initial_offset: None, + mode: Mode::Bulk, + snake_case_columns: false, + include_metadata: true, + jvm_options: vec![], + connection_timeout_ms: 30000, + }; + let source = JdbcSource::new(1, config, None); + + let mut row = serde_json::Map::new(); + row.insert( + "updated_at".to_string(), + serde_json::json!("2024-06-15 12:00:00"), + ); + assert_eq!( + source.extract_offset_value(&row, "updated_at"), + Some("2024-06-15 12:00:00".to_string()) + ); + } + + #[test] + fn test_extract_offset_value_with_float() { + let config = JdbcSourceConfig { + jdbc_url: SecretString::from("jdbc:h2:mem:test"), + driver_class: "org.h2.Driver".to_string(), + driver_jar_path: "/tmp/h2.jar".to_string(), + username: None, + password: None, + query: "SELECT 1".to_string(), + poll_interval: Duration::from_secs(10), + batch_size: 100, + tracking_column: None, + initial_offset: None, + mode: Mode::Bulk, + snake_case_columns: false, + include_metadata: true, + jvm_options: vec![], + connection_timeout_ms: 30000, + }; + let source = JdbcSource::new(1, config, None); + + let mut row = serde_json::Map::new(); + row.insert("version".to_string(), serde_json::json!(3.5)); + assert_eq!( + source.extract_offset_value(&row, "version"), + Some("3.5".to_string()) + ); + } + + #[test] + fn test_extract_offset_value_with_null() { + let config = JdbcSourceConfig { + jdbc_url: SecretString::from("jdbc:h2:mem:test"), + driver_class: "org.h2.Driver".to_string(), + driver_jar_path: "/tmp/h2.jar".to_string(), + username: None, + password: None, + query: "SELECT 1".to_string(), + poll_interval: Duration::from_secs(10), + batch_size: 100, + tracking_column: None, + initial_offset: None, + mode: Mode::Bulk, + snake_case_columns: false, + include_metadata: true, + jvm_options: vec![], + connection_timeout_ms: 30000, + }; + let source = JdbcSource::new(1, config, None); + + let mut row = serde_json::Map::new(); + row.insert("id".to_string(), serde_json::Value::Null); + // A SQL NULL tracking value must not become the persisted offset. + assert_eq!(source.extract_offset_value(&row, "id"), None); + } + + #[test] + fn test_extract_offset_value_missing_column() { + let config = JdbcSourceConfig { + jdbc_url: SecretString::from("jdbc:h2:mem:test"), + driver_class: "org.h2.Driver".to_string(), + driver_jar_path: "/tmp/h2.jar".to_string(), + username: None, + password: None, + query: "SELECT 1".to_string(), + poll_interval: Duration::from_secs(10), + batch_size: 100, + tracking_column: None, + initial_offset: None, + mode: Mode::Bulk, + snake_case_columns: false, + include_metadata: true, + jvm_options: vec![], + connection_timeout_ms: 30000, + }; + let source = JdbcSource::new(1, config, None); + + let row = serde_json::Map::new(); + assert_eq!(source.extract_offset_value(&row, "nonexistent"), None); + } + + // ========================================================================= + // build_query edge case tests + // ========================================================================= + + #[test] + fn test_build_query_incremental_no_offset_no_initial_removes_where_clause() { + let config = JdbcSourceConfig { + jdbc_url: SecretString::from("jdbc:h2:mem:test"), + driver_class: "org.h2.Driver".to_string(), + driver_jar_path: "/tmp/h2.jar".to_string(), + username: None, + password: None, + query: "SELECT * FROM users WHERE {tracking_column} > {last_offset} ORDER BY id" + .to_string(), + poll_interval: Duration::from_secs(10), + batch_size: 100, + tracking_column: Some("id".to_string()), + initial_offset: None, + mode: Mode::Incremental, + snake_case_columns: false, + include_metadata: true, + jvm_options: vec![], + connection_timeout_ms: 30000, + }; + let source = JdbcSource::new(1, config, None); + let state = State { + last_offset: None, + processed_rows: 0, + last_poll_time: Utc::now(), + }; + let query = source.build_query(&state).expect("build query"); + // The WHERE clause placeholder should be removed + assert!( + !query.contains("{last_offset}"), + "Query should not contain unresolved placeholder: {}", + query + ); + } + + #[test] + fn test_build_query_rejects_unresolved_placeholder() { + // Incremental, no offset, and a non-canonical predicate that the + // auto-remove does not match: the unresolved {last_offset} must produce + // an error rather than being shipped to the driver as invalid SQL. + let config = JdbcSourceConfig { + jdbc_url: SecretString::from("jdbc:h2:mem:test"), + driver_class: "org.h2.Driver".to_string(), + driver_jar_path: "/tmp/h2.jar".to_string(), + username: None, + password: None, + query: "SELECT * FROM t WHERE id >= {last_offset}".to_string(), + poll_interval: Duration::from_secs(10), + batch_size: 100, + tracking_column: None, + initial_offset: None, + mode: Mode::Incremental, + snake_case_columns: false, + include_metadata: true, + jvm_options: vec![], + connection_timeout_ms: 30000, + }; + let source = JdbcSource::new(1, config, None); + assert!(source.build_query(&State::default()).is_err()); + } + + #[test] + fn test_build_query_bulk_mode_ignores_offset() { + let config = JdbcSourceConfig { + jdbc_url: SecretString::from("jdbc:h2:mem:test"), + driver_class: "org.h2.Driver".to_string(), + driver_jar_path: "/tmp/h2.jar".to_string(), + username: None, + password: None, + query: "SELECT * FROM users ORDER BY id".to_string(), + poll_interval: Duration::from_secs(10), + batch_size: 100, + tracking_column: Some("id".to_string()), + initial_offset: Some("0".to_string()), + mode: Mode::Bulk, + snake_case_columns: false, + include_metadata: true, + jvm_options: vec![], + connection_timeout_ms: 30000, + }; + let source = JdbcSource::new(1, config, None); + let state = State { + last_offset: Some("42".to_string()), + processed_rows: 42, + last_poll_time: Utc::now(), + }; + // In bulk mode the query is used verbatim; the tracked offset is ignored. + let query = source.build_query(&state).expect("build query"); + assert_eq!(query, "SELECT * FROM users ORDER BY id"); + } + + // ========================================================================= + // Mode enum tests + // ========================================================================= + + #[test] + fn test_mode_serialization_roundtrip() { + let incremental = Mode::Incremental; + let serialized = serde_json::to_string(&incremental).unwrap(); + assert_eq!(serialized, r#""incremental""#); + let deserialized: Mode = serde_json::from_str(&serialized).unwrap(); + assert_eq!(deserialized, Mode::Incremental); + + let bulk = Mode::Bulk; + let serialized = serde_json::to_string(&bulk).unwrap(); + assert_eq!(serialized, r#""bulk""#); + let deserialized: Mode = serde_json::from_str(&serialized).unwrap(); + assert_eq!(deserialized, Mode::Bulk); + } + + #[test] + fn test_mode_deserialization_rejects_unknown() { + let result = serde_json::from_str::(r#""streaming""#); + assert!( + result.is_err(), + "Unknown mode 'streaming' should fail deserialization" + ); + } + + // ========================================================================= + // Debug impl tests (ensures secrets are not leaked) + // ========================================================================= + + #[test] + fn test_config_debug_does_not_leak_password() { + let config = JdbcSourceConfig { + jdbc_url: SecretString::from("jdbc:mysql://root:SuperSecret@localhost/db"), + driver_class: "com.mysql.cj.jdbc.Driver".to_string(), + driver_jar_path: "/tmp/mysql.jar".to_string(), + username: Some("admin".to_string()), + password: Some(SecretString::from("MyP@ssw0rd")), + query: "SELECT 1".to_string(), + poll_interval: Duration::from_secs(10), + batch_size: 100, + tracking_column: None, + initial_offset: None, + mode: Mode::Bulk, + snake_case_columns: false, + include_metadata: true, + jvm_options: vec![], + connection_timeout_ms: 30000, + }; + + let debug_output = format!("{:?}", config); + assert!( + !debug_output.contains("SuperSecret"), + "Debug output should not contain JDBC URL password: {}", + debug_output + ); + assert!( + !debug_output.contains("MyP@ssw0rd"), + "Debug output should not contain password field: {}", + debug_output + ); + assert!( + debug_output.contains("***"), + "Debug output should contain masked password: {}", + debug_output + ); + } + + #[test] + fn test_config_debug_without_password() { + let config = JdbcSourceConfig { + jdbc_url: SecretString::from("jdbc:h2:mem:test"), + driver_class: "org.h2.Driver".to_string(), + driver_jar_path: "/tmp/h2.jar".to_string(), + username: None, + password: None, + query: "SELECT 1".to_string(), + poll_interval: Duration::from_secs(10), + batch_size: 100, + tracking_column: None, + initial_offset: None, + mode: Mode::Bulk, + snake_case_columns: false, + include_metadata: true, + jvm_options: vec![], + connection_timeout_ms: 30000, + }; + + let debug_output = format!("{:?}", config); + // Should not panic and should contain the struct name + assert!(debug_output.contains("JdbcSourceConfig")); + } +} diff --git a/core/integration/tests/connectors/jdbc/config_postgres.toml b/core/integration/tests/connectors/jdbc/config_postgres.toml new file mode 100644 index 0000000000..19097f5988 --- /dev/null +++ b/core/integration/tests/connectors/jdbc/config_postgres.toml @@ -0,0 +1,22 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# JDBC Source Connector Runtime Configuration for Postgres + +[connectors] +config_type = "local" +config_dir = "tests/connectors/jdbc/connectors_config_postgres" diff --git a/core/integration/tests/connectors/jdbc/connectors_config_postgres/jdbc_pg.toml b/core/integration/tests/connectors/jdbc/connectors_config_postgres/jdbc_pg.toml new file mode 100644 index 0000000000..0f35c4c4c1 --- /dev/null +++ b/core/integration/tests/connectors/jdbc/connectors_config_postgres/jdbc_pg.toml @@ -0,0 +1,49 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# JDBC Source Connector for PostgreSQL + +type = "source" +key = "jdbc_pg" +enabled = true +version = 0 +name = "JDBC PostgreSQL Source" +path = "../../target/debug/libiggy_connector_jdbc_source" + +[[streams]] +stream = "test_stream" +topic = "test_topic" +schema = "json" +partition_id = 1 + +[plugin_config] +# All required fields - values will be overridden by environment variables +# Use placeholder values that match the expected types +jdbc_url = "" +driver_class = "" +driver_jar_path = "" +username = "" +password = "" +query = "" +poll_interval = "1s" +batch_size = 100 +mode = "bulk" +# Used only by incremental-mode tests; ignored in bulk mode. +tracking_column = "id" +initial_offset = "0" +snake_case_columns = false +include_metadata = true diff --git a/core/integration/tests/connectors/jdbc/mod.rs b/core/integration/tests/connectors/jdbc/mod.rs new file mode 100644 index 0000000000..2d1214308a --- /dev/null +++ b/core/integration/tests/connectors/jdbc/mod.rs @@ -0,0 +1,21 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +// JDBC connector tests. +// Source PostgreSQL tests: test_with_postgres.rs +// Sink PostgreSQL tests: test_sink_with_postgres.rs +mod test_with_postgres; diff --git a/core/integration/tests/connectors/jdbc/test_with_postgres.rs b/core/integration/tests/connectors/jdbc/test_with_postgres.rs new file mode 100644 index 0000000000..6004bf80fe --- /dev/null +++ b/core/integration/tests/connectors/jdbc/test_with_postgres.rs @@ -0,0 +1,624 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use crate::connectors::{ConnectorsRuntime, IggySetup, setup_runtime}; +use serial_test::serial; +use sqlx::postgres::PgPoolOptions; +use std::collections::HashMap; +use std::time::Duration; +use testcontainers_modules::postgres::Postgres; +use testcontainers_modules::testcontainers::ContainerAsync; +use testcontainers_modules::testcontainers::runners::AsyncRunner; +use tokio::time::sleep; +use tracing::info; + +const POSTGRES_USER: &str = "postgres"; +const POSTGRES_PASSWORD: &str = "postgres"; +const POSTGRES_DB: &str = "postgres"; + +/// Maximum number of poll attempts before giving up +const POLL_ATTEMPTS: usize = 30; +/// Delay between poll attempts +const POLL_INTERVAL: Duration = Duration::from_millis(500); + +/// Setup Postgres container with test data +async fn setup_postgres_container() +-> Result<(ContainerAsync, String, String), Box> { + info!("Starting Postgres container for JDBC testing..."); + + let postgres = Postgres::default().start().await?; + + let host = postgres.get_host().await?; + let port = postgres.get_host_port_ipv4(5432).await?; + let jdbc_url: String = format!("jdbc:postgresql://{}:{}/{}", host, port, POSTGRES_DB); + + let postgres_jar: String = get_postgres_driver_jar().await?; + + info!("Postgres container started at {}:{}", host, port); + Ok((postgres, jdbc_url, postgres_jar)) +} + +/// Get PostgreSQL JDBC driver, downloading if necessary +async fn get_postgres_driver_jar() -> Result> { + let target_dir = std::env::var("CARGO_TARGET_DIR").unwrap_or_else(|_| "target".to_string()); + let jdbc_test_dir = format!("{}/test-jdbc-drivers", target_dir); + let jar_path = format!("{}/postgresql-42.7.1.jar", jdbc_test_dir); + + std::fs::create_dir_all(&jdbc_test_dir)?; + + if std::path::Path::new(&jar_path).exists() { + info!("PostgreSQL JDBC driver found at {}", jar_path); + let absolute_path = std::fs::canonicalize(&jar_path)? + .to_string_lossy() + .to_string(); + return Ok(absolute_path); + } + + info!("Downloading PostgreSQL JDBC driver..."); + let download_url = "https://jdbc.postgresql.org/download/postgresql-42.7.1.jar"; + + let response = reqwest::get(download_url).await?; + if !response.status().is_success() { + return Err(format!("Failed to download driver: HTTP {}", response.status()).into()); + } + + let bytes = response.bytes().await?; + std::fs::write(&jar_path, bytes)?; + + info!("PostgreSQL JDBC driver downloaded to {}", jar_path); + let absolute_path = std::fs::canonicalize(&jar_path)? + .to_string_lossy() + .to_string(); + Ok(absolute_path) +} + +/// Build the environment variables for a JDBC Postgres source connector. +fn build_jdbc_env( + jdbc_url: &str, + postgres_jar: &str, + query: &str, + mode: &str, + iggy_setup: &IggySetup, +) -> HashMap { + let mut envs = HashMap::new(); + + envs.insert( + "IGGY_CONNECTORS_SOURCE_JDBC_PG_PLUGIN_CONFIG_JDBC_URL".to_owned(), + jdbc_url.to_owned(), + ); + envs.insert( + "IGGY_CONNECTORS_SOURCE_JDBC_PG_PLUGIN_CONFIG_DRIVER_CLASS".to_owned(), + "org.postgresql.Driver".to_owned(), + ); + envs.insert( + "IGGY_CONNECTORS_SOURCE_JDBC_PG_PLUGIN_CONFIG_DRIVER_JAR_PATH".to_owned(), + postgres_jar.to_owned(), + ); + envs.insert( + "IGGY_CONNECTORS_SOURCE_JDBC_PG_PLUGIN_CONFIG_USERNAME".to_owned(), + POSTGRES_USER.to_owned(), + ); + envs.insert( + "IGGY_CONNECTORS_SOURCE_JDBC_PG_PLUGIN_CONFIG_PASSWORD".to_owned(), + POSTGRES_PASSWORD.to_owned(), + ); + envs.insert( + "IGGY_CONNECTORS_SOURCE_JDBC_PG_PLUGIN_CONFIG_QUERY".to_owned(), + query.to_owned(), + ); + envs.insert( + "IGGY_CONNECTORS_SOURCE_JDBC_PG_PLUGIN_CONFIG_POLL_INTERVAL".to_owned(), + "1s".to_owned(), + ); + envs.insert( + "IGGY_CONNECTORS_SOURCE_JDBC_PG_PLUGIN_CONFIG_BATCH_SIZE".to_owned(), + "100".to_owned(), + ); + envs.insert( + "IGGY_CONNECTORS_SOURCE_JDBC_PG_PLUGIN_CONFIG_MODE".to_owned(), + mode.to_owned(), + ); + envs.insert( + "IGGY_CONNECTORS_SOURCE_JDBC_PG_PLUGIN_CONFIG_SNAKE_CASE_COLUMNS".to_owned(), + "false".to_owned(), + ); + envs.insert( + "IGGY_CONNECTORS_SOURCE_JDBC_PG_PLUGIN_CONFIG_INCLUDE_METADATA".to_owned(), + "true".to_owned(), + ); + + // Stream configuration + envs.insert( + "IGGY_CONNECTORS_SOURCE_JDBC_PG_STREAMS_0_STREAM".to_owned(), + iggy_setup.stream.to_owned(), + ); + envs.insert( + "IGGY_CONNECTORS_SOURCE_JDBC_PG_STREAMS_0_TOPIC".to_owned(), + iggy_setup.topic.to_owned(), + ); + envs.insert( + "IGGY_CONNECTORS_SOURCE_JDBC_PG_STREAMS_0_SCHEMA".to_owned(), + "json".to_owned(), + ); + + envs +} + +/// Poll messages from Iggy with retry logic, returning deserialized JSON values. +async fn poll_messages_with_retry( + client: &crate::connectors::ConnectorsIggyClient, + expected_count: usize, +) -> Vec { + let mut received: Vec = Vec::new(); + for attempt in 0..POLL_ATTEMPTS { + let polled_messages = client + .get_messages() + .await + .expect("Failed to poll messages"); + + for msg in &polled_messages.messages { + if let Ok(value) = serde_json::from_slice::(&msg.payload) { + received.push(value); + } + } + + if received.len() >= expected_count { + info!( + "Received {} messages after {} attempts", + received.len(), + attempt + 1 + ); + return received; + } + + sleep(POLL_INTERVAL).await; + } + + received +} + +/// Setup connector runtime with JDBC source for Postgres +async fn setup_jdbc_postgres_source( + jdbc_url: &str, + postgres_jar: &str, + query: &str, + mode: &str, +) -> Result< + (ConnectorsRuntime, crate::connectors::ConnectorsIggyClient), + Box, +> { + let iggy_setup = IggySetup::default(); + let envs = build_jdbc_env(jdbc_url, postgres_jar, query, mode, &iggy_setup); + + let mut runtime = setup_runtime(); + runtime + .init("jdbc/config_postgres.toml", Some(envs), iggy_setup) + .await; + + let client = runtime.create_client().await; + Ok((runtime, client)) +} + +/// Test: basic bulk mode query produces messages with correct structure +#[tokio::test] +#[serial] +async fn bulk_query_produces_message_to_iggy() { + let (_postgres_container, jdbc_url, postgres_jar) = match setup_postgres_container().await { + Ok(result) => result, + Err(e) => { + eprintln!("Skipping test: Failed to setup Postgres: {}", e); + return; + } + }; + + let query = "SELECT 1 as id, 'test' as name"; + let (_runtime, client) = setup_jdbc_postgres_source(&jdbc_url, &postgres_jar, query, "bulk") + .await + .expect("Failed to setup runtime"); + + info!("Waiting for JDBC connector to poll from Postgres..."); + let messages = poll_messages_with_retry(&client, 1).await; + + assert!( + !messages.is_empty(), + "Expected at least 1 message from JDBC Postgres source" + ); + + // Verify message structure: should have metadata wrapping (include_metadata=true) + let first = &messages[0]; + assert!( + first.get("data").is_some(), + "Expected 'data' field in message (include_metadata=true), got: {}", + first + ); + assert_eq!( + first.get("operation_type").and_then(|v| v.as_str()), + Some("SELECT"), + "Expected operation_type=SELECT" + ); + + // Verify the actual data content + let data = first.get("data").unwrap(); + assert_eq!( + data.get("id").and_then(|v| v.as_i64()), + Some(1), + "Expected id=1 in data" + ); + assert_eq!( + data.get("name").and_then(|v| v.as_str()), + Some("test"), + "Expected name='test' in data" + ); +} + +/// Test: bulk mode with multiple rows from an actual table +#[tokio::test] +#[serial] +async fn bulk_query_produces_multiple_rows_to_iggy() { + let (postgres_container, jdbc_url, postgres_jar) = match setup_postgres_container().await { + Ok(result) => result, + Err(e) => { + eprintln!("Skipping test: Failed to setup Postgres: {}", e); + return; + } + }; + + // Use a multi-row SELECT to simulate table data without needing DDL + let query = r#" + SELECT * FROM (VALUES + (1, 'alice', true), + (2, 'bob', false), + (3, 'carol', true) + ) AS t(id, name, active) + "#; + + let (_runtime, client) = setup_jdbc_postgres_source(&jdbc_url, &postgres_jar, query, "bulk") + .await + .expect("Failed to setup runtime"); + + info!("Waiting for JDBC connector to poll multiple rows..."); + let messages = poll_messages_with_retry(&client, 3).await; + + assert!( + messages.len() >= 3, + "Expected at least 3 messages, got {}", + messages.len() + ); + + // Verify each row has the expected structure + for msg in &messages[..3] { + let data = msg.get("data").expect("Missing 'data' field"); + assert!(data.get("id").is_some(), "Missing 'id' column in row data"); + assert!( + data.get("name").is_some(), + "Missing 'name' column in row data" + ); + assert!( + data.get("active").is_some(), + "Missing 'active' column in row data" + ); + } + + // Verify specific values for the first row + let first_data = messages[0].get("data").unwrap(); + assert_eq!(first_data.get("id").and_then(|v| v.as_i64()), Some(1)); + assert_eq!( + first_data.get("name").and_then(|v| v.as_str()), + Some("alice") + ); + + // Keep container alive until assertions complete + drop(postgres_container); +} + +/// Test: message contains timestamp field when metadata is enabled +#[tokio::test] +#[serial] +async fn source_includes_metadata_fields_when_enabled() { + let (_postgres_container, jdbc_url, postgres_jar) = match setup_postgres_container().await { + Ok(result) => result, + Err(e) => { + eprintln!("Skipping test: Failed to setup Postgres: {}", e); + return; + } + }; + + let query = "SELECT 42 as value"; + let (_runtime, client) = setup_jdbc_postgres_source(&jdbc_url, &postgres_jar, query, "bulk") + .await + .expect("Failed to setup runtime"); + + let messages = poll_messages_with_retry(&client, 1).await; + assert!(!messages.is_empty(), "Expected at least 1 message"); + + let msg = &messages[0]; + + // Verify all metadata fields are present + assert!( + msg.get("timestamp").is_some(), + "Missing 'timestamp' metadata field" + ); + assert!( + msg.get("operation_type").is_some(), + "Missing 'operation_type' metadata field" + ); + assert!(msg.get("data").is_some(), "Missing 'data' metadata field"); + + // table_name should be null for SELECT queries without a specific table + // (this is expected behavior for computed queries) + assert!( + msg.get("table_name").is_some(), + "Missing 'table_name' metadata field" + ); +} + +/// Derive a sqlx (`postgres://`) URL from the connector's JDBC URL so the test +/// can seed the table the source reads from. +fn pg_sqlx_url(jdbc_url: &str) -> String { + let host_and_db = jdbc_url + .strip_prefix("jdbc:postgresql://") + .unwrap_or(jdbc_url); + format!("postgres://{POSTGRES_USER}:{POSTGRES_PASSWORD}@{host_and_db}") +} + +/// Collect the `data.id` integer from each polled (metadata-wrapped) message. +fn collect_ids(messages: &[serde_json::Value]) -> Vec { + messages + .iter() + .filter_map(|m| { + m.get("data") + .and_then(|d| d.get("id")) + .and_then(|v| v.as_i64()) + }) + .collect() +} + +/// Test: incremental mode advances its tracking offset across polls; newly +/// inserted rows are delivered exactly once and previously read rows are not +/// re-delivered. +#[tokio::test] +#[serial] +async fn incremental_mode_advances_offset_across_polls() { + let (_container, jdbc_url, postgres_jar) = match setup_postgres_container().await { + Ok(result) => result, + Err(e) => { + eprintln!("Skipping test: Failed to setup Postgres: {e}"); + return; + } + }; + + // Seed a real table BEFORE the source starts polling. + let pool = PgPoolOptions::new() + .max_connections(2) + .connect(&pg_sqlx_url(&jdbc_url)) + .await + .expect("Failed to connect to Postgres for seeding"); + sqlx::query("CREATE TABLE inc_test (id INT PRIMARY KEY, name TEXT)") + .execute(&pool) + .await + .expect("Failed to create table"); + sqlx::query("INSERT INTO inc_test (id, name) VALUES (1, 'a'), (2, 'b'), (3, 'c')") + .execute(&pool) + .await + .expect("Failed to insert initial rows"); + + let query = "SELECT id, name FROM inc_test WHERE id > {last_offset} ORDER BY id"; + let (_runtime, client) = + setup_jdbc_postgres_source(&jdbc_url, &postgres_jar, query, "incremental") + .await + .expect("Failed to setup runtime"); + + // First batch: ids 1..3. + let first = poll_messages_with_retry(&client, 3).await; + let mut first_ids = collect_ids(&first); + first_ids.sort_unstable(); + assert_eq!(first_ids, vec![1, 2, 3], "Expected ids 1,2,3 on first poll"); + + // Insert more rows; only these (id > last_offset) should arrive next. + sqlx::query("INSERT INTO inc_test (id, name) VALUES (4, 'd'), (5, 'e')") + .execute(&pool) + .await + .expect("Failed to insert additional rows"); + + let second = poll_messages_with_retry(&client, 2).await; + let mut second_ids = collect_ids(&second); + second_ids.sort_unstable(); + assert_eq!( + second_ids, + vec![4, 5], + "Expected only the new ids 4,5 (offset must have advanced past 3), got {second_ids:?}" + ); +} + +/// Test: a single poll over many rows succeeds. This exercises the JNI +/// local-reference frame management in `read_rows`: a few-hundred-row result set +/// creates hundreds of per-column local references in one native call, which +/// would overflow the JNI local reference table (and abort the JVM) if each row +/// were not read inside its own local frame. +#[tokio::test] +#[serial] +async fn large_result_set_streams_without_crashing() { + let (_container, jdbc_url, postgres_jar) = match setup_postgres_container().await { + Ok(result) => result, + Err(e) => { + eprintln!("Skipping test: Failed to setup Postgres: {e}"); + return; + } + }; + + let pool = PgPoolOptions::new() + .max_connections(2) + .connect(&pg_sqlx_url(&jdbc_url)) + .await + .expect("Failed to connect to Postgres for seeding"); + sqlx::query("CREATE TABLE big_test (id INT PRIMARY KEY, name TEXT, val NUMERIC(12,2))") + .execute(&pool) + .await + .expect("Failed to create table"); + sqlx::query( + "INSERT INTO big_test (id, name, val) \ + SELECT g, 'row_' || g, (g * 1.5)::numeric(12,2) FROM generate_series(1, 300) g", + ) + .execute(&pool) + .await + .expect("Failed to insert rows"); + + // batch_size well above the row count so the whole table is read in a single + // poll (one read_rows call → hundreds of local refs). + let iggy_setup = IggySetup::default(); + let query = "SELECT id, name, val FROM big_test ORDER BY id"; + let mut envs = build_jdbc_env(&jdbc_url, &postgres_jar, query, "bulk", &iggy_setup); + envs.insert( + "IGGY_CONNECTORS_SOURCE_JDBC_PG_PLUGIN_CONFIG_BATCH_SIZE".to_owned(), + "5000".to_owned(), + ); + + let mut runtime = setup_runtime(); + runtime + .init("jdbc/config_postgres.toml", Some(envs), iggy_setup) + .await; + let client = runtime.create_client().await; + + // The runtime would have crashed on the oversized poll without per-row local + // frames; receiving a healthy batch of well-formed messages proves it did not. + let messages = poll_messages_with_retry(&client, 150).await; + assert!( + messages.len() >= 150, + "Expected the source to stream a large result set without crashing; got {} messages", + messages.len() + ); + for msg in &messages[..150] { + let data = msg.get("data").expect("Missing 'data' field"); + assert!(data.get("id").and_then(|v| v.as_i64()).is_some()); + assert!(data.get("name").and_then(|v| v.as_str()).is_some()); + } +} + +/// Test: the source keeps polling and recovers after a query that raises a +/// SQLException on every poll. The query targets a table that does not exist +/// yet, so `executeQuery` throws each cycle; once the table is created the very +/// next poll must succeed and deliver its rows. +/// +/// This is the regression guard for exception clearing: a thrown Java exception +/// left pending would make the following JNI call (the statement `close()` in +/// the error path, or the next poll's `isValid`) run with an exception pending, +/// which the JNI spec forbids and which aborts the embedded JVM (the whole +/// runtime process). If that happened the runtime would die and never deliver +/// the post-recovery rows below. +#[tokio::test] +#[serial] +async fn source_recovers_after_repeated_query_errors() { + let (_container, jdbc_url, postgres_jar) = match setup_postgres_container().await { + Ok(result) => result, + Err(e) => { + eprintln!("Skipping test: Failed to setup Postgres: {e}"); + return; + } + }; + + // Start the source against a table that does not exist yet: every poll + // raises "relation does not exist" (SQLState 42P01). + let query = "SELECT id, name FROM recover_test ORDER BY id"; + let (_runtime, client) = setup_jdbc_postgres_source(&jdbc_url, &postgres_jar, query, "bulk") + .await + .expect("Failed to setup runtime"); + + // Let the source fail across several poll cycles (poll interval is 1s). + sleep(Duration::from_secs(4)).await; + + // Now create and seed the table; the next successful poll should deliver it. + let pool = PgPoolOptions::new() + .max_connections(2) + .connect(&pg_sqlx_url(&jdbc_url)) + .await + .expect("Failed to connect to Postgres for seeding"); + sqlx::query("CREATE TABLE recover_test (id INT PRIMARY KEY, name TEXT)") + .execute(&pool) + .await + .expect("Failed to create table"); + sqlx::query("INSERT INTO recover_test (id, name) VALUES (1, 'a'), (2, 'b')") + .execute(&pool) + .await + .expect("Failed to insert rows"); + + let messages = poll_messages_with_retry(&client, 2).await; + let mut ids = collect_ids(&messages); + ids.sort_unstable(); + assert_eq!( + ids, + vec![1, 2], + "Source must recover after repeated query failures and deliver ids 1,2, got {ids:?}" + ); +} + +/// Test: bulk mode fails closed when the result set is larger than batch_size, +/// rather than silently syncing a truncated subset. With batch_size below the +/// row count the source errors every poll and delivers nothing (in particular, +/// never a truncated partial set). +#[tokio::test] +#[serial] +async fn bulk_result_larger_than_batch_size_fails_closed() { + let (_container, jdbc_url, postgres_jar) = match setup_postgres_container().await { + Ok(result) => result, + Err(e) => { + eprintln!("Skipping test: Failed to setup Postgres: {e}"); + return; + } + }; + + let pool = PgPoolOptions::new() + .max_connections(2) + .connect(&pg_sqlx_url(&jdbc_url)) + .await + .expect("Failed to connect to Postgres for seeding"); + sqlx::query("CREATE TABLE trunc_test (id INT PRIMARY KEY)") + .execute(&pool) + .await + .expect("Failed to create table"); + sqlx::query("INSERT INTO trunc_test (id) SELECT generate_series(1, 5)") + .execute(&pool) + .await + .expect("Failed to insert rows"); + + // Bulk mode with batch_size below the 5-row result set. + let iggy_setup = IggySetup::default(); + let query = "SELECT id FROM trunc_test ORDER BY id"; + let mut envs = build_jdbc_env(&jdbc_url, &postgres_jar, query, "bulk", &iggy_setup); + envs.insert( + "IGGY_CONNECTORS_SOURCE_JDBC_PG_PLUGIN_CONFIG_BATCH_SIZE".to_owned(), + "2".to_owned(), + ); + + let mut runtime = setup_runtime(); + runtime + .init("jdbc/config_postgres.toml", Some(envs), iggy_setup) + .await; + let client = runtime.create_client().await; + + // Several poll cycles (poll interval is 1s). A fail-closed source delivers + // nothing, and in particular never the truncated 2-row subset. + sleep(Duration::from_secs(4)).await; + let polled = client + .get_messages() + .await + .expect("Failed to poll messages"); + assert!( + polled.messages.is_empty(), + "bulk truncation must fail closed and deliver nothing, got {} messages", + polled.messages.len() + ); +} diff --git a/core/integration/tests/connectors/mod.rs b/core/integration/tests/connectors/mod.rs index a1433160b9..fb1cc67afb 100644 --- a/core/integration/tests/connectors/mod.rs +++ b/core/integration/tests/connectors/mod.rs @@ -25,6 +25,7 @@ mod http; mod http_config_provider; mod iceberg; mod influxdb; +mod jdbc; mod mongodb; mod postgres; mod quickwit; @@ -35,8 +36,44 @@ mod s3; mod stdout; mod surrealdb; -use iggy_common::IggyTimestamp; +use iggy::prelude::{IggyClient, IggyMessage, Partitioning}; +use iggy_common::Client; +use iggy_common::{ + CompressionAlgorithm, IggyExpiry, IggyTimestamp, MaxTopicSize, MessageClient, PolledMessages, + StreamClient, TopicClient, +}; +use integration::harness::{ConnectorsRuntimeConfig, IpAddrKind, TestHarness, TestServerConfig}; use serde::{Deserialize, Serialize}; +use std::collections::HashMap; + +const DEFAULT_TEST_STREAM: &str = "test_stream"; +const DEFAULT_TEST_TOPIC: &str = "test_topic"; + +fn setup_runtime() -> ConnectorsRuntime { + ConnectorsRuntime { + harness: TestHarness::builder() + .server( + TestServerConfig::builder() + .ip_kind(IpAddrKind::V4) + .quic_enabled(false) + .http_enabled(false) + .websocket_enabled(false) + .extra_envs(HashMap::from([ + // The harness pre-reserves a fixed TCP port (see PortReserver), + // so the server binds a non-zero port. That relies on the server + // writing current_config.toml on bind regardless of how the port + // was chosen (see tcp_listener.rs); the harness reads that file to + // discover the bound address before it considers startup complete. + ("IGGY_TCP_ADDRESS".to_owned(), "127.0.0.1:0".to_owned()), + ])) + .build(), + ) + .build() + .unwrap(), + stream: "".to_owned(), + topic: "".to_owned(), + } +} const ONE_DAY_MICROS: u64 = 24 * 60 * 60 * 1_000_000; @@ -63,3 +100,154 @@ pub fn create_test_messages(count: usize) -> Vec { }) .collect() } + +#[derive(Debug)] +struct ConnectorsRuntime { + stream: String, + topic: String, + harness: TestHarness, +} + +#[derive(Debug)] +struct ConnectorsIggyClient { + stream: String, + topic: String, + client: IggyClient, +} + +impl ConnectorsIggyClient { + /// Send messages to the configured stream/topic (used by sink connector tests). + #[allow(dead_code)] + async fn send_messages( + &self, + messages: &mut [IggyMessage], + ) -> Result<(), iggy_common::IggyError> { + self.client + .send_messages( + &self.stream.clone().try_into().unwrap(), + &self.topic.clone().try_into().unwrap(), + &Partitioning::balanced(), + messages, + ) + .await + } + + async fn get_messages(&self) -> Result { + self.client + .poll_messages( + &self.stream.clone().try_into().unwrap(), + &self.topic.clone().try_into().unwrap(), + None, + &iggy_common::Consumer::new("test_consumer".try_into().unwrap()), + &iggy_common::PollingStrategy::next(), + 10, + true, + ) + .await + } +} + +#[derive(Debug)] +pub struct IggySetup { + pub stream: String, + pub topic: String, +} + +impl Default for IggySetup { + fn default() -> Self { + Self { + stream: DEFAULT_TEST_STREAM.to_owned(), + topic: DEFAULT_TEST_TOPIC.to_owned(), + } + } +} + +impl ConnectorsRuntime { + pub async fn init( + &mut self, + config_path: &str, + envs: Option>, + iggy_setup: IggySetup, + ) { + let config_path = format!("tests/connectors/{config_path}"); + let mut all_envs = HashMap::new(); + all_envs.insert( + "IGGY_CONNECTORS_CONFIG_PATH".to_owned(), + config_path.to_owned(), + ); + + if let Some(envs) = envs { + for (k, v) in envs { + all_envs.insert(k, v); + } + } + + // Start the iggy server + self.harness + .start() + .await + .expect("Failed to start test harness"); + + let client = self.create_iggy_client().await; + client + .create_stream(&iggy_setup.stream) + .await + .expect("Failed to create stream"); + let stream_id = iggy_setup + .stream + .clone() + .try_into() + .expect("Invalid stream name in Iggy setup"); + client + .create_topic( + &stream_id, + &iggy_setup.topic, + 1, + CompressionAlgorithm::None, + None, + IggyExpiry::ServerDefault, + MaxTopicSize::ServerDefault, + ) + .await + .expect("Failed to create topic"); + client.shutdown().await.expect("Failed to shutdown client"); + + let connectors_config = ConnectorsRuntimeConfig::builder() + .extra_envs(all_envs) + .build(); + + self.harness + .server_mut() + .set_connectors_runtime_config(connectors_config); + self.harness + .server_mut() + .start_dependents() + .await + .expect("Failed to start connectors runtime"); + + self.stream = iggy_setup.stream; + self.topic = iggy_setup.topic; + } + + pub async fn create_client(&self) -> ConnectorsIggyClient { + ConnectorsIggyClient { + stream: self.stream.clone(), + topic: self.topic.clone(), + client: self.create_iggy_client().await, + } + } + + async fn create_iggy_client(&self) -> IggyClient { + self.harness + .tcp_root_client() + .await + .expect("Failed to create root TCP client") + } + + pub fn connectors_api_address(&self) -> Option { + self.harness + .server() + .connectors_runtime() + .map(|cr| cr.http_address().to_string()) + } +} diff --git a/core/server/src/tcp/tcp_listener.rs b/core/server/src/tcp/tcp_listener.rs index 3f53e4e640..c805f28d10 100644 --- a/core/server/src/tcp/tcp_listener.rs +++ b/core/server/src/tcp/tcp_listener.rs @@ -73,11 +73,11 @@ pub async fn start( // Store bound address locally shard.tcp_bound_address.set(Some(actual_addr)); - if addr.port() == 0 { - // Notify config writer on shard 0 - let _ = shard.config_writer_notify.try_send(()); + // Always notify config writer so it can write the current_config.toml + let _ = shard.config_writer_notify.try_send(()); - // Broadcast to other shards for SO_REUSEPORT binding + if addr.port() == 0 { + // Broadcast to other shards for SO_REUSEPORT binding (only needed for dynamic ports) let event = ShardEvent::AddressBound { protocol: TransportProtocol::Tcp, address: actual_addr, From 6ecc1845033e5906b52ac6262a0ff3e0bd31c485 Mon Sep 17 00:00:00 2001 From: shbhmrzd Date: Tue, 21 Jul 2026 02:46:07 +0530 Subject: [PATCH 02/11] refactor(connectors): align JDBC source with sibling conventions Aligns the JDBC source crate with the other source connectors: crate version/type and publish flag, and poll_interval parsing via the workspace humantime crate rather than a direct humantime-serde pin. Removes a redundant per-column JNI getObject probe (using wasNull after the typed getter instead) and caps the per-poll liveness check at five seconds so a dead connection cannot stall a shared worker. Drops the unused partition_id field from the example configs and docs. --- Cargo.lock | 14 +- .../connectors/jdbc_bulk_mode.toml | 1 - .../example_config/connectors/jdbc_h2.toml | 1 - .../example_config/connectors/jdbc_mysql.toml | 1 - .../connectors/jdbc_oracle.toml | 1 - .../connectors/jdbc_sqlserver.toml | 1 - .../connectors/test_jdbc_h2.toml | 1 - .../connectors/sources/jdbc_source/Cargo.toml | 11 +- core/connectors/sources/jdbc_source/README.md | 5 +- .../connectors/sources/jdbc_source/src/lib.rs | 191 +++++++++++++----- .../connectors_config_postgres/jdbc_pg.toml | 1 - 11 files changed, 145 insertions(+), 83 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 31c3666e92..e5491f8778 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6203,16 +6203,6 @@ version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "135b12329e5e3ce057a9f972339ea52bc954fe1e9358ef27f95e89716fbc5424" -[[package]] -name = "humantime-serde" -version = "1.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "57a3db5ea5923d99402c94e9feb261dc5ee9b4efa158b0315f788cf549cc200c" -dependencies = [ - "humantime", - "serde", -] - [[package]] name = "hwlocality" version = "1.0.0-alpha.12" @@ -7036,13 +7026,13 @@ dependencies = [ [[package]] name = "iggy_connector_jdbc_source" -version = "0.1.0" +version = "0.4.1-edge.1" dependencies = [ "async-trait", "base64", "chrono", "dashmap", - "humantime-serde", + "humantime", "iggy_common", "iggy_connector_sdk", "jni 0.21.1", diff --git a/core/connectors/runtime/example_config/connectors/jdbc_bulk_mode.toml b/core/connectors/runtime/example_config/connectors/jdbc_bulk_mode.toml index 25e2b458ae..a179b42a50 100644 --- a/core/connectors/runtime/example_config/connectors/jdbc_bulk_mode.toml +++ b/core/connectors/runtime/example_config/connectors/jdbc_bulk_mode.toml @@ -77,5 +77,4 @@ include_metadata = false [[streams]] stream = "warehouse" topic = "product_summary" -partition_id = 1 schema = "json" diff --git a/core/connectors/runtime/example_config/connectors/jdbc_h2.toml b/core/connectors/runtime/example_config/connectors/jdbc_h2.toml index 82ee459ff5..4dd1f0cc6e 100644 --- a/core/connectors/runtime/example_config/connectors/jdbc_h2.toml +++ b/core/connectors/runtime/example_config/connectors/jdbc_h2.toml @@ -56,5 +56,4 @@ include_metadata = true [[streams]] stream = "test" topic = "users" -partition_id = 1 schema = "json" diff --git a/core/connectors/runtime/example_config/connectors/jdbc_mysql.toml b/core/connectors/runtime/example_config/connectors/jdbc_mysql.toml index fd2ed5fc48..f246781611 100644 --- a/core/connectors/runtime/example_config/connectors/jdbc_mysql.toml +++ b/core/connectors/runtime/example_config/connectors/jdbc_mysql.toml @@ -81,5 +81,4 @@ jvm_options = ["-Xmx512m", "-Xms128m"] [[streams]] stream = "ecommerce" topic = "orders" -partition_id = 1 schema = "json" diff --git a/core/connectors/runtime/example_config/connectors/jdbc_oracle.toml b/core/connectors/runtime/example_config/connectors/jdbc_oracle.toml index 04478bf4ab..92d5a1e99e 100644 --- a/core/connectors/runtime/example_config/connectors/jdbc_oracle.toml +++ b/core/connectors/runtime/example_config/connectors/jdbc_oracle.toml @@ -78,5 +78,4 @@ jvm_options = ["-Xmx512m", "-Xms256m"] [[streams]] stream = "crm" topic = "customers" -partition_id = 1 schema = "json" diff --git a/core/connectors/runtime/example_config/connectors/jdbc_sqlserver.toml b/core/connectors/runtime/example_config/connectors/jdbc_sqlserver.toml index 9e28d08a03..895f4beaf7 100644 --- a/core/connectors/runtime/example_config/connectors/jdbc_sqlserver.toml +++ b/core/connectors/runtime/example_config/connectors/jdbc_sqlserver.toml @@ -62,5 +62,4 @@ connection_timeout_ms = 30000 [[streams]] stream = "sales" topic = "orders" -partition_id = 1 schema = "json" diff --git a/core/connectors/runtime/example_config/connectors/test_jdbc_h2.toml b/core/connectors/runtime/example_config/connectors/test_jdbc_h2.toml index 06b65c21b2..30457d163f 100644 --- a/core/connectors/runtime/example_config/connectors/test_jdbc_h2.toml +++ b/core/connectors/runtime/example_config/connectors/test_jdbc_h2.toml @@ -41,5 +41,4 @@ include_metadata = true [[streams]] stream = "test" topic = "users" -partition_id = 1 schema = "json" diff --git a/core/connectors/sources/jdbc_source/Cargo.toml b/core/connectors/sources/jdbc_source/Cargo.toml index c3eabd9e61..839b7e424a 100644 --- a/core/connectors/sources/jdbc_source/Cargo.toml +++ b/core/connectors/sources/jdbc_source/Cargo.toml @@ -17,19 +17,20 @@ [package] name = "iggy_connector_jdbc_source" -version = "0.1.0" +version = "0.4.1-edge.1" edition = "2024" license = "Apache-2.0" keywords = ["iggy", "messaging", "streaming", "jdbc", "source"] categories = ["database"] description = "Generic JDBC source connector for Iggy - supports MySQL, Oracle, SQL Server, H2, and any JDBC-compliant database" readme = "README.md" +publish = false [package.metadata.cargo-machete] -ignored = ["dashmap", "humantime-serde"] +ignored = ["dashmap"] [lib] -crate-type = ["cdylib", "rlib"] +crate-type = ["cdylib", "lib"] [features] default = [] @@ -42,8 +43,8 @@ chrono = { workspace = true } # Required by source_connector! macro dashmap = { workspace = true } -# For parsing duration strings -humantime-serde = "1.1" +# For parsing duration strings (poll_interval) +humantime = { workspace = true } # Shared serde helpers (SecretString serialization) iggy_common = { workspace = true } diff --git a/core/connectors/sources/jdbc_source/README.md b/core/connectors/sources/jdbc_source/README.md index c2e4d979b4..65bd59edab 100644 --- a/core/connectors/sources/jdbc_source/README.md +++ b/core/connectors/sources/jdbc_source/README.md @@ -99,7 +99,6 @@ include_metadata = true [[streams]] stream = "ecommerce" topic = "orders" -partition_id = 1 ``` ### Bulk Mode Configuration @@ -188,12 +187,12 @@ topic = "orders" | `username` | string | No | - | Database username (optional if in jdbc_url) | | `password` | string | No | - | Database password (optional if in jdbc_url) | | `query` | string | Yes | - | SQL query to execute (supports `{last_offset}` and `{tracking_column}` placeholders) | -| `poll_interval` | duration | Yes | - | How often to poll (e.g., "30s", "5m", "1h") | +| `poll_interval` | string (duration) | No | 5s | How often to poll, as a humantime string (e.g., "30s", "5m", "1h") | | `batch_size` | u32 | No | 1000 | Maximum rows to fetch per poll | | `tracking_column` | string | Incremental | - | Column to track for incremental reads (required in incremental mode; the query must also `ORDER BY` it) | | `initial_offset` | string | No | - | Starting offset value for first poll | | `mode` | string | No | "incremental" | Sync mode: "incremental" or "bulk" (bulk works with ALL databases) | -| `connection_timeout_ms` | u64 | No | 30000 | Timeout (ms) for the per-poll connection liveness check | +| `connection_timeout_ms` | u64 | No | 5000 | Timeout (ms) for the per-poll `isValid` liveness check; converted to seconds and capped at 5s | | `jvm_options` | array | No | [] | Custom JVM options (e.g., ["-Xmx1g"]) | | `snake_case_columns` | bool | No | false | Convert column names to snake_case | | `include_metadata` | bool | No | true | Include metadata (table, operation, timestamp) | diff --git a/core/connectors/sources/jdbc_source/src/lib.rs b/core/connectors/sources/jdbc_source/src/lib.rs index ea19ab0916..c99058a25b 100644 --- a/core/connectors/sources/jdbc_source/src/lib.rs +++ b/core/connectors/sources/jdbc_source/src/lib.rs @@ -107,6 +107,21 @@ static RE_INCREMENTAL_PREDICATE: std::sync::LazyLock = std::sync::LazyLoc const CONNECTOR_NAME: &str = "JDBC source"; +/// Poll interval used when `poll_interval` is unset or empty. +const DEFAULT_POLL_INTERVAL: Duration = Duration::from_secs(5); + +/// Parse the configured `poll_interval` humantime string into a `Duration`, +/// falling back to [`DEFAULT_POLL_INTERVAL`] when unset, empty, or unparseable. +/// `validate_config` separately rejects a set-but-unparseable value so a typo +/// surfaces at `open()` rather than silently defaulting. +fn parse_poll_interval(poll_interval: Option<&str>) -> Duration { + poll_interval + .map(str::trim) + .filter(|value| !value.is_empty()) + .and_then(|value| humantime::parse_duration(value).ok()) + .unwrap_or(DEFAULT_POLL_INTERVAL) +} + /// Source mode for the JDBC connector #[derive(Debug, Clone, Deserialize, Serialize, PartialEq)] #[serde(rename_all = "lowercase")] @@ -148,9 +163,10 @@ pub struct JdbcSourceConfig { /// Can use {last_offset} placeholder for incremental reads pub query: String, - /// Polling interval (e.g., "30s", "5m", "1h") - #[serde(with = "humantime_serde")] - pub poll_interval: Duration, + /// Polling interval as a humantime string (e.g., "30s", "5m", "1h"). Parsed + /// once at construction; defaults to 5s when unset. + #[serde(default)] + pub poll_interval: Option, /// Batch size - maximum rows to fetch per poll #[serde(default = "default_batch_size")] @@ -181,15 +197,16 @@ pub struct JdbcSourceConfig { pub jvm_options: Vec, /// Timeout for the per-poll `Connection.isValid` liveness check (default: - /// 30000). JDBC expresses this timeout in whole seconds, so the value is - /// converted to seconds and clamped to the 1..=30s range; it does not govern - /// connection establishment. + /// 5000). JDBC expresses this timeout in whole seconds, so the value is + /// converted to seconds and clamped to the 1..=5s range (the check runs on a + /// shared worker and must stay short); it does not govern connection + /// establishment. #[serde(default = "default_connection_timeout")] pub connection_timeout_ms: u64, } fn default_connection_timeout() -> u64 { - 30000 + 5000 } fn default_batch_size() -> u32 { @@ -270,6 +287,8 @@ pub struct JdbcSource { // direct connection without `&mut self`. connection: Mutex>, state: Arc>, + // Poll interval parsed once from `config.poll_interval` at construction. + poll_interval: Duration, // Scheduled start of the next poll, used to pace polls at a fixed cadence // that does not drift with per-poll work time. `None` until the first poll. next_poll_at: Mutex>, @@ -304,12 +323,14 @@ impl JdbcSource { default_state }); + let poll_interval = parse_poll_interval(config.poll_interval.as_deref()); Self { id, config, jvm: None, connection: Mutex::new(None), state: Arc::new(Mutex::new(state)), + poll_interval, next_poll_at: Mutex::new(None), } } @@ -555,7 +576,9 @@ impl JdbcSource { /// Best-effort `Connection.isValid(timeout)` check. Returns false on any /// JNI error so the caller re-establishes the connection. fn connection_is_valid(&self, env: &mut JNIEnv, conn: &JObject) -> bool { - let timeout_secs = (self.config.connection_timeout_ms / 1000).clamp(1, 30) as i32; + // Cap the liveness check short: it runs on a shared block_in_place worker, + // so a dead connection must not block it for tens of seconds. + let timeout_secs = (self.config.connection_timeout_ms / 1000).clamp(1, 5) as i32; match env .call_method(conn, "isValid", "(I)Z", &[JValue::Int(timeout_secs)]) .and_then(|v| v.z()) @@ -1006,6 +1029,27 @@ impl JdbcSource { Ok(col_type) } + /// Return `value`, or JSON `null` when the last primitive getter read a SQL + /// NULL (detected via `ResultSet.wasNull()`). + fn null_or( + &self, + env: &mut JNIEnv, + result_set: &JObject, + value: serde_json::Value, + ) -> Result { + let was_null = jni!( + env, + env.call_method(result_set, "wasNull", "()Z", &[]) + .and_then(|v| v.z()), + "Failed to check wasNull" + ); + Ok(if was_null { + serde_json::Value::Null + } else { + value + }) + } + /// Extract column value based on JDBC type fn extract_column_value( &self, @@ -1016,23 +1060,11 @@ impl JdbcSource { ) -> Result { use java::sql::Types; - // Check if null first - let obj = jni!( - env, - env.call_method( - result_set, - "getObject", - "(I)Ljava/lang/Object;", - &[JValue::Int(column_index)], - ) - .and_then(|v| v.l()), - "Failed to get object" - ); - - if obj.is_null() { - return Ok(serde_json::Value::Null); - } - + // Primitive getters (getInt/getBoolean/...) return 0/false for SQL NULL, + // so `null_or` consults ResultSet.wasNull() after the getter to tell an + // actual NULL from a zero value. Object getters (getString/getBytes) + // return a null reference for SQL NULL and are null-checked directly, so + // there is no separate getObject probe (one JNI call per column, not two). match *sql_type { Types::BIT | Types::BOOLEAN => { let value = jni!( @@ -1046,7 +1078,7 @@ impl JdbcSource { .and_then(|v| v.z()), "Failed to get boolean" ); - Ok(serde_json::Value::Bool(value)) + self.null_or(env, result_set, serde_json::Value::Bool(value)) } Types::TINYINT | Types::SMALLINT | Types::INTEGER => { let value = jni!( @@ -1055,7 +1087,7 @@ impl JdbcSource { .and_then(|v| v.i()), "Failed to get int" ); - Ok(serde_json::json!(value)) + self.null_or(env, result_set, serde_json::json!(value)) } Types::BIGINT => { let value = jni!( @@ -1064,7 +1096,7 @@ impl JdbcSource { .and_then(|v| v.j()), "Failed to get long" ); - Ok(serde_json::json!(value)) + self.null_or(env, result_set, serde_json::json!(value)) } Types::FLOAT | Types::REAL => { let value = jni!( @@ -1073,7 +1105,7 @@ impl JdbcSource { .and_then(|v| v.f()), "Failed to get float" ); - Ok(serde_json::json!(value)) + self.null_or(env, result_set, serde_json::json!(value)) } Types::DOUBLE => { let value = jni!( @@ -1087,7 +1119,7 @@ impl JdbcSource { .and_then(|v| v.d()), "Failed to get double" ); - Ok(serde_json::json!(value)) + self.null_or(env, result_set, serde_json::json!(value)) } // NUMERIC/DECIMAL can carry more precision than an f64 can represent // (e.g. money/large decimals), so emit them as strings to avoid @@ -1223,6 +1255,17 @@ impl JdbcSource { ))); } + // A set poll_interval must be a valid humantime string; an unparseable + // value would otherwise silently fall back to the default. + if let Some(value) = self.config.poll_interval.as_deref() + && !value.trim().is_empty() + && humantime::parse_duration(value.trim()).is_err() + { + return Err(Error::InvalidConfigValue(format!( + "poll_interval '{value}' is not a valid duration (e.g. \"30s\", \"5m\", \"1h\")" + ))); + } + // Separate-credential auth requires both username and password; a // half-set pair would silently fall through to URL-embedded credentials. if self.config.username.is_some() != self.config.password.is_some() { @@ -1297,7 +1340,7 @@ impl Source for JdbcSource { let scheduled = { let mut next = lock_mutex(&self.next_poll_at, "next_poll_at")?; let scheduled = next.map_or_else(Instant::now, |planned| planned.max(Instant::now())); - *next = Some(scheduled + self.config.poll_interval); + *next = Some(scheduled + self.poll_interval); scheduled }; let now = Instant::now(); @@ -1686,7 +1729,7 @@ mod tests { username: None, password: None, query: "SELECT 1".to_string(), - poll_interval: Duration::from_secs(10), + poll_interval: Some("10s".to_string()), batch_size: 100, tracking_column: None, initial_offset: None, @@ -1706,6 +1749,32 @@ mod tests { path.to_string_lossy().into_owned() } + #[test] + fn test_parse_poll_interval() { + assert_eq!(parse_poll_interval(Some("30s")), Duration::from_secs(30)); + assert_eq!(parse_poll_interval(Some("5m")), Duration::from_secs(300)); + // Unset, empty, and unparseable all fall back to the default. + assert_eq!(parse_poll_interval(None), DEFAULT_POLL_INTERVAL); + assert_eq!(parse_poll_interval(Some(" ")), DEFAULT_POLL_INTERVAL); + assert_eq!( + parse_poll_interval(Some("not-a-duration")), + DEFAULT_POLL_INTERVAL + ); + } + + #[test] + fn test_validate_config_rejects_bad_poll_interval() { + let jar = write_temp_jar("jdbc_validate_poll_interval.jar"); + let mut config = base_config(); + config.driver_jar_path = jar; + config.poll_interval = Some("banana".to_string()); + let source = JdbcSource::new(1, config, None); + let err = source + .validate_config() + .expect_err("must reject bad poll_interval"); + assert!(matches!(err, Error::InvalidConfigValue(msg) if msg.contains("poll_interval"))); + } + #[test] fn test_quote_sql_literal_escapes_backslash() { // Backslash is doubled before quotes so a trailing backslash cannot @@ -2051,7 +2120,7 @@ mod tests { username: None, password: None, query: "SELECT * FROM users WHERE id > {last_offset} ORDER BY id".to_string(), - poll_interval: Duration::from_secs(10), + poll_interval: Some("10s".to_string()), batch_size: 100, tracking_column: Some("id".to_string()), initial_offset: Some("0".to_string()), @@ -2093,7 +2162,7 @@ mod tests { query: "SELECT * FROM orders WHERE {tracking_column} > {last_offset} ORDER BY {tracking_column}" .to_string(), - poll_interval: Duration::from_secs(10), + poll_interval: Some("10s".to_string()), batch_size: 100, tracking_column: Some("updated_at".to_string()), initial_offset: Some("2024-01-01".to_string()), @@ -2127,7 +2196,7 @@ mod tests { query: "SELECT * FROM orders WHERE {tracking_column} > {last_offset} ORDER BY {tracking_column}" .to_string(), - poll_interval: Duration::from_secs(10), + poll_interval: Some("10s".to_string()), batch_size: 100, tracking_column: Some("updated_at".to_string()), initial_offset: None, @@ -2161,7 +2230,7 @@ mod tests { username: None, password: None, query: "SELECT * FROM t WHERE {tracking_column} > {last_offset}".to_string(), - poll_interval: Duration::from_secs(10), + poll_interval: Some("10s".to_string()), batch_size: 100, tracking_column: Some("id; DROP TABLE t".to_string()), initial_offset: Some("0".to_string()), @@ -2184,7 +2253,7 @@ mod tests { username: None, password: None, query: "SELECT * FROM products".to_string(), - poll_interval: Duration::from_secs(60), + poll_interval: Some("60s".to_string()), batch_size: 5000, tracking_column: None, initial_offset: None, @@ -2219,7 +2288,7 @@ mod tests { username: None, password: None, query: "SELECT * FROM orders WHERE updated_at > {last_offset}".to_string(), - poll_interval: Duration::from_secs(30), + poll_interval: Some("30s".to_string()), batch_size: 1000, tracking_column: Some("updated_at".to_string()), initial_offset: Some("2024-01-01 00:00:00".to_string()), @@ -2313,13 +2382,17 @@ mod tests { toml::from_str(toml_str).expect("Failed to parse minimal TOML config"); assert_eq!(config.driver_class, "org.h2.Driver"); assert_eq!(config.query, "SELECT * FROM users"); - assert_eq!(config.poll_interval, Duration::from_secs(30)); + assert_eq!(config.poll_interval.as_deref(), Some("30s")); + assert_eq!( + parse_poll_interval(config.poll_interval.as_deref()), + Duration::from_secs(30) + ); // Verify defaults are applied assert_eq!(config.mode, Mode::Incremental); assert_eq!(config.batch_size, 1000); assert!(config.include_metadata); assert!(!config.snake_case_columns); - assert_eq!(config.connection_timeout_ms, 30000); + assert_eq!(config.connection_timeout_ms, 5000); assert!(config.username.is_none()); assert!(config.password.is_none()); assert!(config.tracking_column.is_none()); @@ -2359,7 +2432,10 @@ mod tests { assert!(!config.include_metadata); assert_eq!(config.jvm_options, vec!["-Xmx512m", "-Xms128m"]); assert_eq!(config.connection_timeout_ms, 60000); - assert_eq!(config.poll_interval, Duration::from_secs(300)); + assert_eq!( + parse_poll_interval(config.poll_interval.as_deref()), + Duration::from_secs(300) + ); } #[test] @@ -2375,7 +2451,10 @@ mod tests { let config: JdbcSourceConfig = toml::from_str(toml_str).expect("Failed to parse bulk mode config"); assert_eq!(config.mode, Mode::Bulk); - assert_eq!(config.poll_interval, Duration::from_secs(3600)); + assert_eq!( + parse_poll_interval(config.poll_interval.as_deref()), + Duration::from_secs(3600) + ); } #[test] @@ -2411,7 +2490,7 @@ mod tests { username: None, password: None, query: "SELECT 1".to_string(), - poll_interval: Duration::from_secs(10), + poll_interval: Some("10s".to_string()), batch_size: 100, tracking_column: None, initial_offset: None, @@ -2439,7 +2518,7 @@ mod tests { username: None, password: None, query: "SELECT 1".to_string(), - poll_interval: Duration::from_secs(10), + poll_interval: Some("10s".to_string()), batch_size: 100, tracking_column: None, initial_offset: None, @@ -2464,7 +2543,7 @@ mod tests { username: None, password: None, query: "SELECT * FROM orders WHERE id > {last_offset}".to_string(), - poll_interval: Duration::from_secs(10), + poll_interval: Some("10s".to_string()), batch_size: 100, tracking_column: Some("id".to_string()), initial_offset: Some("100".to_string()), @@ -2489,7 +2568,7 @@ mod tests { username: None, password: None, query: "SELECT * FROM products".to_string(), - poll_interval: Duration::from_secs(60), + poll_interval: Some("60s".to_string()), batch_size: 100, tracking_column: None, initial_offset: None, @@ -2518,7 +2597,7 @@ mod tests { username: None, password: None, query: "SELECT 1".to_string(), - poll_interval: Duration::from_secs(10), + poll_interval: Some("10s".to_string()), batch_size: 100, tracking_column: None, initial_offset: None, @@ -2547,7 +2626,7 @@ mod tests { username: None, password: None, query: "SELECT 1".to_string(), - poll_interval: Duration::from_secs(10), + poll_interval: Some("10s".to_string()), batch_size: 100, tracking_column: None, initial_offset: None, @@ -2579,7 +2658,7 @@ mod tests { username: None, password: None, query: "SELECT 1".to_string(), - poll_interval: Duration::from_secs(10), + poll_interval: Some("10s".to_string()), batch_size: 100, tracking_column: None, initial_offset: None, @@ -2608,7 +2687,7 @@ mod tests { username: None, password: None, query: "SELECT 1".to_string(), - poll_interval: Duration::from_secs(10), + poll_interval: Some("10s".to_string()), batch_size: 100, tracking_column: None, initial_offset: None, @@ -2635,7 +2714,7 @@ mod tests { username: None, password: None, query: "SELECT 1".to_string(), - poll_interval: Duration::from_secs(10), + poll_interval: Some("10s".to_string()), batch_size: 100, tracking_column: None, initial_offset: None, @@ -2665,7 +2744,7 @@ mod tests { password: None, query: "SELECT * FROM users WHERE {tracking_column} > {last_offset} ORDER BY id" .to_string(), - poll_interval: Duration::from_secs(10), + poll_interval: Some("10s".to_string()), batch_size: 100, tracking_column: Some("id".to_string()), initial_offset: None, @@ -2702,7 +2781,7 @@ mod tests { username: None, password: None, query: "SELECT * FROM t WHERE id >= {last_offset}".to_string(), - poll_interval: Duration::from_secs(10), + poll_interval: Some("10s".to_string()), batch_size: 100, tracking_column: None, initial_offset: None, @@ -2725,7 +2804,7 @@ mod tests { username: None, password: None, query: "SELECT * FROM users ORDER BY id".to_string(), - poll_interval: Duration::from_secs(10), + poll_interval: Some("10s".to_string()), batch_size: 100, tracking_column: Some("id".to_string()), initial_offset: Some("0".to_string()), @@ -2787,7 +2866,7 @@ mod tests { username: Some("admin".to_string()), password: Some(SecretString::from("MyP@ssw0rd")), query: "SELECT 1".to_string(), - poll_interval: Duration::from_secs(10), + poll_interval: Some("10s".to_string()), batch_size: 100, tracking_column: None, initial_offset: None, @@ -2825,7 +2904,7 @@ mod tests { username: None, password: None, query: "SELECT 1".to_string(), - poll_interval: Duration::from_secs(10), + poll_interval: Some("10s".to_string()), batch_size: 100, tracking_column: None, initial_offset: None, diff --git a/core/integration/tests/connectors/jdbc/connectors_config_postgres/jdbc_pg.toml b/core/integration/tests/connectors/jdbc/connectors_config_postgres/jdbc_pg.toml index 0f35c4c4c1..0e2e1bad76 100644 --- a/core/integration/tests/connectors/jdbc/connectors_config_postgres/jdbc_pg.toml +++ b/core/integration/tests/connectors/jdbc/connectors_config_postgres/jdbc_pg.toml @@ -28,7 +28,6 @@ path = "../../target/debug/libiggy_connector_jdbc_source" stream = "test_stream" topic = "test_topic" schema = "json" -partition_id = 1 [plugin_config] # All required fields - values will be overridden by environment variables From 0e8047987230460e56e6d5b75d8233412772f35a Mon Sep 17 00:00:00 2001 From: shbhmrzd Date: Thu, 23 Jul 2026 14:06:42 +0530 Subject: [PATCH 03/11] fix(connectors): harden JDBC source incremental cursor and null handling Derives the incremental cursor from the last ordered row rather than a Rust-side max, so the cursor always matches the database's own ORDER BY and degrades to re-reads rather than skips if the ordering is imperfect. Fails loudly when the tracking column is absent from the result set, and rejects an empty query, a blank tracking_column, or a blank initial_offset at open(). Routes date/time columns through the null-safe string path so a NULL value no longer fails the poll, and preserves companion WHERE conditions (such as an operator-added IS NOT NULL) when stripping the offset predicate at cold start. --- core/connectors/sources/jdbc_source/README.md | 35 ++- .../connectors/sources/jdbc_source/src/lib.rs | 236 ++++++++++++------ 2 files changed, 181 insertions(+), 90 deletions(-) diff --git a/core/connectors/sources/jdbc_source/README.md b/core/connectors/sources/jdbc_source/README.md index 65bd59edab..4bfbce3554 100644 --- a/core/connectors/sources/jdbc_source/README.md +++ b/core/connectors/sources/jdbc_source/README.md @@ -218,21 +218,36 @@ connector refuses to start otherwise): `ORDER BY {tracking_column}` or the column name (optionally table-qualified, e.g. `ORDER BY t.updated_at`). A composite order such as `ORDER BY other, id` (tracking column not first) or a `DESC` order is rejected at `open()`. + The `ORDER BY` check is a lexical heuristic that validates single-block + `SELECT`s; `UNION`, window-function, or CTE queries may not be validated + correctly, so verify the result ordering yourself for those. +The connector takes the tracking value of the **last row** of each ordered batch +as the next cursor, so the cursor always matches the database's own `ORDER BY`. The tracking column must also be: -- **Homogeneously typed and monotonic.** Offsets are compared as integers, then - floats, then lexicographically; a column mixing numeric-looking and - non-numeric strings can make the connector's comparison disagree with the - database's `>` and skip or re-read rows. Prefer an auto-increment ID or a - timestamp. +- **Unique / strictly increasing.** The next poll resumes with a strict + `> {last_offset}`, and each batch is capped by `setMaxRows`. If a batch ends in + the middle of a run of rows that share the same tracking value (common for a + non-unique column like a timestamp), the remaining same-value rows are skipped + on the next poll. Use a unique, strictly-increasing key (an auto-increment ID + is ideal). If you must track a non-unique column, ensure `batch_size` exceeds + the largest group of equal values so a tie never spans a batch boundary. + (Keyset pagination with a tie-break is a planned follow-up.) +- **Monotonic under the database's own ordering.** Because the cursor is the last + ordered row and is fed back as `WHERE {tracking_column} > ''`, the column + must increase monotonically under the same ordering the database applies to that + `>` (including its collation, for text). Prefer an auto-increment ID or a + timestamp; a case-insensitively-collated text key can order differently than its + bytes and skip or re-read rows. - **`NOT NULL`.** A NULL tracking value cannot be watermarked, so the connector errors the poll if it reads one and keeps erroring (making no progress) until the query is fixed. Exclude NULLs in the query, e.g. `AND {tracking_column} IS NOT NULL`. -- **Lexicographically ordered, for timestamps.** Timestamp columns are read via - the driver's string form; ensure that form is ordered (ISO-8601 is). A locale - format such as `MM/DD/YYYY` is not monotonic as text and will misorder. +- **Round-trippable string form, for timestamps.** Timestamp columns are read as + the driver's string form and substituted back into the next `WHERE`; ensure the + driver emits a form the database orders correctly and can parse back (ISO-8601 + is safe; a locale format such as `MM/DD/YYYY` is not). **Example:** @@ -324,7 +339,9 @@ JDBC SQL types are automatically mapped to JSON: guarantee is tracked as a follow-up. - **Connection recovery.** The connection is validated with `Connection.isValid` each poll and transparently re-established (closing the old handle) if it has - dropped. + dropped. The check runs on the shared `block_in_place` worker, so its timeout + (`connection_timeout_ms`) is intentionally converted to whole seconds and + capped at 5s: a dead connection must not block the worker for tens of seconds. - **`SQLState` classification is informational today.** Query failures are classified into transient vs permanent error variants, but the runtime does not yet apply differentiated backoff based on that distinction; it currently diff --git a/core/connectors/sources/jdbc_source/src/lib.rs b/core/connectors/sources/jdbc_source/src/lib.rs index c99058a25b..7bbfd6e3d2 100644 --- a/core/connectors/sources/jdbc_source/src/lib.rs +++ b/core/connectors/sources/jdbc_source/src/lib.rs @@ -97,11 +97,20 @@ static RE_PASSWORD_PARAM: std::sync::LazyLock = static RE_ORACLE_PASS: std::sync::LazyLock = std::sync::LazyLock::new(|| Regex::new(r"thin:([^/]+)/([^@]+)@").unwrap()); -/// Matches the canonical incremental predicate so it can be removed on the first -/// (no-offset) poll. Case- and whitespace-tolerant so `where {tracking_column}>{last_offset}` -/// and `WHERE {tracking_column} > {last_offset}` are all recognized, rather -/// than only one exact literal spelling. -static RE_INCREMENTAL_PREDICATE: std::sync::LazyLock = std::sync::LazyLock::new(|| { +/// Regexes that remove the incremental offset predicate `{tracking_column} > +/// {last_offset}` on the first (no-offset) poll while preserving any other +/// `WHERE` conditions. All are case- and whitespace-tolerant. They are applied in +/// order so an operator-added companion condition (e.g. +/// `AND {tracking_column} IS NOT NULL`) does not leave a dangling `WHERE ... AND` +/// or leading `AND ...` fragment. See [`strip_offset_predicate`]. +static RE_OFFSET_PREDICATE_AND_AFTER: std::sync::LazyLock = std::sync::LazyLock::new(|| { + Regex::new(r"(?i)\bWHERE\s+\{tracking_column\}\s*>\s*\{last_offset\}\s+AND\s+").unwrap() +}); +static RE_OFFSET_PREDICATE_AND_BEFORE: std::sync::LazyLock = + std::sync::LazyLock::new(|| { + Regex::new(r"(?i)\s+AND\s+\{tracking_column\}\s*>\s*\{last_offset\}").unwrap() + }); +static RE_OFFSET_PREDICATE_BARE: std::sync::LazyLock = std::sync::LazyLock::new(|| { Regex::new(r"(?i)\bWHERE\s+\{tracking_column\}\s*>\s*\{last_offset\}").unwrap() }); @@ -790,7 +799,7 @@ impl JdbcSource { // request an absurd allocation up front. let mut messages = Vec::with_capacity((self.config.batch_size as usize).min(8192)); let mut row_count: u64 = 0; - let mut max_offset: Option = None; + let mut last_offset: Option = None; loop { let has_next = jni!( @@ -816,14 +825,15 @@ impl JdbcSource { let _ = unsafe { env.pop_local_frame(&JObject::null()) }; let (row_data, offset) = row_result?; - // Track the maximum tracking value across the batch rather than - // assuming the last row is the largest, so incremental mode is - // correct even if the query is not ordered by the tracking column. + // Take the tracking value of the LAST row as the next offset. Rows + // arrive in ascending tracking order (validate_config enforces + // ORDER BY the tracking column ascending), so the last row is the + // high-water mark. Using the last row rather than a Rust-side max + // keeps the cursor consistent with the database's own ordering, and + // degrades to re-reads (safe, at-least-once) rather than skips if the + // ordering is ever imperfect. if let Some(offset) = offset { - max_offset = Some(match max_offset { - Some(current) => larger_offset(current, offset), - None => offset, - }); + last_offset = Some(offset); } let message = self.build_message(row_data)?; @@ -831,7 +841,20 @@ impl JdbcSource { row_count += 1; } - Ok((messages, row_count, max_offset)) + // In incremental mode a non-empty batch that never yielded a tracking + // value means the tracking column is absent from the result set: the + // offset could never advance, so the same batch would be re-read forever. + // Fail loudly instead of stalling silently. (A NULL tracking value in a + // present column is already rejected per-row by tracking_offset_or_error.) + if self.config.mode == Mode::Incremental && row_count > 0 && last_offset.is_none() { + return Err(Error::InvalidConfigValue(format!( + "tracking column '{}' is not present in the query result; incremental mode cannot \ + advance its offset. Include it in the SELECT list.", + self.config.tracking_column.as_deref().unwrap_or("") + ))); + } + + Ok((messages, row_count, last_offset)) } /// Extract data from a single result set row, returning the row map and optional offset. @@ -951,7 +974,7 @@ impl JdbcSource { // Without an offset yet, drop the incremental predicate but keep the rest // of the query (e.g. an ORDER BY) intact. if offset.is_none() { - query = RE_INCREMENTAL_PREDICATE.replace(&query, "").into_owned(); + query = strip_offset_predicate(&query); } // Substitute {tracking_column} wherever it still appears (a WHERE and/or @@ -1154,25 +1177,11 @@ impl JdbcSource { base64::engine::general_purpose::STANDARD.encode(&buf), )) } + // Date/time types are read via their driver string form. Route + // through the null-safe getString path so a NULL date/time yields + // JSON null instead of failing the whole poll on get_string(null). Types::TIMESTAMP | Types::DATE | Types::TIME => { - let value = jni!( - env, - env.call_method( - result_set, - "getString", - "(I)Ljava/lang/String;", - &[JValue::Int(column_index)], - ) - .and_then(|v| v.l()), - "Failed to get timestamp" - ); - let str_value: String = jni!( - env, - env.get_string(&JString::from(value)), - "Failed to convert timestamp" - ) - .into(); - Ok(serde_json::Value::String(str_value)) + self.get_column_as_string(env, result_set, column_index) } // Default: getString for all other types (CHAR, VARCHAR, etc.) _ => self.get_column_as_string(env, result_set, column_index), @@ -1266,6 +1275,25 @@ impl JdbcSource { ))); } + // The query must be non-empty; an empty query only fails later at + // prepareStatement with an opaque driver error. + if self.config.query.trim().is_empty() { + return Err(Error::InvalidConfigValue( + "query must not be empty".to_string(), + )); + } + + // A set initial_offset must be non-blank: an empty value would build + // `WHERE tracking > ''` (a type error or always-false on many databases) + // rather than the intended cold-start scan. + if let Some(initial_offset) = self.config.initial_offset.as_deref() + && initial_offset.trim().is_empty() + { + return Err(Error::InvalidConfigValue( + "initial_offset must not be empty; omit it to start from the beginning".to_string(), + )); + } + // Separate-credential auth requires both username and password; a // half-set pair would silently fall through to URL-embedded credentials. if self.config.username.is_some() != self.config.password.is_some() { @@ -1281,10 +1309,15 @@ impl JdbcSource { // by that column, setMaxRows returns an arbitrary subset, and advancing // the offset to its max permanently skips the unread lower keys. if self.config.mode == Mode::Incremental { - let Some(tracking_column) = self.config.tracking_column.as_deref() else { + let Some(tracking_column) = self + .config + .tracking_column + .as_deref() + .filter(|column| !column.trim().is_empty()) + else { return Err(Error::InvalidConfigValue( - "incremental mode requires tracking_column so the offset can advance; set \ - tracking_column, or use mode = \"bulk\"" + "incremental mode requires a non-empty tracking_column so the offset can \ + advance; set tracking_column, or use mode = \"bulk\"" .to_string(), )); }; @@ -1514,28 +1547,17 @@ fn finalize_query(query: String) -> Result { Ok(query) } -/// Return the larger of two offset values. Integer keys are compared as `i128` -/// first (exact across the full `BIGINT` range, avoiding the f64 precision loss -/// past 2^53 that would misorder large IDs and contradict the NUMERIC/DECIMAL -/// -as-string rationale elsewhere in this file); genuinely fractional values -/// fall back to f64; non-numeric values (ISO-8601 timestamps, text keys) compare -/// lexicographically. -/// -/// This assumes the tracking column is homogeneously typed and monotonic under -/// the database's own ordering. A column that mixes numeric-looking and -/// non-numeric strings, or a timestamp whose driver string form is not -/// lexicographically ordered, can make this Rust-side max disagree with the -/// database's `>` comparison and cause rows to be skipped or re-read. See the -/// tracking-column guidance in the README. -fn larger_offset(a: String, b: String) -> String { - let b_is_larger = match (a.parse::(), b.parse::()) { - (Ok(na), Ok(nb)) => nb > na, - _ => match (a.parse::(), b.parse::()) { - (Ok(na), Ok(nb)) => nb > na, - _ => b > a, - }, - }; - if b_is_larger { b } else { a } +/// Remove the incremental offset predicate `{tracking_column} > {last_offset}` +/// from a query for the cold-start (no-offset) poll, preserving any other `WHERE` +/// conditions. Handles the offset term followed by `AND ...`, preceded by +/// `... AND`, or standing alone, so a companion condition such as +/// `AND {tracking_column} IS NOT NULL` survives as a valid `WHERE`. +fn strip_offset_predicate(query: &str) -> String { + let query = RE_OFFSET_PREDICATE_AND_AFTER.replace_all(query, "WHERE "); + let query = RE_OFFSET_PREDICATE_AND_BEFORE.replace_all(&query, ""); + RE_OFFSET_PREDICATE_BARE + .replace_all(&query, "") + .into_owned() } /// Check that an incremental query's result set is ordered ascending by the @@ -1784,16 +1806,6 @@ mod tests { assert_eq!(quote_sql_literal(r"\'"), r"'\\'''"); } - #[test] - fn test_larger_offset_i128_precision_beyond_f64() { - // Two BIGINTs that differ only past 2^53 must still be ordered exactly; - // an f64 comparison would collapse them and misorder the offset. - let a = "9007199254740993".to_string(); // 2^53 + 1 - let b = "9007199254740992".to_string(); // 2^53 - assert_eq!(larger_offset(a.clone(), b.clone()), a); - assert_eq!(larger_offset(b, a.clone()), a); - } - #[test] fn test_build_query_removes_predicate_case_and_whitespace_insensitive() { for query in [ @@ -1816,6 +1828,48 @@ mod tests { } } + #[test] + fn test_strip_offset_predicate_preserves_other_conditions() { + // Offset term followed by a companion condition (the README-advised + // IS NOT NULL): the AND and the companion condition survive. + assert_eq!( + strip_offset_predicate( + "SELECT * FROM t WHERE {tracking_column} > {last_offset} AND {tracking_column} IS NOT NULL ORDER BY {tracking_column}" + ), + "SELECT * FROM t WHERE {tracking_column} IS NOT NULL ORDER BY {tracking_column}" + ); + // Offset term preceded by a condition. + assert_eq!( + strip_offset_predicate( + "SELECT * FROM t WHERE active = 1 AND {tracking_column} > {last_offset} ORDER BY id" + ), + "SELECT * FROM t WHERE active = 1 ORDER BY id" + ); + // Bare offset term (no other condition) drops the whole WHERE. + assert!( + !strip_offset_predicate( + "SELECT * FROM t WHERE {tracking_column} > {last_offset} ORDER BY id" + ) + .contains("{last_offset}") + ); + } + + #[test] + fn test_build_query_cold_start_compound_predicate_is_valid() { + let mut config = base_config(); + config.mode = Mode::Incremental; + config.tracking_column = Some("id".to_string()); + config.query = "SELECT * FROM t WHERE {tracking_column} > {last_offset} AND {tracking_column} IS NOT NULL ORDER BY {tracking_column}".to_string(); + let source = JdbcSource::new(1, config, None); + // Cold start: no persisted offset, no initial_offset. + let built = source.build_query(&State::default()).expect("build query"); + assert_eq!(built, "SELECT * FROM t WHERE id IS NOT NULL ORDER BY id"); + assert!(!built.contains("{last_offset}") && !built.contains("{tracking_column}")); + // No dangling AND / empty WHERE. + assert!(!built.to_uppercase().contains("WHERE AND")); + assert!(!built.to_uppercase().contains("AND ORDER")); + } + #[test] fn test_validate_config_rejects_half_set_credentials() { let jar = write_temp_jar("jdbc_validate_half_creds.jar"); @@ -1972,6 +2026,39 @@ mod tests { )); } + #[test] + fn test_validate_config_rejects_empty_query_and_blank_offsets() { + let jar = write_temp_jar("jdbc_validate_blanks.jar"); + // Empty query. + let mut config = base_config(); + config.driver_jar_path = jar.clone(); + config.query = " ".to_string(); + let source = JdbcSource::new(1, config, None); + assert!( + matches!(source.validate_config(), Err(Error::InvalidConfigValue(msg)) if msg.contains("query")) + ); + + // Blank initial_offset. + let mut config = base_config(); + config.driver_jar_path = jar.clone(); + config.initial_offset = Some(" ".to_string()); + let source = JdbcSource::new(1, config, None); + assert!( + matches!(source.validate_config(), Err(Error::InvalidConfigValue(msg)) if msg.contains("initial_offset")) + ); + + // Blank tracking_column in incremental mode. + let mut config = base_config(); + config.driver_jar_path = jar; + config.mode = Mode::Incremental; + config.tracking_column = Some("".to_string()); + config.query = "SELECT id FROM t ORDER BY id".to_string(); + let source = JdbcSource::new(1, config, None); + assert!( + matches!(source.validate_config(), Err(Error::InvalidConfigValue(msg)) if msg.contains("tracking_column")) + ); + } + #[test] fn test_validate_config_rejects_bad_batch_size() { let jar = write_temp_jar("jdbc_validate_batch_size.jar"); @@ -2332,19 +2419,6 @@ mod tests { assert!(!is_transient_sql_state(Some(""))); } - #[test] - fn test_larger_offset_numeric_and_lexical() { - // Numeric comparison, not lexical: "100" > "9" numerically. - assert_eq!(larger_offset("9".into(), "100".into()), "100"); - assert_eq!(larger_offset("100".into(), "9".into()), "100"); - assert_eq!(larger_offset("10.5".into(), "9.9".into()), "10.5"); - // Non-numeric (timestamps / text) compare lexicographically. - assert_eq!( - larger_offset("2024-01-01".into(), "2024-06-15".into()), - "2024-06-15" - ); - } - #[test] fn test_is_valid_identifier() { assert!(is_valid_identifier("id")); From dfc245761ada7ee4d038db0de94c73d4c55f8ddb Mon Sep 17 00:00:00 2001 From: shbhmrzd Date: Thu, 23 Jul 2026 14:11:08 +0530 Subject: [PATCH 04/11] docs(connectors): caveat non-unique timestamp tracking columns in JDBC examples The incremental examples used timestamp tracking columns (updated_at, OrderDate) without noting that a non-unique tracking value can skip rows when a same-value group is split across a batch boundary. Add that caveat to the affected examples and point to the unique/strictly-increasing requirement. --- core/connectors/sources/jdbc_source/README.md | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/core/connectors/sources/jdbc_source/README.md b/core/connectors/sources/jdbc_source/README.md index 4bfbce3554..1537d7d7b1 100644 --- a/core/connectors/sources/jdbc_source/README.md +++ b/core/connectors/sources/jdbc_source/README.md @@ -90,6 +90,10 @@ password = "secret_password" query = "SELECT * FROM orders WHERE updated_at > {last_offset} ORDER BY updated_at ASC" poll_interval = "30s" batch_size = 1000 +# updated_at is a timestamp and may not be unique. A run of rows sharing one +# timestamp that is split across a batch boundary would skip the remainder (see +# "Unique / strictly increasing" below). Prefer a unique auto-increment key, or +# keep batch_size larger than any same-timestamp group. tracking_column = "updated_at" initial_offset = "2024-01-01 00:00:00" mode = "incremental" @@ -168,6 +172,9 @@ password = "YourPassword123" query = "SELECT * FROM Orders WHERE OrderDate > {last_offset} ORDER BY OrderDate" poll_interval = "15s" batch_size = 2000 +# OrderDate is a timestamp and may not be unique; see the uniqueness note on the +# MySQL example above. Prefer a unique key, or keep batch_size above any +# same-timestamp group. tracking_column = "OrderDate" initial_offset = "2024-01-01" mode = "incremental" @@ -519,11 +526,13 @@ query = "SELECT * FROM table WHERE {tracking_column} > {last_offset} ORDER BY {t - Efficient for large tables - Works with timestamps, IDs, or any orderable column -**Database Examples** (the query must order by the tracking column): +**Database Examples** (the query must order by the tracking column; a unique, +strictly-increasing key like an auto-increment ID is safest, see the tracking +column requirements above): -- MySQL: `WHERE updated_at > {last_offset} ORDER BY updated_at` +- MySQL: `WHERE updated_at > {last_offset} ORDER BY updated_at` (timestamp; ensure `batch_size` exceeds any same-timestamp group, or track a unique id) - Oracle: `WHERE id > {last_offset} ORDER BY id` (use a monotonic key; `ROWNUM` is not a valid tracking column) -- SQL Server: `WHERE updated_at > {last_offset} ORDER BY updated_at` +- SQL Server: `WHERE updated_at > {last_offset} ORDER BY updated_at` (timestamp; same caveat as MySQL) - PostgreSQL: `WHERE id > {last_offset} ORDER BY id` ### Bulk Mode (Universal) From 6f1eedcfa8de5f0f92aa2a60fde03d11c2bb93bd Mon Sep 17 00:00:00 2001 From: shbhmrzd Date: Mon, 3 Aug 2026 19:29:34 +0530 Subject: [PATCH 05/11] fix(connectors): align JDBC ORDER BY validation with read-time matching Incremental-mode validation and row reading disagreed about what a tracking-column name refers to, so a config that reads rows correctly could be rejected at open(), and a config that skips rows could be accepted. Read-time tracking_column_matches compares against both the raw driver label and the snake_cased output key, but the ORDER BY check compared a literal lowercased string. With snake_case_columns on, a snake_cased tracking_column against a CamelCase ORDER BY column (order_date vs OrderDate) failed validation despite matching at runtime. The check now normalizes the ORDER BY key the same way and routes both paths through tracking_column_matches, and it strips a surrounding identifier quote so a case-preserving column such as PostgreSQL's ORDER BY "OrderDate" validates against its unquoted driver label. Validation also derived the outer ordering with rfind("order by"), which matched a window function's internal ORDER BY (ROW_NUMBER() OVER (ORDER BY id) with no outer clause). That orders values within the frame, not the emitted ResultSet, so setMaxRows still returns an arbitrary subset and the advancing cursor skips rows, reopening the skip-row bug at validate time. Ordering detection now scans at parenthesis depth zero, ignoring ORDER BY inside a subquery, CTE, or OVER (...), and skips single-quoted string literals so a parenthesis in a literal cannot perturb the depth. A query whose only ordering sits inside such a construct is rejected. Update the README ORDER BY caveat to match and drop a stray line-ending backslash in the bulk example query. --- .../connectors/jdbc_bulk_mode.toml | 2 +- core/connectors/sources/jdbc_source/README.md | 13 +- .../connectors/sources/jdbc_source/src/lib.rs | 230 +++++++++++++++--- 3 files changed, 211 insertions(+), 34 deletions(-) diff --git a/core/connectors/runtime/example_config/connectors/jdbc_bulk_mode.toml b/core/connectors/runtime/example_config/connectors/jdbc_bulk_mode.toml index a179b42a50..c939d2285c 100644 --- a/core/connectors/runtime/example_config/connectors/jdbc_bulk_mode.toml +++ b/core/connectors/runtime/example_config/connectors/jdbc_bulk_mode.toml @@ -44,7 +44,7 @@ driver_jar_path = "/opt/jdbc-drivers/postgresql-42.6.0.jar" # Can include JOINs, aggregations, complex WHERE clauses, etc. query = """ SELECT - p.product_id,\ + p.product_id, p.product_name, p.category, p.price, diff --git a/core/connectors/sources/jdbc_source/README.md b/core/connectors/sources/jdbc_source/README.md index 1537d7d7b1..ee2e6ada3d 100644 --- a/core/connectors/sources/jdbc_source/README.md +++ b/core/connectors/sources/jdbc_source/README.md @@ -225,9 +225,16 @@ connector refuses to start otherwise): `ORDER BY {tracking_column}` or the column name (optionally table-qualified, e.g. `ORDER BY t.updated_at`). A composite order such as `ORDER BY other, id` (tracking column not first) or a `DESC` order is rejected at `open()`. - The `ORDER BY` check is a lexical heuristic that validates single-block - `SELECT`s; `UNION`, window-function, or CTE queries may not be validated - correctly, so verify the result ordering yourself for those. + The `ORDER BY` check is a lexical heuristic, not a full SQL parser: it inspects + the last `ORDER BY` at parenthesis depth zero, so an `ORDER BY` inside a + subquery, CTE, or a window function's `OVER (...)` is ignored rather than + mistaken for the outer ordering (a query whose only ordering sits inside such a + construct is rejected, since it has no outer `ORDER BY`). A surrounding + identifier quote (`"OrderDate"`, `` `col` ``, `[col]`) and, when + `snake_case_columns` is set, snake_case folding are both accounted for, so the + same `tracking_column` that matches a row at read time also passes validation. + The check does not interpret `UNION`; for a multi-branch `UNION` verify the + result ordering yourself. The connector takes the tracking value of the **last row** of each ordered batch as the next cursor, so the cursor always matches the database's own `ORDER BY`. diff --git a/core/connectors/sources/jdbc_source/src/lib.rs b/core/connectors/sources/jdbc_source/src/lib.rs index 7bbfd6e3d2..16e3b9e12c 100644 --- a/core/connectors/sources/jdbc_source/src/lib.rs +++ b/core/connectors/sources/jdbc_source/src/lib.rs @@ -1321,7 +1321,11 @@ impl JdbcSource { .to_string(), )); }; - if !query_orders_by_tracking_column(&self.config.query, tracking_column) { + if !query_orders_by_tracking_column( + &self.config.query, + tracking_column, + self.config.snake_case_columns, + ) { return Err(Error::InvalidConfigValue(format!( "incremental mode requires the query to order by the tracking column so each \ batch is a contiguous ascending range; add `ORDER BY {tracking_column}` (or \ @@ -1567,40 +1571,112 @@ fn strip_offset_predicate(query: &str) -> String { /// `ORDER BY`, and must not be descending. /// /// This is a lexical check, not a SQL parser, so it errs strict: it inspects the -/// last `ORDER BY` in the text (the outer query's, not a subquery's), takes the -/// first ordering term, and requires it to be the `{tracking_column}` placeholder -/// or an identifier whose final path segment equals the tracking column -/// (case-insensitive). A trailing `DESC` is rejected, and a composite -/// `ORDER BY other, tracking` (tracking not primary) is rejected, because -/// truncation then yields a prefix ordered by `other`. -fn query_orders_by_tracking_column(query: &str, tracking_column: &str) -> bool { - let lower = query.to_lowercase(); - let Some(pos) = lower.rfind("order by") else { +/// last `ORDER BY` at parenthesis depth zero (the outer query's clause, never one +/// inside a subquery or a window function's `OVER (...)`), takes the first +/// ordering term, and requires it to be the `{tracking_column}` placeholder or an +/// identifier whose final path segment equals the tracking column. Matching +/// mirrors the read-time `tracking_column_matches`: case-insensitive against the +/// raw identifier and, when `snake_case_columns` is set, against its snake_cased +/// form, so a CamelCase `ORDER BY OrderDate` with `tracking_column = "order_date"` +/// validates exactly as it matches when reading rows (otherwise validation would +/// reject a config that runs correctly). A trailing `DESC` is rejected, and a +/// composite `ORDER BY other, tracking` (tracking not primary) is rejected, +/// because truncation then yields a prefix ordered by `other`. +fn query_orders_by_tracking_column( + query: &str, + tracking_column: &str, + snake_case_columns: bool, +) -> bool { + let Some(order_by) = outer_order_by(query) else { return false; }; - let after = lower[pos + "order by".len()..].trim_start(); - let first_term = after.split(',').next().unwrap_or("").trim(); + let first_term = order_by.split(',').next().unwrap_or("").trim(); let mut tokens = first_term.split_whitespace(); let key = tokens.next().unwrap_or(""); // Any explicit descending direction breaks ascending offset advancement. - if tokens.any(|token| token == "desc") { + if tokens.any(|token| token.eq_ignore_ascii_case("desc")) { return false; } if key.starts_with("{tracking_column}") { return true; } - // Compare the final path segment (`t.updated_at` -> `updated_at`), keeping - // only leading identifier characters so trailing punctuation is ignored. - let key_ident: String = key + // Compare the final path segment (`t.updated_at` -> `updated_at`), first + // stripping a surrounding identifier quote (`"pg"`, `` `mysql` ``, `[mssql]`) + // then keeping only leading identifier characters so trailing punctuation is + // ignored. A case-preserving column such as PostgreSQL's `ORDER BY + // "OrderDate"` must validate against the same `tracking_column` that matches + // its unquoted driver label when reading rows. + let segment = key .rsplit('.') .next() .unwrap_or(key) + .trim_matches(|c| c == '"' || c == '`' || c == '[' || c == ']'); + let key_ident: String = segment .chars() .take_while(|c| c.is_ascii_alphanumeric() || *c == '_') .collect(); - let tracking_lower = tracking_column.to_lowercase(); - let tracking_ident = tracking_lower.rsplit('.').next().unwrap_or(&tracking_lower); - !key_ident.is_empty() && key_ident == tracking_ident + if key_ident.is_empty() { + return false; + } + // Normalize identically to the read-time path so validate-time cannot reject + // a config whose ORDER BY column would match a returned row at runtime. + let normalized = if snake_case_columns { + to_snake_case(&key_ident) + } else { + key_ident.clone() + }; + tracking_column_matches(tracking_column, &key_ident, &normalized) +} + +/// Return the text following the outer `ORDER BY` (the last `order by` token at +/// parenthesis depth zero), or `None` if the query has no top-level ordering. An +/// `ORDER BY` inside a subquery or a window function's `OVER (...)` sits at depth +/// greater than zero and is ignored, so a window's internal ordering (which +/// orders values within the frame, not the emitted ResultSet) is never mistaken +/// for the row-emission order. Single-quoted string literals are skipped so a +/// parenthesis or the words `order by` inside a literal cannot perturb the depth +/// or produce a spurious match. +fn outer_order_by(query: &str) -> Option<&str> { + const NEEDLE: &[u8] = b"order by"; + let bytes = query.as_bytes(); + // ASCII lowercasing is byte-length-preserving, so indices into `lower` map + // one-to-one onto `query`; this keeps the matched key in its original case. + let lower = query.to_ascii_lowercase(); + let lower_bytes = lower.as_bytes(); + let mut depth: u32 = 0; + let mut in_quote = false; + let mut last_end = None; + for i in 0..bytes.len() { + let byte = bytes[i]; + if in_quote { + if byte == b'\'' { + in_quote = false; + } + continue; + } + match byte { + b'\'' => in_quote = true, + b'(' => depth += 1, + b')' => depth = depth.saturating_sub(1), + _ if depth == 0 + && lower_bytes[i..].starts_with(NEEDLE) + && (i == 0 || !is_identifier_byte(bytes[i - 1])) => + { + let end = i + NEEDLE.len(); + if bytes.get(end).is_none_or(|next| !is_identifier_byte(*next)) { + last_end = Some(end); + } + } + _ => {} + } + } + last_end.map(|end| query[end..].trim_start()) +} + +/// Whether a byte is part of an unquoted SQL identifier (ASCII alphanumeric or +/// `_`), used to require whole-token boundaries around a matched keyword. +fn is_identifier_byte(byte: u8) -> bool { + byte.is_ascii_alphanumeric() || byte == b'_' } /// Whether a configured `tracking_column` name refers to this result column. @@ -1971,25 +2047,30 @@ mod tests { fn test_query_orders_by_tracking_column_accepts_valid() { assert!(query_orders_by_tracking_column( "SELECT * FROM t WHERE id > {last_offset} ORDER BY id", - "id" + "id", + false )); // Placeholder form, case/whitespace variance, explicit ASC. assert!(query_orders_by_tracking_column( "select * from t order by {tracking_column}", - "updated_at" + "updated_at", + false )); assert!(query_orders_by_tracking_column( "SELECT * FROM t ORDER BY Updated_At ASC", - "updated_at" + "updated_at", + false )); // Table-qualified column, and the outer ORDER BY after a subquery. assert!(query_orders_by_tracking_column( "SELECT * FROM t ORDER BY t.updated_at", - "updated_at" + "updated_at", + false )); assert!(query_orders_by_tracking_column( "SELECT * FROM (SELECT * FROM t ORDER BY x) s ORDER BY id", - "id" + "id", + false )); } @@ -1998,31 +2079,120 @@ mod tests { // No ORDER BY. assert!(!query_orders_by_tracking_column( "SELECT * FROM t WHERE id > 0", - "id" + "id", + false )); // Different column. assert!(!query_orders_by_tracking_column( "SELECT * FROM t ORDER BY name", - "id" + "id", + false )); // Descending breaks ascending offset advancement. assert!(!query_orders_by_tracking_column( "SELECT * FROM t ORDER BY updated_at DESC", - "updated_at" + "updated_at", + false )); // Substring-only match must not pass (id is a substring of valid_flag/id_backup). assert!(!query_orders_by_tracking_column( "SELECT * FROM t ORDER BY valid_flag", - "id" + "id", + false )); assert!(!query_orders_by_tracking_column( "SELECT * FROM t ORDER BY id_backup", - "id" + "id", + false )); // Tracking column not the primary (first) ordering term. assert!(!query_orders_by_tracking_column( "SELECT * FROM t ORDER BY name, id", - "id" + "id", + false + )); + } + + #[test] + fn test_query_orders_by_tracking_column_ignores_window_order_by() { + // A window function's internal ORDER BY orders values within the frame, + // not the emitted ResultSet, so it must not satisfy the outer-ordering + // requirement even though it is the only `order by` in the text. + assert!(!query_orders_by_tracking_column( + "SELECT id, ROW_NUMBER() OVER (ORDER BY id) rn FROM t", + "id", + false + )); + assert!(!query_orders_by_tracking_column( + "SELECT id, ROW_NUMBER() OVER (ORDER BY id) rn FROM t WHERE id > {last_offset}", + "id", + false + )); + // A window ORDER BY plus a genuine outer ORDER BY is accepted on the outer. + assert!(query_orders_by_tracking_column( + "SELECT id, ROW_NUMBER() OVER (ORDER BY x) rn FROM t ORDER BY id", + "id", + false + )); + // A parenthesis inside a string literal must not shift the depth and hide + // the real outer ORDER BY. + assert!(query_orders_by_tracking_column( + "SELECT * FROM t WHERE note = 'a (b' ORDER BY id", + "id", + false + )); + } + + #[test] + fn test_query_orders_by_tracking_column_snake_case_matches_read_time() { + // snake_case_columns = true: a CamelCase ORDER BY column with a + // snake_cased tracking_column validates, mirroring tracking_column_matches + // so validate-time never rejects a config that reads rows correctly. + assert!(query_orders_by_tracking_column( + "SELECT * FROM t ORDER BY OrderDate", + "order_date", + true + )); + // Without normalization enabled the same pair does not match (the driver + // label would not be snake_cased at read time either). + assert!(!query_orders_by_tracking_column( + "SELECT * FROM t ORDER BY OrderDate", + "order_date", + false + )); + // Raw label match still works regardless of the normalization flag. + assert!(query_orders_by_tracking_column( + "SELECT * FROM t ORDER BY OrderDate", + "orderdate", + true + )); + } + + #[test] + fn test_query_orders_by_tracking_column_strips_identifier_quotes() { + // PostgreSQL preserves case only for quoted identifiers, so a genuinely + // CamelCase column is ordered as `"OrderDate"`; the surrounding quotes + // must not defeat the match against its unquoted driver label. + assert!(query_orders_by_tracking_column( + r#"SELECT * FROM t ORDER BY "OrderDate""#, + "order_date", + true + )); + assert!(query_orders_by_tracking_column( + r#"SELECT * FROM t ORDER BY "updated_at""#, + "updated_at", + false + )); + // MySQL backtick and SQL Server bracket quoting. + assert!(query_orders_by_tracking_column( + "SELECT * FROM t ORDER BY `updated_at`", + "updated_at", + false + )); + assert!(query_orders_by_tracking_column( + "SELECT * FROM t ORDER BY [updated_at]", + "updated_at", + false )); } From d22fa4eaecd6fbeb95f8896d5c3a54e14ce710d1 Mon Sep 17 00:00:00 2001 From: shbhmrzd Date: Tue, 4 Aug 2026 21:13:58 +0530 Subject: [PATCH 06/11] fix(connectors): harden JDBC source startup, types, and secret docs open() booted the JVM and ran DriverManager.getConnection synchronously and unbounded, so a single unreachable or slow-DNS database hung the whole connectors runtime at startup (sources open sequentially, and the FFI drives open() via block_on), taking every other source, sink, and the HTTP control API with it. Wrap the JVM init and connection in block_in_place like poll()/close(), and add login_timeout_ms (DriverManager.setLoginTimeout) so a stuck connect fails instead of blocking forever. BIGINT was emitted as a JSON number, silently rounding values above 2^53 in consumers that parse JSON numbers as f64. Emit it as a string, the same as NUMERIC/DECIMAL already do. The incremental cursor comment claimed it "degrades to re-reads rather than skips", which is false when a run of rows sharing one tracking value is split across a setMaxRows batch boundary: the remainder is skipped. Correct the comment and warn at validate time that the tracking column must be unique or batch_size must exceed the largest tie group. Hoist the jni dependency to the workspace so the pin is shared with the upcoming JDBC sink rather than duplicated. Document that SecretString redaction is not end-to-end: the runtime's generic configs/plugin control endpoint and trace-level config logging emit the raw plugin_config for every connector. --- Cargo.toml | 1 + .../connectors/sources/jdbc_source/Cargo.toml | 2 +- core/connectors/sources/jdbc_source/README.md | 12 ++- .../connectors/sources/jdbc_source/src/lib.rs | 100 ++++++++++++++++-- 4 files changed, 104 insertions(+), 11 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index b24e95296d..da5a6d204c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -207,6 +207,7 @@ iggy_common = { path = "core/common", version = "0.10.3-edge.3" } iggy_connector_sdk = { path = "core/connectors/sdk", version = "0.3.1-edge.1" } indexmap = "2.14.0" integration = { path = "core/integration" } +jni = { version = "0.21", features = ["invocation"] } journal = { path = "core/journal" } js-sys = "0.3" jsonwebtoken = { version = "10.4.0", features = ["rust_crypto"] } diff --git a/core/connectors/sources/jdbc_source/Cargo.toml b/core/connectors/sources/jdbc_source/Cargo.toml index 839b7e424a..686fcdb419 100644 --- a/core/connectors/sources/jdbc_source/Cargo.toml +++ b/core/connectors/sources/jdbc_source/Cargo.toml @@ -53,7 +53,7 @@ iggy_common = { workspace = true } iggy_connector_sdk = { workspace = true } # JNI for Java interop with invocation support -jni = { version = "0.21", features = ["invocation"] } +jni = { workspace = true } # For sanitizing passwords in logs regex = { workspace = true } diff --git a/core/connectors/sources/jdbc_source/README.md b/core/connectors/sources/jdbc_source/README.md index ee2e6ada3d..139e3f6f4e 100644 --- a/core/connectors/sources/jdbc_source/README.md +++ b/core/connectors/sources/jdbc_source/README.md @@ -200,6 +200,7 @@ topic = "orders" | `initial_offset` | string | No | - | Starting offset value for first poll | | `mode` | string | No | "incremental" | Sync mode: "incremental" or "bulk" (bulk works with ALL databases) | | `connection_timeout_ms` | u64 | No | 5000 | Timeout (ms) for the per-poll `isValid` liveness check; converted to seconds and capped at 5s | +| `login_timeout_ms` | u64 | No | 30000 | Bound on establishing the connection (`DriverManager.setLoginTimeout`); rounded up to whole seconds. Stops an unreachable database from hanging startup | | `jvm_options` | array | No | [] | Custom JVM options (e.g., ["-Xmx1g"]) | | `snake_case_columns` | bool | No | false | Convert column names to snake_case | | `include_metadata` | bool | No | true | Include metadata (table, operation, timestamp) | @@ -316,7 +317,7 @@ JDBC SQL types are automatically mapped to JSON: | ---------- | ----------- | ------- | | BIT, BOOLEAN | boolean | - | | TINYINT, SMALLINT, INTEGER | number | Integer | -| BIGINT | number | Long integer (values above 2^53 may lose precision in JSON consumers that parse numbers as f64) | +| BIGINT | string | Emitted as a string to preserve full 64-bit precision (many JSON consumers parse numbers as f64 and would lose precision above 2^53) | | FLOAT, REAL | number | Float | | DOUBLE | number | Double | | NUMERIC, DECIMAL | string | Emitted as a string to preserve arbitrary precision (e.g. money) | @@ -365,6 +366,15 @@ JDBC SQL types are automatically mapped to JSON: so the password is copied onto the JVM heap as an ordinary (non-zeroed) string for the lifetime of the connection. This is inherent to the JDBC surface and is an accepted risk. +- **`SecretString` redaction is not end-to-end.** `jdbc_url`/`password` are typed + as `SecretString`, and this connector's `Debug` output and startup logs redact + them. That redaction does **not** cover the connectors runtime's generic + config surface: the `GET /sources/{key}/configs/plugin` control-API endpoint + and the runtime's `trace`-level config logging emit the raw, untyped + `plugin_config` (credentials included), the same as every other connector. Do + not expose the control API to untrusted callers and do not run the runtime at + `trace` level in production. This is a known runtime-wide limitation shared by + all connectors, not something this connector can address on its own. ### Credential precedence diff --git a/core/connectors/sources/jdbc_source/src/lib.rs b/core/connectors/sources/jdbc_source/src/lib.rs index 16e3b9e12c..890b33c789 100644 --- a/core/connectors/sources/jdbc_source/src/lib.rs +++ b/core/connectors/sources/jdbc_source/src/lib.rs @@ -212,12 +212,24 @@ pub struct JdbcSourceConfig { /// establishment. #[serde(default = "default_connection_timeout")] pub connection_timeout_ms: u64, + + /// Bound on establishing the JDBC connection (default: 30000). Applied via + /// `DriverManager.setLoginTimeout`, which is expressed in whole seconds, so + /// the value is rounded up to at least 1s. Prevents an unreachable or + /// slow-DNS database from hanging `open()` (and thus the whole connectors + /// runtime, which opens sources sequentially at startup) indefinitely. + #[serde(default = "default_login_timeout")] + pub login_timeout_ms: u64, } fn default_connection_timeout() -> u64 { 5000 } +fn default_login_timeout() -> u64 { + 30000 +} + fn default_batch_size() -> u32 { 1000 } @@ -250,6 +262,7 @@ impl std::fmt::Debug for JdbcSourceConfig { .field("snake_case_columns", &self.snake_case_columns) .field("include_metadata", &self.include_metadata) .field("connection_timeout_ms", &self.connection_timeout_ms) + .field("login_timeout_ms", &self.login_timeout_ms) .finish() } } @@ -492,6 +505,27 @@ impl JdbcSource { "Failed to create JDBC URL string" ); + // Bound connection establishment so an unreachable or slow-DNS database + // fails instead of hanging open() (which the runtime drives sequentially + // at startup) forever. setLoginTimeout is process-wide and in whole + // seconds, so round the configured milliseconds up to at least 1s. + let login_timeout_secs = self + .config + .login_timeout_ms + .div_ceil(1000) + .clamp(1, i32::MAX as u64) as i32; + jni_init!( + env, + env.call_static_method( + &driver_manager, + "setLoginTimeout", + "(I)V", + &[JValue::Int(login_timeout_secs)], + ) + .and_then(|v| v.v()), + "Failed to set JDBC login timeout" + ); + // If username/password are provided separately, use 3-arg getConnection let connection_obj = if let (Some(username), Some(password)) = (&self.config.username, &self.config.password) @@ -829,9 +863,16 @@ impl JdbcSource { // arrive in ascending tracking order (validate_config enforces // ORDER BY the tracking column ascending), so the last row is the // high-water mark. Using the last row rather than a Rust-side max - // keeps the cursor consistent with the database's own ordering, and - // degrades to re-reads (safe, at-least-once) rather than skips if the - // ordering is ever imperfect. + // keeps the cursor consistent with the database's own ordering. + // + // The next poll resumes with a strict `> last_offset`, so if this + // setMaxRows-capped batch ends in the middle of a run of rows sharing + // one tracking value (a non-unique column such as a timestamp), the + // remaining tied rows are skipped. The tracking column must therefore + // be unique / strictly increasing, or batch_size must exceed the + // largest group of equal values. See the README tracking-column + // requirements; keyset pagination with a tie-break is a planned + // follow-up. if let Some(offset) = offset { last_offset = Some(offset); } @@ -1112,6 +1153,9 @@ impl JdbcSource { ); self.null_or(env, result_set, serde_json::json!(value)) } + // BIGINT is emitted as a string (like NUMERIC/DECIMAL below) so a + // value above 2^53 is not silently rounded by a JSON consumer that + // parses numbers as f64. Types::BIGINT => { let value = jni!( env, @@ -1119,7 +1163,7 @@ impl JdbcSource { .and_then(|v| v.j()), "Failed to get long" ); - self.null_or(env, result_set, serde_json::json!(value)) + self.null_or(env, result_set, serde_json::json!(value.to_string())) } Types::FLOAT | Types::REAL => { let value = jni!( @@ -1332,6 +1376,19 @@ impl JdbcSource { `ORDER BY {{tracking_column}}`) to the query" ))); } + + // The cursor advances with a strict `> last_offset` per batch, so a + // group of rows sharing one tracking value that is split across a + // batch_size boundary loses its remainder. This cannot be detected + // without inspecting the data, so warn: the column should be unique / + // strictly increasing, or batch_size must exceed the largest tie group. + warn!( + "JDBC source [{}] incremental tracking_column '{tracking_column}': ensure it is \ + unique / strictly increasing, or that batch_size ({}) exceeds the largest group \ + of rows sharing one value. A tie split across a batch boundary skips the \ + remaining rows (common for non-unique timestamp columns).", + self.id, self.config.batch_size + ); } // Dry-run the query build so an unresolved placeholder or an invalid @@ -1356,11 +1413,15 @@ impl Source for JdbcSource { // Fail fast on bad config before starting the JVM or opening a connection. self.validate_config()?; - // Initialize JVM - self.initialize_jvm()?; - - // Create database connection - self.create_connection()?; + // JVM boot and DriverManager.getConnection are blocking JNI work. Run + // them via block_in_place (like poll()/close()) so they do not monopolize + // a shared async-runtime worker; the login timeout set in the connection + // path bounds how long a stuck connect can block. + tokio::task::block_in_place(|| -> Result<(), Error> { + self.initialize_jvm()?; + self.create_connection()?; + Ok(()) + })?; info!("JDBC source connector [{}] opened successfully", self.id); Ok(()) @@ -1836,6 +1897,7 @@ mod tests { include_metadata: true, jvm_options: vec![], connection_timeout_ms: 30000, + login_timeout_ms: 30000, } } @@ -2386,6 +2448,7 @@ mod tests { include_metadata: true, jvm_options: vec![], connection_timeout_ms: 30000, + login_timeout_ms: 30000, }; let source = JdbcSource::new(1, config, None); @@ -2428,6 +2491,7 @@ mod tests { include_metadata: true, jvm_options: vec![], connection_timeout_ms: 30000, + login_timeout_ms: 30000, }; let source = JdbcSource::new(1, config, None); let state = State { @@ -2462,6 +2526,7 @@ mod tests { include_metadata: true, jvm_options: vec![], connection_timeout_ms: 30000, + login_timeout_ms: 30000, }; let source = JdbcSource::new(1, config, None); // No last_offset and no initial_offset: the WHERE predicate is dropped, @@ -2496,6 +2561,7 @@ mod tests { include_metadata: true, jvm_options: vec![], connection_timeout_ms: 30000, + login_timeout_ms: 30000, }; let source = JdbcSource::new(1, config, None); assert!(source.build_query(&State::default()).is_err()); @@ -2519,6 +2585,7 @@ mod tests { include_metadata: false, jvm_options: vec![], connection_timeout_ms: 30000, + login_timeout_ms: 30000, }; let source = JdbcSource::new(1, config, None); let state = State::default(); @@ -2554,6 +2621,7 @@ mod tests { include_metadata: true, jvm_options: vec![], connection_timeout_ms: 30000, + login_timeout_ms: 30000, }; let source = JdbcSource::new(1, config, Some(connector_state)); let state = source.state.lock().unwrap(); @@ -2743,6 +2811,7 @@ mod tests { include_metadata: true, jvm_options: vec![], connection_timeout_ms: 30000, + login_timeout_ms: 30000, }; let source = JdbcSource::new(1, config, Some(connector_state)); let state = source.state.lock().unwrap(); @@ -2771,6 +2840,7 @@ mod tests { include_metadata: true, jvm_options: vec![], connection_timeout_ms: 30000, + login_timeout_ms: 30000, }; let source = JdbcSource::new(1, config, Some(connector_state)); let state = source.state.lock().unwrap(); @@ -2796,6 +2866,7 @@ mod tests { include_metadata: true, jvm_options: vec![], connection_timeout_ms: 30000, + login_timeout_ms: 30000, }; let source = JdbcSource::new(1, config, None); let state = source.state.lock().unwrap(); @@ -2821,6 +2892,7 @@ mod tests { include_metadata: true, jvm_options: vec![], connection_timeout_ms: 30000, + login_timeout_ms: 30000, }; let source = JdbcSource::new(1, config, None); let state = source.state.lock().unwrap(); @@ -2850,6 +2922,7 @@ mod tests { include_metadata: true, jvm_options: vec![], connection_timeout_ms: 30000, + login_timeout_ms: 30000, }; let source = JdbcSource::new(1, config, None); @@ -2879,6 +2952,7 @@ mod tests { include_metadata: true, jvm_options: vec![], connection_timeout_ms: 30000, + login_timeout_ms: 30000, }; let source = JdbcSource::new(1, config, None); @@ -2911,6 +2985,7 @@ mod tests { include_metadata: true, jvm_options: vec![], connection_timeout_ms: 30000, + login_timeout_ms: 30000, }; let source = JdbcSource::new(1, config, None); @@ -2940,6 +3015,7 @@ mod tests { include_metadata: true, jvm_options: vec![], connection_timeout_ms: 30000, + login_timeout_ms: 30000, }; let source = JdbcSource::new(1, config, None); @@ -2967,6 +3043,7 @@ mod tests { include_metadata: true, jvm_options: vec![], connection_timeout_ms: 30000, + login_timeout_ms: 30000, }; let source = JdbcSource::new(1, config, None); @@ -2997,6 +3074,7 @@ mod tests { include_metadata: true, jvm_options: vec![], connection_timeout_ms: 30000, + login_timeout_ms: 30000, }; let source = JdbcSource::new(1, config, None); let state = State { @@ -3034,6 +3112,7 @@ mod tests { include_metadata: true, jvm_options: vec![], connection_timeout_ms: 30000, + login_timeout_ms: 30000, }; let source = JdbcSource::new(1, config, None); assert!(source.build_query(&State::default()).is_err()); @@ -3057,6 +3136,7 @@ mod tests { include_metadata: true, jvm_options: vec![], connection_timeout_ms: 30000, + login_timeout_ms: 30000, }; let source = JdbcSource::new(1, config, None); let state = State { @@ -3119,6 +3199,7 @@ mod tests { include_metadata: true, jvm_options: vec![], connection_timeout_ms: 30000, + login_timeout_ms: 30000, }; let debug_output = format!("{:?}", config); @@ -3157,6 +3238,7 @@ mod tests { include_metadata: true, jvm_options: vec![], connection_timeout_ms: 30000, + login_timeout_ms: 30000, }; let debug_output = format!("{:?}", config); From 3e0dfa496f784c04633f2eda7e6361f9a2e2ed0d Mon Sep 17 00:00:00 2001 From: shbhmrzd Date: Tue, 4 Aug 2026 21:33:26 +0530 Subject: [PATCH 07/11] docs(connectors): correct JDBC source metadata and dedup overclaims A proactive claims-vs-code sweep found two README statements the code does not back. `table_name` is a reserved field hardcoded to null, never derived from the query, yet the metadata was advertised as including the table name; document that it is always null and that operation_type is always SELECT. "Prevents duplicate reads" contradicted the connector's own at-least-once guarantee; reword to "avoids re-reading rows below the tracked offset (at-least-once, not exactly-once)". --- core/connectors/sources/jdbc_source/README.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/core/connectors/sources/jdbc_source/README.md b/core/connectors/sources/jdbc_source/README.md index 139e3f6f4e..ab6b81ab88 100644 --- a/core/connectors/sources/jdbc_source/README.md +++ b/core/connectors/sources/jdbc_source/README.md @@ -13,7 +13,7 @@ This connector reads data from relational databases using JDBC (Java Database Co - **Bulk Mode**: Re-runs the query each poll for snapshots (capped at `batch_size` rows; see limitations) - **Type Mapping**: Automatic conversion of SQL types to JSON - **Configurable Polling**: Control how frequently data is fetched -- **State Management**: Automatically tracks offsets to prevent duplicate reads +- **State Management**: Tracks offsets so rows below the cursor are not re-read (at-least-once across restarts, not exactly-once) - **Flexible Queries**: Support for custom SQL queries with placeholders ## Supported Databases @@ -203,7 +203,7 @@ topic = "orders" | `login_timeout_ms` | u64 | No | 30000 | Bound on establishing the connection (`DriverManager.setLoginTimeout`); rounded up to whole seconds. Stops an unreachable database from hanging startup | | `jvm_options` | array | No | [] | Custom JVM options (e.g., ["-Xmx1g"]) | | `snake_case_columns` | bool | No | false | Convert column names to snake_case | -| `include_metadata` | bool | No | true | Include metadata (table, operation, timestamp) | +| `include_metadata` | bool | No | true | Wrap each row with metadata (operation type, timestamp). `table_name` is a reserved field and is currently always null | ## Query Placeholders @@ -298,6 +298,9 @@ Each database row is converted to a JSON message: } ``` +`table_name` is always `null` today: it is a reserved field, not derived +from the query. `operation_type` is always `"SELECT"`. + ### Without Metadata ```json @@ -538,7 +541,7 @@ query = "SELECT * FROM table WHERE {tracking_column} > {last_offset} ORDER BY {t **Benefits:** -- Prevents duplicate reads +- Avoids re-reading rows below the tracked offset (at-least-once, not exactly-once) - Tracks offset automatically - Efficient for large tables - Works with timestamps, IDs, or any orderable column From c62990483b4b147a1047d9561a2bd8497e3198d8 Mon Sep 17 00:00:00 2001 From: shbhmrzd Date: Wed, 5 Aug 2026 12:28:37 +0530 Subject: [PATCH 08/11] fix(connectors): address JDBC source review must-fixes Harden outer_order_by() to skip SQL line (--) and block (/* */) comments, so a commented-out ORDER BY can no longer satisfy the incremental ordering requirement and let an unordered query pass validation and skip rows at runtime. Complete JNI exception clearing in throwable_string_method(): a Java exception raised by getMessage()/getSQLState() or the string conversion is now cleared before returning, so the next JNI call on the thread is not aborted for running with an exception pending. Drop the Serialize derive (and the serialize_secret helpers) from JdbcSourceConfig: the runtime only deserializes it, and serializing would risk writing the jdbc_url/password SecretStrings in plaintext. This also removes the now-unused iggy_common dependency. Redacted logging still goes through the manual Debug impl. Read column names with ResultSetMetaData.getColumnLabel() instead of getColumnName() so a SELECT expr AS alias yields the alias the caller configured rather than the base-table column (or an empty string). Remove the unused connectors_api_address() test helper that failed integration clippy, and make the JDBC integration tests panic on a Postgres/testcontainers setup failure instead of returning early and reporting a false pass. --- Cargo.lock | 1 - .../connectors/sources/jdbc_source/Cargo.toml | 3 - .../connectors/sources/jdbc_source/src/lib.rs | 98 ++++++++++++++++--- .../connectors/jdbc/test_with_postgres.rs | 35 ++----- core/integration/tests/connectors/mod.rs | 7 -- 5 files changed, 89 insertions(+), 55 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bc41278d57..76a69133e2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7072,7 +7072,6 @@ dependencies = [ "chrono", "dashmap", "humantime", - "iggy_common", "iggy_connector_sdk", "jni 0.21.1", "regex", diff --git a/core/connectors/sources/jdbc_source/Cargo.toml b/core/connectors/sources/jdbc_source/Cargo.toml index 686fcdb419..425f6cb554 100644 --- a/core/connectors/sources/jdbc_source/Cargo.toml +++ b/core/connectors/sources/jdbc_source/Cargo.toml @@ -46,9 +46,6 @@ dashmap = { workspace = true } # For parsing duration strings (poll_interval) humantime = { workspace = true } -# Shared serde helpers (SecretString serialization) -iggy_common = { workspace = true } - # Connector SDK iggy_connector_sdk = { workspace = true } diff --git a/core/connectors/sources/jdbc_source/src/lib.rs b/core/connectors/sources/jdbc_source/src/lib.rs index 890b33c789..5e9a449cb5 100644 --- a/core/connectors/sources/jdbc_source/src/lib.rs +++ b/core/connectors/sources/jdbc_source/src/lib.rs @@ -17,7 +17,6 @@ use async_trait::async_trait; use chrono::{DateTime, Utc}; -use iggy_common::serde_secret; use iggy_connector_sdk::{ ConnectorState, Error, ProducedMessage, ProducedMessages, Schema, Source, source_connector, }; @@ -146,12 +145,16 @@ pub enum Mode { Incremental, } -/// Configuration for JDBC source connector -#[derive(Clone, Deserialize, Serialize)] +/// Configuration for JDBC source connector. +/// +/// Deliberately does NOT derive `Serialize`: the runtime only ever deserializes +/// this from config, and serializing it would risk writing `jdbc_url`/`password` +/// (both `SecretString`) to logs or state in plaintext. Redacted logging goes +/// through the manual `Debug` impl below. +#[derive(Clone, Deserialize)] pub struct JdbcSourceConfig { /// JDBC connection URL (e.g., "jdbc:mysql://localhost:3306/mydb") /// Can include credentials: "jdbc:mysql://localhost:3306/mydb?user=root&password=secret" - #[serde(serialize_with = "serde_secret::serialize_secret")] pub jdbc_url: SecretString, /// JDBC driver class name (e.g., "com.mysql.cj.jdbc.Driver") @@ -165,7 +168,7 @@ pub struct JdbcSourceConfig { pub username: Option, /// Database password (optional if included in jdbc_url) - #[serde(default, serialize_with = "serde_secret::serialize_optional_secret")] + #[serde(default)] pub password: Option, /// SQL query to execute for fetching data @@ -813,7 +816,7 @@ impl JdbcSource { // negative i32 would sign-extend to an enormous usize and abort on alloc. let mut columns = Vec::with_capacity((column_count.max(0) as usize).min(8192)); for i in 1..=column_count { - let col_name = self.get_column_name(env, &metadata, i)?; + let col_name = self.get_column_label(env, &metadata, i)?; let col_type = self.get_column_type(env, &metadata, i)?; columns.push((col_name, col_type)); } @@ -1042,8 +1045,12 @@ impl JdbcSource { finalize_query(query) } - /// Get column name from ResultSetMetaData - fn get_column_name( + /// Get the column label from ResultSetMetaData. Uses `getColumnLabel` (not + /// `getColumnName`) so a `SELECT expr AS alias` yields the alias the caller + /// asked for; `getColumnName` returns the underlying base-table column (or + /// empty for computed columns), which would not match a configured + /// `tracking_column` alias and can be blank. + fn get_column_label( &self, env: &mut JNIEnv, metadata: &JObject, @@ -1053,12 +1060,12 @@ impl JdbcSource { env, env.call_method( metadata, - "getColumnName", + "getColumnLabel", "(I)Ljava/lang/String;", &[JValue::Int(column_index)], ) .and_then(|v| v.l()), - "Failed to get column name" + "Failed to get column label" ); let col_name: String = jni!( @@ -1707,12 +1714,33 @@ fn outer_order_by(query: &str) -> Option<&str> { let mut depth: u32 = 0; let mut in_quote = false; let mut last_end = None; - for i in 0..bytes.len() { + let mut i = 0; + while i < bytes.len() { let byte = bytes[i]; if in_quote { if byte == b'\'' { in_quote = false; } + i += 1; + continue; + } + // Skip SQL comments entirely: an `ORDER BY` (or a stray paren/quote) + // inside a comment must not be mistaken for the outer ordering, which + // would let a query with no real ordering pass validation and then skip + // rows at runtime. + if byte == b'-' && bytes.get(i + 1) == Some(&b'-') { + i += 2; + while i < bytes.len() && bytes[i] != b'\n' { + i += 1; + } + continue; + } + if byte == b'/' && bytes.get(i + 1) == Some(&b'*') { + i += 2; + while i < bytes.len() && !(bytes[i] == b'*' && bytes.get(i + 1) == Some(&b'/')) { + i += 1; + } + i += 2; // consume the closing `*/` continue; } match byte { @@ -1730,6 +1758,7 @@ fn outer_order_by(query: &str) -> Option<&str> { } _ => {} } + i += 1; } last_end.map(|end| query[end..].trim_start()) } @@ -1823,20 +1852,34 @@ fn take_pending_sql_exception(env: &mut JNIEnv) -> (Option, String) { } /// Call a no-arg `String`-returning method on a throwable; None on JNI error/null. +/// Any pending exception raised by the call itself is cleared before returning, +/// so a later JNI call on this thread is not aborted for running with an +/// exception pending (JNI forbids that). fn throwable_string_method( env: &mut JNIEnv, throwable: &JThrowable, method: &str, ) -> Option { - let obj = env + let obj = match env .call_method(throwable, method, "()Ljava/lang/String;", &[]) - .ok()? - .l() - .ok()?; + .and_then(|v| v.l()) + { + Ok(obj) => obj, + Err(_) => { + clear_pending_exception(env); + return None; + } + }; if obj.is_null() { return None; } - env.get_string(&JString::from(obj)).ok().map(|s| s.into()) + match env.get_string(&JString::from(obj)) { + Ok(s) => Some(s.into()), + Err(_) => { + clear_pending_exception(env); + None + } + } } /// JDBC SQL Types constants @@ -2203,6 +2246,29 @@ mod tests { "id", false )); + // An ORDER BY inside a SQL comment must NOT satisfy the requirement: the + // real query has no ordering, so accepting it would skip rows at runtime. + assert!(!query_orders_by_tracking_column( + "SELECT * FROM t -- ORDER BY id\n", + "id", + false + )); + assert!(!query_orders_by_tracking_column( + "SELECT * FROM t /* ORDER BY id */", + "id", + false + )); + // A comment before the genuine outer ORDER BY must not hide it. + assert!(query_orders_by_tracking_column( + "SELECT * FROM t /* pick a key */ ORDER BY id", + "id", + false + )); + assert!(query_orders_by_tracking_column( + "SELECT * FROM t -- note\n ORDER BY id", + "id", + false + )); } #[test] diff --git a/core/integration/tests/connectors/jdbc/test_with_postgres.rs b/core/integration/tests/connectors/jdbc/test_with_postgres.rs index 6004bf80fe..2e4c226f39 100644 --- a/core/integration/tests/connectors/jdbc/test_with_postgres.rs +++ b/core/integration/tests/connectors/jdbc/test_with_postgres.rs @@ -219,10 +219,7 @@ async fn setup_jdbc_postgres_source( async fn bulk_query_produces_message_to_iggy() { let (_postgres_container, jdbc_url, postgres_jar) = match setup_postgres_container().await { Ok(result) => result, - Err(e) => { - eprintln!("Skipping test: Failed to setup Postgres: {}", e); - return; - } + Err(e) => panic!("Failed to set up Postgres container: {e}"), }; let query = "SELECT 1 as id, 'test' as name"; @@ -271,10 +268,7 @@ async fn bulk_query_produces_message_to_iggy() { async fn bulk_query_produces_multiple_rows_to_iggy() { let (postgres_container, jdbc_url, postgres_jar) = match setup_postgres_container().await { Ok(result) => result, - Err(e) => { - eprintln!("Skipping test: Failed to setup Postgres: {}", e); - return; - } + Err(e) => panic!("Failed to set up Postgres container: {e}"), }; // Use a multi-row SELECT to simulate table data without needing DDL @@ -331,10 +325,7 @@ async fn bulk_query_produces_multiple_rows_to_iggy() { async fn source_includes_metadata_fields_when_enabled() { let (_postgres_container, jdbc_url, postgres_jar) = match setup_postgres_container().await { Ok(result) => result, - Err(e) => { - eprintln!("Skipping test: Failed to setup Postgres: {}", e); - return; - } + Err(e) => panic!("Failed to set up Postgres container: {e}"), }; let query = "SELECT 42 as value"; @@ -395,10 +386,7 @@ fn collect_ids(messages: &[serde_json::Value]) -> Vec { async fn incremental_mode_advances_offset_across_polls() { let (_container, jdbc_url, postgres_jar) = match setup_postgres_container().await { Ok(result) => result, - Err(e) => { - eprintln!("Skipping test: Failed to setup Postgres: {e}"); - return; - } + Err(e) => panic!("Failed to set up Postgres container: {e}"), }; // Seed a real table BEFORE the source starts polling. @@ -454,10 +442,7 @@ async fn incremental_mode_advances_offset_across_polls() { async fn large_result_set_streams_without_crashing() { let (_container, jdbc_url, postgres_jar) = match setup_postgres_container().await { Ok(result) => result, - Err(e) => { - eprintln!("Skipping test: Failed to setup Postgres: {e}"); - return; - } + Err(e) => panic!("Failed to set up Postgres container: {e}"), }; let pool = PgPoolOptions::new() @@ -524,10 +509,7 @@ async fn large_result_set_streams_without_crashing() { async fn source_recovers_after_repeated_query_errors() { let (_container, jdbc_url, postgres_jar) = match setup_postgres_container().await { Ok(result) => result, - Err(e) => { - eprintln!("Skipping test: Failed to setup Postgres: {e}"); - return; - } + Err(e) => panic!("Failed to set up Postgres container: {e}"), }; // Start the source against a table that does not exist yet: every poll @@ -574,10 +556,7 @@ async fn source_recovers_after_repeated_query_errors() { async fn bulk_result_larger_than_batch_size_fails_closed() { let (_container, jdbc_url, postgres_jar) = match setup_postgres_container().await { Ok(result) => result, - Err(e) => { - eprintln!("Skipping test: Failed to setup Postgres: {e}"); - return; - } + Err(e) => panic!("Failed to set up Postgres container: {e}"), }; let pool = PgPoolOptions::new() diff --git a/core/integration/tests/connectors/mod.rs b/core/integration/tests/connectors/mod.rs index 59f6126178..4859397730 100644 --- a/core/integration/tests/connectors/mod.rs +++ b/core/integration/tests/connectors/mod.rs @@ -244,11 +244,4 @@ impl ConnectorsRuntime { .await .expect("Failed to create root TCP client") } - - pub fn connectors_api_address(&self) -> Option { - self.harness - .server() - .connectors_runtime() - .map(|cr| cr.http_address().to_string()) - } } From 6187707a1131445632fb2c7498cda54dda4767ef Mon Sep 17 00:00:00 2001 From: shbhmrzd Date: Wed, 5 Aug 2026 17:35:24 +0530 Subject: [PATCH 09/11] fix(connectors): fix JDBC CI lint failures (typos, TOML format) Correct "unparseable" -> "unparsable" in poll_interval doc comments and code comments (flagged by the typos check), and reflow jdbc_oracle.toml to taplo's canonical format (a stray blank line). --- .../runtime/example_config/connectors/jdbc_oracle.toml | 1 - core/connectors/sources/jdbc_source/src/lib.rs | 8 ++++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/core/connectors/runtime/example_config/connectors/jdbc_oracle.toml b/core/connectors/runtime/example_config/connectors/jdbc_oracle.toml index 92d5a1e99e..1610ddf7b1 100644 --- a/core/connectors/runtime/example_config/connectors/jdbc_oracle.toml +++ b/core/connectors/runtime/example_config/connectors/jdbc_oracle.toml @@ -70,7 +70,6 @@ snake_case_columns = true # Include metadata wrapper in output messages include_metadata = true - # Custom JVM options (optional) jvm_options = ["-Xmx512m", "-Xms256m"] diff --git a/core/connectors/sources/jdbc_source/src/lib.rs b/core/connectors/sources/jdbc_source/src/lib.rs index 5e9a449cb5..2d1165b8bb 100644 --- a/core/connectors/sources/jdbc_source/src/lib.rs +++ b/core/connectors/sources/jdbc_source/src/lib.rs @@ -119,8 +119,8 @@ const CONNECTOR_NAME: &str = "JDBC source"; const DEFAULT_POLL_INTERVAL: Duration = Duration::from_secs(5); /// Parse the configured `poll_interval` humantime string into a `Duration`, -/// falling back to [`DEFAULT_POLL_INTERVAL`] when unset, empty, or unparseable. -/// `validate_config` separately rejects a set-but-unparseable value so a typo +/// falling back to [`DEFAULT_POLL_INTERVAL`] when unset, empty, or unparsable. +/// `validate_config` separately rejects a set-but-unparsable value so a typo /// surfaces at `open()` rather than silently defaulting. fn parse_poll_interval(poll_interval: Option<&str>) -> Duration { poll_interval @@ -1315,7 +1315,7 @@ impl JdbcSource { ))); } - // A set poll_interval must be a valid humantime string; an unparseable + // A set poll_interval must be a valid humantime string; an unparsable // value would otherwise silently fall back to the default. if let Some(value) = self.config.poll_interval.as_deref() && !value.trim().is_empty() @@ -1956,7 +1956,7 @@ mod tests { fn test_parse_poll_interval() { assert_eq!(parse_poll_interval(Some("30s")), Duration::from_secs(30)); assert_eq!(parse_poll_interval(Some("5m")), Duration::from_secs(300)); - // Unset, empty, and unparseable all fall back to the default. + // Unset, empty, and unparsable all fall back to the default. assert_eq!(parse_poll_interval(None), DEFAULT_POLL_INTERVAL); assert_eq!(parse_poll_interval(Some(" ")), DEFAULT_POLL_INTERVAL); assert_eq!( From 850838fc2732e648b938f09eed816564644c3ab4 Mon Sep 17 00:00:00 2001 From: shbhmrzd Date: Wed, 5 Aug 2026 17:44:45 +0530 Subject: [PATCH 10/11] fix(integration): integrity-check the JDBC driver download The JDBC Postgres tests downloaded the driver JAR at runtime and cached it without validating it. When the download returned a truncated body or an error page (seen in CI), the file was still cached and put on the JVM classpath; the JVM booted fine but Class.forName("org.postgresql.Driver") then failed, so the connector never initialized and every test expecting messages saw zero, while the bad jar was reused by every later test. Download from Maven Central, verify the bytes are a real JAR (ZIP magic plus a minimum size) before persisting, and write via a temp file and atomic rename so a partial or corrupt download can never be cached. A previously cached invalid jar now self-heals, and a genuinely bad download fails setup with a clear message instead of a later opaque ClassNotFoundException. --- .../connectors/jdbc/test_with_postgres.rs | 65 ++++++++++++++----- 1 file changed, 48 insertions(+), 17 deletions(-) diff --git a/core/integration/tests/connectors/jdbc/test_with_postgres.rs b/core/integration/tests/connectors/jdbc/test_with_postgres.rs index 2e4c226f39..fcf8cd20a0 100644 --- a/core/integration/tests/connectors/jdbc/test_with_postgres.rs +++ b/core/integration/tests/connectors/jdbc/test_with_postgres.rs @@ -52,38 +52,69 @@ async fn setup_postgres_container() Ok((postgres, jdbc_url, postgres_jar)) } -/// Get PostgreSQL JDBC driver, downloading if necessary +/// A file that starts with the ZIP local-file-header magic (`PK\x03\x04`) and is +/// at least this large is treated as a real driver JAR. A truncated download or +/// an HTML error page fails this check, so it is never cached or handed to the +/// JVM (where it would only surface later as a `ClassNotFoundException` on +/// `Class.forName`, with the bad file silently reused by every later test). +const MIN_DRIVER_JAR_BYTES: usize = 500_000; + +fn looks_like_jar(bytes: &[u8]) -> bool { + bytes.len() >= MIN_DRIVER_JAR_BYTES && bytes.starts_with(b"PK\x03\x04") +} + +/// Get the PostgreSQL JDBC driver, downloading and integrity-checking it if a +/// valid copy is not already cached. Downloads from Maven Central, verifies the +/// bytes are a real JAR before persisting, and writes via a temp file + atomic +/// rename so a partial or corrupt download can never be cached and reused. async fn get_postgres_driver_jar() -> Result> { let target_dir = std::env::var("CARGO_TARGET_DIR").unwrap_or_else(|_| "target".to_string()); - let jdbc_test_dir = format!("{}/test-jdbc-drivers", target_dir); - let jar_path = format!("{}/postgresql-42.7.1.jar", jdbc_test_dir); + let jdbc_test_dir = format!("{target_dir}/test-jdbc-drivers"); + let jar_path = format!("{jdbc_test_dir}/postgresql-42.7.1.jar"); std::fs::create_dir_all(&jdbc_test_dir)?; + // Reuse the cached jar only if it is actually a valid JAR; a previously + // cached bad download must self-heal rather than fail every run. if std::path::Path::new(&jar_path).exists() { - info!("PostgreSQL JDBC driver found at {}", jar_path); - let absolute_path = std::fs::canonicalize(&jar_path)? - .to_string_lossy() - .to_string(); - return Ok(absolute_path); + match std::fs::read(&jar_path) { + Ok(bytes) if looks_like_jar(&bytes) => { + info!("PostgreSQL JDBC driver found at {jar_path}"); + return Ok(std::fs::canonicalize(&jar_path)?.to_string_lossy().to_string()); + } + _ => { + info!("Cached JDBC driver at {jar_path} is invalid; re-downloading"); + let _ = std::fs::remove_file(&jar_path); + } + } } info!("Downloading PostgreSQL JDBC driver..."); - let download_url = "https://jdbc.postgresql.org/download/postgresql-42.7.1.jar"; - + let download_url = + "https://repo1.maven.org/maven2/org/postgresql/postgresql/42.7.1/postgresql-42.7.1.jar"; let response = reqwest::get(download_url).await?; if !response.status().is_success() { return Err(format!("Failed to download driver: HTTP {}", response.status()).into()); } - let bytes = response.bytes().await?; - std::fs::write(&jar_path, bytes)?; + if !looks_like_jar(&bytes) { + return Err(format!( + "Downloaded JDBC driver is not a valid JAR ({} bytes, magic {:02x?}); \ + the download endpoint may have returned an error page", + bytes.len(), + bytes.get(..4).unwrap_or(&[]) + ) + .into()); + } + + // Write to a unique temp file, then atomically rename, so a crash or a + // concurrent test never observes a half-written jar at `jar_path`. + let tmp_path = format!("{jar_path}.{}.tmp", std::process::id()); + std::fs::write(&tmp_path, &bytes)?; + std::fs::rename(&tmp_path, &jar_path)?; - info!("PostgreSQL JDBC driver downloaded to {}", jar_path); - let absolute_path = std::fs::canonicalize(&jar_path)? - .to_string_lossy() - .to_string(); - Ok(absolute_path) + info!("PostgreSQL JDBC driver downloaded to {jar_path}"); + Ok(std::fs::canonicalize(&jar_path)?.to_string_lossy().to_string()) } /// Build the environment variables for a JDBC Postgres source connector. From 487024c2beb28f8b1a5826487750cbf28105df33 Mon Sep 17 00:00:00 2001 From: shbhmrzd Date: Wed, 5 Aug 2026 20:15:04 +0530 Subject: [PATCH 11/11] style(integration): rustfmt the JDBC driver-download helper --- .../tests/connectors/jdbc/test_with_postgres.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/core/integration/tests/connectors/jdbc/test_with_postgres.rs b/core/integration/tests/connectors/jdbc/test_with_postgres.rs index fcf8cd20a0..ffb3701db2 100644 --- a/core/integration/tests/connectors/jdbc/test_with_postgres.rs +++ b/core/integration/tests/connectors/jdbc/test_with_postgres.rs @@ -80,7 +80,9 @@ async fn get_postgres_driver_jar() -> Result { info!("PostgreSQL JDBC driver found at {jar_path}"); - return Ok(std::fs::canonicalize(&jar_path)?.to_string_lossy().to_string()); + return Ok(std::fs::canonicalize(&jar_path)? + .to_string_lossy() + .to_string()); } _ => { info!("Cached JDBC driver at {jar_path} is invalid; re-downloading"); @@ -114,7 +116,9 @@ async fn get_postgres_driver_jar() -> Result