Skip to content

SelectBuilder makes assumptions about join conditions being "trivial" #995

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

Closed
nt-gt opened this issue Jun 30, 2021 · 3 comments
Closed

SelectBuilder makes assumptions about join conditions being "trivial" #995

nt-gt opened this issue Jun 30, 2021 · 3 comments
Assignees
Labels
in: selectbuilder SelectBuilder stands for the complete API for creating SQL statements programmatically. type: enhancement A general enhancement

Comments

@nt-gt
Copy link

nt-gt commented Jun 30, 2021

Hi,

Like with #968, I have a case where I want to delegate the SELECT query generation to Spring to leverage Spring's support for different dialects (giving me support for different databases "out of the box"). Unfortunately, I am faced with a non-trivial join where I need to go beyond Spring's current assumptions for how JOIN works.

My two key issues:

  1. When initiating a join (selectBuilder.join(...)), the parameter is assumed to be a Table (possible via String which is immediately passed to Table.create). However, SQL supports joining on a "subselect" (LEFT OUTER JOIN (SELECT ... ) AS alias ON alias.X = other_table.Y). This is now tracked in Add support for join with subselect #1003
  2. The condition part on a join assumes x.A = y.B [AND z.C = Y.D ...] formatted conditions. However, other formats can be used - such as OR'ing conditions. This is tracked as this issue.

The concrete example

The example I am working on is a bit complex, but I have attempted to reduce it to these model classes:

@Data
public class TransportLocation {

    @Id
    private UUID id;

}

@Data
public class Transport {

    @Id
    private UUID id;

    private UUID pickupLocationId;

    private UUID deliveryLocationId;

    private UUID vehicleId;
}

@Data
public class TransportLocationTO extends TransportLocation {
    private Vehicle vehicle;
}

@Data
public class Vehicle {
   @Id
    private UUID id;
}

What I want to do is to generate an SQL query that maps up a TransportLocationTO from a single SELECT statement. The Vehicle entity comes via the Transport entity on either pickupLocationId or deliveryLocationId. Some additional context:

  • The TransportLocation can be on "either side" of the Transport. Often it will appear in the pickupLocationId of one Transport and deliveryLocationId of another. This happens if the TransportLocation is a "middle stops" in the route.

    • The id of the TransportLocation depends implicit on the Vehicle due to business rules (that are not reflected in this example) - but SQL-wise the link between the two goes via Transport. This is why it is "fine" in this case for the JOIN to use an OR (with a DISTINCT). But on the flip side, if I only use one of them then the join will fail for either very first or the very last TransportLocation.
  • The API has TransportLocationTO has a "primary entity" - as in "GET /transport-locations" + "GET /transport-locations/{ID}" are defined endpoints and they want to include the Vehicle entity.

  • I will be passing the generated SQL to Spring R2DBC's DatabaseClient in case that bit will matter (I do not think it will, but mentioning it for completeness).

Version used

In case it matters, I am currently using:

		<groupId>org.springframework.boot</groupId>
		<artifactId>spring-boot-starter-parent</artifactId>
		<version>2.4.2</version>

I am not entirely sure which version of spring-data-jdbc + spring-r2dbc that implies.

Solutions / work arounds

As far as I have gathered, the "optimal" solution is to use JOIN + sub-select as this avoids the need for DISTINCT, but this is not possible at all.

An alternative in my case is to use the JOIN ... ON A=B OR A=C + DISTINCT. I will test whether the code accepts the variant of JOIN ... ON (A=B OR A=C) = TRUE, which is at best a rewrite of questionable readability and at worst might neuter some database/query planner optimizations.

Sadly changing the datamodel is not an option for me (political decision that I cannot change).

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 30, 2021
@schauder
Copy link
Contributor

schauder commented Jun 30, 2021

I'd vote to make two issues out of this: one to support subqueries in FROM and JOIN clauses, and one to support non trivial join conditions.

@nt-gt
Copy link
Author

nt-gt commented Jun 30, 2021

I am fine with creating a second issue if the consensus is to split this into two. :)

@nt-gt nt-gt changed the title SelectBuilder makes assumptions about join expressions being "trivial" SelectBuilder makes assumptions about join conditions being "trivial" Jul 7, 2021
@nt-gt
Copy link
Author

nt-gt commented Jul 7, 2021

I have now split it. This issue being the condition and the missing subselect is #1003.

@schauder schauder removed the status: waiting-for-triage An issue we've not yet triaged label Jul 19, 2021
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 19, 2021
@schauder schauder added in: selectbuilder SelectBuilder stands for the complete API for creating SQL statements programmatically. type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 21, 2021
mp911de added a commit that referenced this issue Sep 28, 2021
Reformat code. Add since tag.

See #995
Original pull request: #1014.
@mp911de mp911de added this to the 2.3 RC1 (2021.1.0) milestone Sep 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: selectbuilder SelectBuilder stands for the complete API for creating SQL statements programmatically. type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants