Add initial dialect support #24
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mp911de
force-pushed
the
issue/gh-20-initial-dialect-support
branch
from
edad8ca to
54baf40
Compare
schauder
approved these changes
Dec 3, 2018
There was a problem hiding this comment.
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.
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
|
|
||
| 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.
Suggested change
| return new DefaultReactiveDataAccessStrategy(getDialect(connectionFactory()), | |
| return new DefaultReactiveDataAccessStrategy(getDialect(connectionFactory()), |
There was a problem hiding this comment.
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.
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.
mp911de
force-pushed
the
issue/gh-20-initial-dialect-support
branch
from
1a2046e to
1f21d0d
Compare
mp911de
added a commit
that referenced
this pull request
Dec 3, 2018
…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.
mp911de
pushed a commit
that referenced
this pull request
Dec 3, 2018
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 😊 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
ReactiveDataAccessStrategythat 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.