spuru9 commented on code in PR #231: URL: https://github.com/apache/flink-connector-jdbc/pull/231#discussion_r4524864172
########## AGENTS.md: ########## @@ -0,0 +1,248 @@ +<!-- +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. +--> + +# Flink JDBC Connector AI Agent Instructions + +This file provides guidance for AI coding agents working with the Apache Flink JDBC connector codebase. + +## Prerequisites + +- Java 11, 17, or 21. The build compiles with `maven.compiler.source` 1.8 (nothing overrides `target.java.version` from the parent POM), so write Java 8-compatible syntax. PR CI runs on JDK 17 only (`.github/workflows/push_pr.yml`); the weekly build covers JDK 11, 17 and 21 on the `main` row and JDK 8 and 11 on the `v3.4` row (`.github/workflows/weekly.yml`). +- Maven 3.8.6 (Maven wrapper `./mvnw` included; prefer it) +- Git +- Docker (database-module `*ITCase`s start their database through Testcontainers; core ITCases run on Derby without Docker) +- Unix-like environment (Linux, macOS, WSL) +- The connector builds against `flink.version` in the root `pom.xml`. A change must also work against every Flink version in `.github/workflows/push_pr.yml` and the `main` row of `.github/workflows/weekly.yml`. + +## Commands + +### Build + +- Build without tests: `./mvnw clean package -DskipTests` +- Full build with tests: `./mvnw clean verify` +- Build against another Flink version (what CI does): `./mvnw clean install -DskipTests -Dflink.version=<version>` +- Single module: `./mvnw clean install -DskipTests -pl flink-connector-jdbc-core` +- `-Dfast` skips RAT, checkstyle, spotless, enforcer, javadoc and japicmp. +- A database module depends on `flink-connector-jdbc-core`; build it with its dependency using `-pl flink-connector-jdbc-mysql -am`. +- Dependency convergence (CI runs this on PRs): `./mvnw clean install -DskipTests -Pcheck-convergence -Dflink.convergence.phase=install` + +### Testing + +- `*Test` classes are unit tests; `*ITCase` classes are integration tests that start a real database via Testcontainers (Docker required). `flink-connector-jdbc-core` also runs against in-memory Derby and H2, so its unit tests need no Docker. +- Single unit test class: `./mvnw test -pl flink-connector-jdbc-core -Dtest=JdbcOutputFormatTest` +- Single test method: `./mvnw test -pl flink-connector-jdbc-core -Dtest=JdbcOutputFormatTest#testX` +- Single ITCase in a database module: `./mvnw test -pl flink-connector-jdbc-postgres -am -Dtest=PostgresDynamicTableSourceITCase` +- ArchUnit rules live in `flink-connector-jdbc-architecture`; violation stores are frozen (see Testing Standards). +- The standalone `flink-connector-jdbc-backward-compatibility` module is not part of the root reactor; CI builds it separately (`.github/workflows/backwards_compatibility.yml`) to check that current code reads savepoints written by older connector versions. +- CI for PRs and the weekly build is defined by `apache/flink-connector-shared-utils` (`.github/workflows/ci.yml@ci_utils`); its Maven command line, including the license check, is the reference when a local run differs from CI. + +### Code Quality + +- Format code: `./mvnw spotless:apply` (the `java21` profile sets `spotless.skip=true`, so run it on JDK 11 or 17; google-java-format does not run on JDK 21) +- Check formatting: `./mvnw spotless:check` +- Checkstyle: `./mvnw checkstyle:check` (config in `tools/maven/checkstyle.xml`, suppressions in `tools/maven/suppressions.xml`) +- License headers: CI gates on `exec:java@check-license`, not `apache-rat:check` (which reports pre-existing unapproved files on a clean `main`) +- japicmp compares against `japicmp.referenceVersion` from the root `pom.xml` and checks `@Public` API only; `@PublicEvolving`, `@Experimental` and `@Internal` are excluded. + +### Documentation + +- There is no docs build in this repository. The Flink docs build in `apache/flink` clones the release branch of this repository and renders `docs/content` and `docs/content.zh`. +- Documentation exists in English (`docs/content`) and Chinese (`docs/content.zh`); the two carry the same file set. + +## Repository Structure + +### Modules + +The root `pom.xml` reactor contains: + +- `flink-connector-jdbc-architecture` — ArchUnit tests and their frozen violation stores (`archunit-violations/`). +- `flink-connector-jdbc-core` — The connector: the FLIP-27 `JdbcSource`, the Sink V2 `JdbcSink`, the Table/SQL factory (identifier `jdbc`), the dialect SPI, the XA/exactly-once machinery, and the Derby + H2 in-memory test support. Everything database-agnostic lives here. +- `flink-connector-jdbc-cratedb`, `-db2`, `-mysql`, `-oceanbase`, `-oracle`, `-postgres`, `-sqlserver`, `-trino` — one module per database. Each contributes a `JdbcDialect`, a `JdbcFactory` (dialect SPI), usually a `JdbcCatalog` + type mapper, and a lineage location extractor, discovered via `META-INF/services`. + +Not in the reactor: + +- `flink-connector-jdbc-backward-compatibility` — standalone module, built by its own CI workflow, that verifies savepoint/state compatibility with older connector versions. + +### Supporting directories + +- `docs/content/docs/connectors/` and `docs/content.zh/docs/connectors/` — DataStream and Table docs, English and Chinese, same file set. +- `tools/maven/` — checkstyle config and suppressions. +- `tools/releasing/shared` — git submodule (`apache/flink-connector-shared-utils`); empty unless initialized with `git submodule update --init`. +- `tools/ci/` — the log4j config CI passes to Maven. +- `.github/workflows/` — `push_pr.yml` (PR CI), `weekly.yml` (release branches and Flink snapshots), `backwards_compatibility.yml` (the standalone module). + +### Key packages in `flink-connector-jdbc-core/src/main/java` + +- `org.apache.flink.connector.jdbc` — top-level user-facing option/builder types: `JdbcConnectionOptions`, `JdbcExecutionOptions`, `JdbcExactlyOnceOptions`, `JdbcStatementBuilder`. +- `org.apache.flink.connector.jdbc.core.datastream.source` — FLIP-27 source. Mostly `@PublicEvolving`: `JdbcSource`, `JdbcSourceBuilder`, `config/ContinuousUnBoundingSettings`, and in `enumerator/` (+ `splitter/`) `JdbcSqlSplitEnumeratorBase`, `JdbcSourceEnumeratorState`, `SplitterEnumerator` and its implementations, `reader/extractor/ResultExtractor`, `split/JdbcSourceSplit` and `split/CheckpointedOffset`. `@Internal`: the enumerator, reader and split serializers/state, `JdbcSourceEnumerator`, `JdbcSourceReader`, `JdbcSourceSplitReader`. `JdbcSourceOptions` has no annotation. Check the annotation on the class before changing it. Review Comment: Thanks, but that is the actual class name: `ContinuousUnBoundingSettings` in `core/datastream/source/config/`. The doc matches the code, so I left it. Renaming it would be an API change, which is out of scope for this PR. ########## AGENTS.md: ########## @@ -0,0 +1,248 @@ +<!-- +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. +--> + +# Flink JDBC Connector AI Agent Instructions + +This file provides guidance for AI coding agents working with the Apache Flink JDBC connector codebase. + +## Prerequisites + +- Java 11, 17, or 21. The build compiles with `maven.compiler.source` 1.8 (nothing overrides `target.java.version` from the parent POM), so write Java 8-compatible syntax. PR CI runs on JDK 17 only (`.github/workflows/push_pr.yml`); the weekly build covers JDK 11, 17 and 21 on the `main` row and JDK 8 and 11 on the `v3.4` row (`.github/workflows/weekly.yml`). +- Maven 3.8.6 (Maven wrapper `./mvnw` included; prefer it) +- Git +- Docker (database-module `*ITCase`s start their database through Testcontainers; core ITCases run on Derby without Docker) +- Unix-like environment (Linux, macOS, WSL) +- The connector builds against `flink.version` in the root `pom.xml`. A change must also work against every Flink version in `.github/workflows/push_pr.yml` and the `main` row of `.github/workflows/weekly.yml`. + +## Commands + +### Build + +- Build without tests: `./mvnw clean package -DskipTests` +- Full build with tests: `./mvnw clean verify` +- Build against another Flink version (what CI does): `./mvnw clean install -DskipTests -Dflink.version=<version>` +- Single module: `./mvnw clean install -DskipTests -pl flink-connector-jdbc-core` +- `-Dfast` skips RAT, checkstyle, spotless, enforcer, javadoc and japicmp. +- A database module depends on `flink-connector-jdbc-core`; build it with its dependency using `-pl flink-connector-jdbc-mysql -am`. +- Dependency convergence (CI runs this on PRs): `./mvnw clean install -DskipTests -Pcheck-convergence -Dflink.convergence.phase=install` + +### Testing + +- `*Test` classes are unit tests; `*ITCase` classes are integration tests that start a real database via Testcontainers (Docker required). `flink-connector-jdbc-core` also runs against in-memory Derby and H2, so its unit tests need no Docker. +- Single unit test class: `./mvnw test -pl flink-connector-jdbc-core -Dtest=JdbcOutputFormatTest` +- Single test method: `./mvnw test -pl flink-connector-jdbc-core -Dtest=JdbcOutputFormatTest#testX` +- Single ITCase in a database module: `./mvnw test -pl flink-connector-jdbc-postgres -am -Dtest=PostgresDynamicTableSourceITCase` +- ArchUnit rules live in `flink-connector-jdbc-architecture`; violation stores are frozen (see Testing Standards). +- The standalone `flink-connector-jdbc-backward-compatibility` module is not part of the root reactor; CI builds it separately (`.github/workflows/backwards_compatibility.yml`) to check that current code reads savepoints written by older connector versions. +- CI for PRs and the weekly build is defined by `apache/flink-connector-shared-utils` (`.github/workflows/ci.yml@ci_utils`); its Maven command line, including the license check, is the reference when a local run differs from CI. + +### Code Quality + +- Format code: `./mvnw spotless:apply` (the `java21` profile sets `spotless.skip=true`, so run it on JDK 11 or 17; google-java-format does not run on JDK 21) +- Check formatting: `./mvnw spotless:check` +- Checkstyle: `./mvnw checkstyle:check` (config in `tools/maven/checkstyle.xml`, suppressions in `tools/maven/suppressions.xml`) +- License headers: CI gates on `exec:java@check-license`, not `apache-rat:check` (which reports pre-existing unapproved files on a clean `main`) +- japicmp compares against `japicmp.referenceVersion` from the root `pom.xml` and checks `@Public` API only; `@PublicEvolving`, `@Experimental` and `@Internal` are excluded. + +### Documentation + +- There is no docs build in this repository. The Flink docs build in `apache/flink` clones the release branch of this repository and renders `docs/content` and `docs/content.zh`. +- Documentation exists in English (`docs/content`) and Chinese (`docs/content.zh`); the two carry the same file set. + +## Repository Structure + +### Modules + +The root `pom.xml` reactor contains: + +- `flink-connector-jdbc-architecture` — ArchUnit tests and their frozen violation stores (`archunit-violations/`). +- `flink-connector-jdbc-core` — The connector: the FLIP-27 `JdbcSource`, the Sink V2 `JdbcSink`, the Table/SQL factory (identifier `jdbc`), the dialect SPI, the XA/exactly-once machinery, and the Derby + H2 in-memory test support. Everything database-agnostic lives here. +- `flink-connector-jdbc-cratedb`, `-db2`, `-mysql`, `-oceanbase`, `-oracle`, `-postgres`, `-sqlserver`, `-trino` — one module per database. Each contributes a `JdbcDialect`, a `JdbcFactory` (dialect SPI), usually a `JdbcCatalog` + type mapper, and a lineage location extractor, discovered via `META-INF/services`. + +Not in the reactor: + +- `flink-connector-jdbc-backward-compatibility` — standalone module, built by its own CI workflow, that verifies savepoint/state compatibility with older connector versions. + +### Supporting directories + +- `docs/content/docs/connectors/` and `docs/content.zh/docs/connectors/` — DataStream and Table docs, English and Chinese, same file set. +- `tools/maven/` — checkstyle config and suppressions. +- `tools/releasing/shared` — git submodule (`apache/flink-connector-shared-utils`); empty unless initialized with `git submodule update --init`. +- `tools/ci/` — the log4j config CI passes to Maven. +- `.github/workflows/` — `push_pr.yml` (PR CI), `weekly.yml` (release branches and Flink snapshots), `backwards_compatibility.yml` (the standalone module). + +### Key packages in `flink-connector-jdbc-core/src/main/java` + +- `org.apache.flink.connector.jdbc` — top-level user-facing option/builder types: `JdbcConnectionOptions`, `JdbcExecutionOptions`, `JdbcExactlyOnceOptions`, `JdbcStatementBuilder`. +- `org.apache.flink.connector.jdbc.core.datastream.source` — FLIP-27 source. Mostly `@PublicEvolving`: `JdbcSource`, `JdbcSourceBuilder`, `config/ContinuousUnBoundingSettings`, and in `enumerator/` (+ `splitter/`) `JdbcSqlSplitEnumeratorBase`, `JdbcSourceEnumeratorState`, `SplitterEnumerator` and its implementations, `reader/extractor/ResultExtractor`, `split/JdbcSourceSplit` and `split/CheckpointedOffset`. `@Internal`: the enumerator, reader and split serializers/state, `JdbcSourceEnumerator`, `JdbcSourceReader`, `JdbcSourceSplitReader`. `JdbcSourceOptions` has no annotation. Check the annotation on the class before changing it. +- `org.apache.flink.connector.jdbc.core.datastream.sink` — Sink V2 sink. `@PublicEvolving`: `JdbcSink`, `JdbcSinkBuilder`. `writer/` and `committer/` are `@Internal`. +- `org.apache.flink.connector.jdbc.core.table` — Table/SQL layer. `JdbcDynamicTableFactory` (identifier `jdbc`, registered in `META-INF/services/org.apache.flink.table.factories.Factory`), `JdbcConnectorOptions` (`@PublicEvolving`), and the `@Internal` `JdbcDynamicTableSource` / `JdbcDynamicTableSink`. +- `org.apache.flink.connector.jdbc.core.database` — the pluggable-database SPI: `JdbcFactory` (`@PublicEvolving`), `dialect/` (`JdbcDialect`, `@PublicEvolving`; statement building, converters), and `catalog/` (`JdbcCatalog`, the catalog factory). +- `org.apache.flink.connector.jdbc.datasource` — connection and transaction management. `connections/` holds `JdbcConnectionProvider` / `SimpleJdbcConnectionProvider`; `connections/xa/` and `transactions/xa/` hold the XA two-phase-commit path used for exactly-once. +- `org.apache.flink.connector.jdbc.internal` — `JdbcOutputFormat`, the DML/insert/read option holders (`internal/options/`), and the connection/statement execution glue. `@Internal`. +- `org.apache.flink.connector.jdbc.lineage` — OpenLineage facet extraction and the `JdbcLocationExtractorFactory` SPI. +- `org.apache.flink.connector.jdbc.split` — `JdbcParameterValuesProvider` implementations, used by the deprecated `setSql` path of `JdbcSourceBuilder` (superseded by `SplitterEnumerator`). + +## Architecture Boundaries + +1. **Source (FLIP-27).** `JdbcSourceEnumerator` runs on the coordinator thread and hands `JdbcSourceSplit`s to `JdbcSourceReader`; `JdbcSourceSplitReader` runs on the fetcher thread and owns the JDBC `Connection`/`ResultSet`. A JDBC connection is not thread-safe — access it only from the reader thread. +2. **Sink (Sink V2).** `JdbcSink` builds on `JdbcOutputFormat`, which batches rows and flushes on a size/time trigger and on checkpoint. The at-least-once path uses a plain `JdbcConnectionProvider`; per-record failures surface on the task thread at flush time. +3. **Exactly-once (XA).** The exactly-once sink uses distributed (XA) transactions via `connections/xa` (`XaConnectionProvider`, `PoolingXaConnectionProvider`) and `transactions/xa` (Xid generation, per-checkpoint transaction lifecycle). XA driver support varies by database; this is the most driver-sensitive path — re-verify it on any driver bump. Not every dialect supports it. +4. **Dialect SPI.** A database plugs in through `JdbcFactory` (`acceptsURL` decides which JDBC URLs it claims; `createDialect` / `createDialect(compatibleMode)` builds the `JdbcDialect`), discovered via `META-INF/services/org.apache.flink.connector.jdbc.core.database.JdbcFactory`. The `JdbcDialect` owns quoting, LIMIT, upsert/UPSERT-or-MERGE statement construction, and the `JdbcDialectConverter` type mapping. Keep database-specific behaviour inside the dialect, not in core. +5. **Table layer.** `JdbcDynamicTableSource` / `JdbcDynamicTableSink` wrap the DataStream connectors; all Table options live in `JdbcConnectorOptions`. +6. **Connector vs Flink.** Production code may depend only on stable (`@Public` / `@PublicEvolving`) Flink API outside connector and util packages (enforced by ArchUnit). Every Flink API used must exist with the same annotation in `flink.version`, because the connector is released for several Flink minor versions. + +## Common Change Patterns + +### Adding a new database + +1. Create a `flink-connector-jdbc-<db>` module mirroring an existing one (e.g. `flink-connector-jdbc-postgres`), with the DB module as a sibling of `flink-connector-jdbc-core` and added to the root `pom.xml` `<modules>`. +2. Implement `JdbcDialect` + `JdbcDialectConverter`, and a `JdbcFactory` whose `acceptsURL` matches the driver's JDBC sub-protocol; register the factory in `META-INF/services/org.apache.flink.connector.jdbc.core.database.JdbcFactory`. +3. If the catalog is supported, add a `JdbcCatalog` + type mapper; add a `JdbcLocationExtractorFactory` for lineage. +4. Pin the JDBC driver version as a `<db>.version` property in the module `pom.xml` and add the driver dependency (test or provided scope, as the sibling modules do). +5. Add a Testcontainers `*Database` helper under `testutils/`, a `*TestBase`, and dialect/catalog `*ITCase`s. +6. Document the dialect in `docs/content` and `docs/content.zh`. + +### Adding a Table/SQL option + +1. Define the `ConfigOption<T>` in `JdbcConnectorOptions` +2. Register and validate it in `JdbcDynamicTableFactory` (`optionalOptions()` / validation) +3. Add a factory test and, when behaviour changes, an `ITCase` +4. Add the option row to `docs/content/docs/connectors/table/*.md` and the same file under `docs/content.zh/` +5. Fill in the Release Notes field on the JIRA ticket + +### Adding a DataStream builder option + +1. Add it to `JdbcSourceBuilder` or `JdbcSinkBuilder` with validation in `build()` +2. Add a builder unit test and, when behaviour changes, an `ITCase` +3. Document it under `docs/content/docs/connectors/datastream/` and the `.zh` copy + +### Changing checkpointed state + +State is written by `SimpleVersionedSerializer` implementations. Current versions: `JdbcSourceSplitSerializer` 0, `JdbcSourceEnumStateSerializer` 0, `JdbcWriterStateSerializer` 2, `JdbcCommitableSerializer` 1. Review Comment: Same here, the typo is in the real class name (`JdbcCommitableSerializer` under `sink/committer/`). I kept the doc matching the source so grep finds it. Happy to do a rename in a separate ticket if we want that. ########## AGENTS.md: ########## @@ -0,0 +1,248 @@ +<!-- +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. +--> + +# Flink JDBC Connector AI Agent Instructions + +This file provides guidance for AI coding agents working with the Apache Flink JDBC connector codebase. + +## Prerequisites + +- Java 11, 17, or 21. The build compiles with `maven.compiler.source` 1.8 (nothing overrides `target.java.version` from the parent POM), so write Java 8-compatible syntax. PR CI runs on JDK 17 only (`.github/workflows/push_pr.yml`); the weekly build covers JDK 11, 17 and 21 on the `main` row and JDK 8 and 11 on the `v3.4` row (`.github/workflows/weekly.yml`). +- Maven 3.8.6 (Maven wrapper `./mvnw` included; prefer it) +- Git +- Docker (database-module `*ITCase`s start their database through Testcontainers; core ITCases run on Derby without Docker) +- Unix-like environment (Linux, macOS, WSL) +- The connector builds against `flink.version` in the root `pom.xml`. A change must also work against every Flink version in `.github/workflows/push_pr.yml` and the `main` row of `.github/workflows/weekly.yml`. + +## Commands + +### Build + +- Build without tests: `./mvnw clean package -DskipTests` +- Full build with tests: `./mvnw clean verify` +- Build against another Flink version (what CI does): `./mvnw clean install -DskipTests -Dflink.version=<version>` +- Single module: `./mvnw clean install -DskipTests -pl flink-connector-jdbc-core` +- `-Dfast` skips RAT, checkstyle, spotless, enforcer, javadoc and japicmp. +- A database module depends on `flink-connector-jdbc-core`; build it with its dependency using `-pl flink-connector-jdbc-mysql -am`. +- Dependency convergence (CI runs this on PRs): `./mvnw clean install -DskipTests -Pcheck-convergence -Dflink.convergence.phase=install` + +### Testing + +- `*Test` classes are unit tests; `*ITCase` classes are integration tests that start a real database via Testcontainers (Docker required). `flink-connector-jdbc-core` also runs against in-memory Derby and H2, so its unit tests need no Docker. +- Single unit test class: `./mvnw test -pl flink-connector-jdbc-core -Dtest=JdbcOutputFormatTest` +- Single test method: `./mvnw test -pl flink-connector-jdbc-core -Dtest=JdbcOutputFormatTest#testX` +- Single ITCase in a database module: `./mvnw test -pl flink-connector-jdbc-postgres -am -Dtest=PostgresDynamicTableSourceITCase` +- ArchUnit rules live in `flink-connector-jdbc-architecture`; violation stores are frozen (see Testing Standards). Review Comment: `flink-connector-jdbc-architecture` does exist on `main`. It is a top-level module holding the ArchUnit tests and the `archunit-violations/` stores, so I left those references as they are. The doc already mentions the per-module `archunit-violations/` dirs as well. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
