-
Notifications
You must be signed in to change notification settings - Fork 132
Add initial dialect support #24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
edad8ca
to
54baf40
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed you expect a Postgres database running. While feasible for a single vendor, this doesn't scale to multiple vendors, so I vote for using Testcontainers. I wonder if we could/want to create Testcontainers support that provides a ConnectionFactory
directly.
This could be valuable for people experimenting with R2DBC.
package org.springframework.data.r2dbc.repository.config; | ||
package org.springframework.data.r2dbc.config; | ||
|
||
import io.r2dbc.spi.ConnectionFactory; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Someone has the wrong formatter configuration, which is interesting since you are supposed to be the only editor of the file.
src/main/java/org/springframework/data/r2dbc/dialect/Database.java
Outdated
Show resolved
Hide resolved
* @throws IllegalArgumentException if any of the {@literal mappingContext} is {@literal null}. | ||
*/ | ||
@Bean | ||
public ReactiveDataAccessStrategy reactiveDataAccessStrategy(RelationalMappingContext mappingContext) { | ||
|
||
Assert.notNull(mappingContext, "MappingContext must not be null!"); | ||
return new DefaultReactiveDataAccessStrategy(new BasicRelationalConverter(mappingContext)); | ||
return new DefaultReactiveDataAccessStrategy(getDialect(connectionFactory()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return new DefaultReactiveDataAccessStrategy(getDialect(connectionFactory()), | |
return new DefaultReactiveDataAccessStrategy(getDialect(connectionFactory()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We typically want to share a single MappingContext
across the configuration setup. With only a Dialect
, DefaultReactiveDataAccessStrategy
creates a converter and MappingContext
on its own.
src/main/java/org/springframework/data/r2dbc/dialect/Database.java
Outdated
Show resolved
Hide resolved
src/main/java/org/springframework/data/r2dbc/function/DefaultDatabaseClient.java
Outdated
Show resolved
Hide resolved
src/main/java/org/springframework/data/r2dbc/repository/support/BindSpecWrapper.java
Outdated
Show resolved
Hide resolved
src/main/java/org/springframework/data/r2dbc/repository/support/SimpleR2dbcRepository.java
Outdated
Show resolved
Hide resolved
.database("foo").username("bar").password("password").build()); | ||
H2ConnectionFactory h2 = new H2ConnectionFactory(H2ConnectionConfiguration.builder().inMemory("mem").build()); | ||
|
||
assertThat(Database.findDatabase(postgres)).contains(Database.POSTGRES); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For these kind of independent asserts I like to use SoftAssertions
...est/java/org/springframework/data/r2dbc/function/AbstractDatabaseClientIntegrationTests.java
Show resolved
Hide resolved
…QL Server. We now provide dialect support for H2, PostgreSQL, and Microsoft SQL Server databases, configurable through AbstractR2dbcConfiguration. By default, we obtain the Dialect by inspecting ConnectionFactoryMetadata to identify the database and the most likely dialect to use. BindableOperation encapsulates statements/queries that can accept parameters. Use BindableOperation for statements through DatabaseClient. Extract SQL creation. Split integration test into abstract base class that can be implemented with a database-specific test class.
1a2046e
to
1f21d0d
Compare
…QL Server. We now provide dialect support for H2, PostgreSQL, and Microsoft SQL Server databases, configurable through AbstractR2dbcConfiguration. By default, we obtain the Dialect by inspecting ConnectionFactoryMetadata to identify the database and the most likely dialect to use. BindableOperation encapsulates statements/queries that can accept parameters. Use BindableOperation for statements through DatabaseClient. Extract SQL creation. Split integration test into abstract base class that can be implemented with a database-specific test class. Original pull request: #24.
Rename Database.latestDialect() to defaultDialect(). Rename Dialect.returnGeneratedKeys() to Dialect.generatedKeysClause(). Simplify IndexBindMarkers. Create BindableOperation.bind(Statement, SettableValue) to reduce code duplicates. Rename BindSpecWrapper to BindSpecAdapter. Original pull request: #24.
That's merged and polished now. |
@mp911de great idea! Should be super easy to add 👍 |
Do you want me to file a ticket? |
Yes, please 😊 |
We now support dialects to integrate with various database vendors to support a similar feature set when seen from an application perspective.
We ship with dialects for Postgres, Microsoft SQL Server, and H2. We generate SQL statements through
ReactiveDataAccessStrategy
that considers dialects. Mapped entity support is limited to a single primary key, composite primary keys are not yet supported.SQL generation happens on a basic level and is subject to further evolution.
Tests are now reduced to the minimal common guarantees.