Skip to content
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

JdbcAggregateTemplate honors columns specified in query #1967

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

schauder
Copy link
Contributor

@schauder schauder commented Jan 2, 2025

If no columns are given, all columns are selected by default.

If columns are specified, only these are selected.
Joins normally triggered by columns from 1:1 relationships are not implemented, and the corresponding columns don't get loaded and can't be specified in a query.

Limiting columns is not supported for single query loading.

Closes #1803

@schauder schauder requested a review from mp911de January 2, 2025 13:33
If no columns are given, all columns are selected by default.

If columns are specified, only these are selected.
Joins normally triggered by columns from 1:1 relationships are not implemented, and the corresponding columns don't get loaded and can't be specified in a query.

Limiting columns is not supported for single query loading.

Closes #1803
@schauder schauder force-pushed the issue/1803-projection branch from 155004e to cbde1e7 Compare January 15, 2025 13:55
@mp911de mp911de added this to the 3.5 M1 (2025.0.0) milestone Jan 16, 2025
Extract method. Extend tests. Introduce empty Query object to avoid nullability.
Copy link
Member

@mp911de mp911de left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mapping is missing in case a property uses a renamed column, see SqlGeneratorUnitTests.DummyEntity.id and the updated test selectByQueryWithColumnLimit.


if (!CollectionUtils.isEmpty(query.getColumns())) {
for (SqlIdentifier columnName : query.getColumns()) {
columns.add(Column.create(columnName, table));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Column selection is missing the mapping from property name to column name. Columns (property paths specifically) should be mapped. Ideally, we can reuse the loop below regarding its functionality with just the input columns to derive property paths from. Also, we do not assume that all columns are mapped onto our domain entities.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Query columns now get checked against property names.
If such a property name is found, the property is used.
Otherwise the column is considered a literal column and used as is in the SQL generation.

Original pull request #1967
See #1803
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JdbcAggregateOperations does not use columns parameter of Query
2 participants