makssent opened a new pull request, #40237:
URL: https://github.com/apache/shardingsphere/pull/40237
Changes proposed in this pull request:
- Added Firebird to the SQL E2E tests (`test/e2e/sql`). The `db` scenario
runs through ShardingSphere-Proxy (adapter `proxy`, mode `Cluster`) against
Firebird in the `firebirdsql/firebird:5.0.4` container. The `jdbc` adapter is
not configured for Firebird yet. For now Firebird is added only to SQL E2E, and
E2E operation (`test/e2e/operation`) will follow in a separate PR.
- Firebird environment in `test/e2e/env`, built the same way as for
PostgreSQL, openGauss and Hive:
- `FirebirdStorageContainerCreateOption` describes the container (image,
port 3050, default database `firebird`, UTF8 database charset),
`FirebirdStorageContainerConnectOption` describes the connection (Jaybird
driver, URL `jdbc:firebirdsql://<host>:<port>/<database>?encoding=UTF8`), and
`FirebirdStorageContainerOption` combines
`FirebirdStorageContainerCreateOption` and
`FirebirdStorageContainerConnectOption` and is registered in the
`StorageContainerOption` SPI.
- The URL contains only the database name, without the path to the
database file. The Firebird server prepends the directory from the `ISC_PATH`
environment variable to such a name
([Firebird](https://github.com/FirebirdSQL/firebird/blob/v5.0.4/src/common/db_alias.cpp#L459-L484)),
and `FirebirdStorageContainerCreateOption` sets `ISC_PATH` in the container
environment.
- The container init scripts in `container/firebird/init-sql/` create
the users `test_user` and `ready_user`, as for other databases. `test_user` is
granted `CREATE DATABASE`, because in Firebird a regular user cannot create a
database without this privilege.
- The `db` scenario for Firebird:
- the `actual` and `expected` init scripts create the same databases and
tables as for PostgreSQL, owned by `test_user`;
- `proxy/conf/firebird/database-db.yaml` follows the PostgreSQL
configuration. Table names in the rules are in upper case (`T_ORDER`), because
Firebird stores unquoted names in upper case;
- `Firebird` is added to `db-types` of 12 existing DQL, DML and DDL
cases. There are no new cases or datasets, the cases use the shared datasets.
- `FirebirdDialectQueryBehaviorProvider` (`DialectQueryBehaviorProvider`
SPI) for Firebird:
- `getFallbackOrderByWhenNoPrimaryKey` returns `1 ASC`. The DML
assertion (`BaseDMLE2EIT`) reads the rows of each data node with `ORDER BY` the
primary key, but the E2E framework does not detect the primary key of a
Firebird table. Without `ORDER BY` Firebird returns rows in storage order,
while the DML assertion compares rows in order;
- `getDeleteAllRowsSQLTemplate` returns `DELETE FROM %s` (next item).
- Three changes to the shared E2E logic. The behavior for other databases
does not change:
- Table cleanup after DML. Firebird 5 has no `TRUNCATE TABLE` statement.
Added the default method
`DialectQueryBehaviorProvider.getDeleteAllRowsSQLTemplate` (empty by default).
`DataSetEnvironmentManager.getTruncateSQL` takes the cleanup SQL from
`getDeleteAllRowsSQLTemplate` and uses the previous `TRUNCATE TABLE %s` when
there is no template. Only `FirebirdDialectQueryBehaviorProvider` sets a
template, so the cleanup SQL of other databases is unchanged.
`FirebirdDialectQueryBehaviorProvider` has a TODO to restore `TRUNCATE TABLE`
when Firebird 6 is released.
- Hikari pool connection validation. Added the default method
`StorageContainerConnectOption.getConnectionTestQuery` (empty by default).
`StorageContainerUtils.generateDataSource` passes the query from
`getConnectionTestQuery` to Hikari `connectionTestQuery`, and
`FirebirdStorageContainerConnectOption` returns `SELECT 1 FROM RDB$DATABASE`.
- Without `connectionTestQuery` Hikari validates a connection by
calling `Connection.isValid`. Jaybird runs `isValid` as a task in
`ForkJoinPool.commonPool()` and waits for its result
([Jaybird](https://github.com/FirebirdSQL/jaybird/blob/v5.0.13/src/main/org/firebirdsql/jdbc/FBConnection.java#L500-L503)).
JUnit runs tests in parallel in ForkJoinPool threads, and a ForkJoinPool
thread that waits for a task runs other tasks meanwhile, including another
test. The other test enters the `synchronized` data filling block of
`BaseDQLE2EIT.fillDataOnlyOnce` in the same thread (the monitor is reentrant)
and fills the data a second time, and DQL fails with a primary key violation.
- For other databases `getConnectionTestQuery` is empty, and Hikari
still validates connections by calling `Connection.isValid`.
- `DDLE2EIT.assertColumnMetaData` compares column names in upper case,
as `BaseDMLE2EIT` already compares DML column labels. Firebird stores unquoted
names in upper case (`ORDER_ID`), while the shared DDL datasets use lower case
names (`order_id`). Without this change Firebird would need its own DDL
datasets.
- Added Firebird E2E to `nightly-e2e-sql.yml` for CI.
<details>
<summary>Verification</summary>
The `db` scenario was run in Docker through the proxy for Firebird and, to
check the shared logic changes, for PostgreSQL.
* Firebird: `firebirdsql/firebird:5.0.4`
* E2E client and proxy driver: Jaybird 5.0.13
```bash
./mvnw clean install -am -pl test/e2e/sql -Pe2e.env.docker -DskipTests
./mvnw -nsu install -f test/e2e/sql/pom.xml -De2e.run.type=DOCKER
-De2e.artifact.modes=Cluster -De2e.artifact.adapters=proxy \
-De2e.run.additional.cases=false -De2e.scenarios=db
-De2e.artifact.databases=Firebird
```
PostgreSQL was run with the same command and
`-De2e.artifact.databases=PostgreSQL`. Firebird passed three consecutive runs
with the same result. The skipped tests are the additional cases
(`e2e.run.additional.cases=false`) and the DistSQL cases, which do not exist
for Firebird.
| Database | Tests | Errors | Skipped |
|---|---|---|---|
| Firebird | 92 (DQL 24, DML 32, Batch DML 10, DDL 12) | 0 | 14 |
| PostgreSQL | 616 (DQL 380, DML 92, Batch DML 36, DDL 72, RQL 23) | 0 |
13 |
</details>
---
Before submitting this PR, I confirm that:
- [x] My code follows the [code of
conduct](https://shardingsphere.apache.org/community/en/involved/conduct/code/)
of this project.
- [x] I have reviewed my changes.
- [ ] I have (or in comment I request) added corresponding labels for the
pull request.
- [x] I have passed the required local checks: `./mvnw clean install -B -T1C
-Pcheck`.
- [ ] I have updated the relevant documentation, if applicable.
- [ ] I have added or updated unit tests for my changes, if applicable.
- [x] I have updated the Release Notes of the current development version,
if applicable. For more details, see [Update Release
Note](https://shardingsphere.apache.org/community/en/involved/contribute/contributor/).
- [ ] I have disclosed the tool and affected files or scope for any material
AI assistance, as required by the [AI-assisted contribution policy].
[AI-assisted contribution policy]:
https://github.com/apache/shardingsphere/blob/master/AI_POLICY.md
--
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]
