ebean icon indicating copy to clipboard operation
ebean copied to clipboard

DbForeignkey with noConstraint and NotNull produces incorrect join type - Add forceLeftJoin option

Open AlexWagner opened this issue 2 years ago • 5 comments

The Combination of @NotNull and @DbForeignKey(noConstraint = true) produces the wrong join type in queries.

use case: We import relations between datasets and we only get the primary key of those in this relation. It is not sure if we get those other datasets per imports, but we want to know if there is a relation.

Fix is suggested in PR

AlexWagner avatar Jul 25 '22 08:07 AlexWagner

produces the wrong join type in queries

It isn't clear to me why you expect this to be a LEFT JOIN.

That is, my expectation is for @DbForeignKey(noConstraint = true) isn't that it expects invalid foreign key values, but that the we don't wish to incur the cost of enforcement.

It seems that there is some expectation that it should be a LEFT JOIN to allow for invalid values?

rbygrave avatar Jul 29 '22 10:07 rbygrave

Hello Rob, I want to explain you our use case:

We want to import "stock orders" - All orders have an ISIN - stock value and amount and some other mandatory informations.

So we have a "orders" and an "isin" object

class Orders {
   @Id
   private UUID id;

   @ManyToOne(optional = false)
   @DbForeignKey(noConstraint = true)
   private Isin isin;
   ...
}
// contains detail informations like company name, country and so on
class Isin {
   @Id
   @Size(max=12)
   private String id;
}

Which should produce the followin "order" table:

  • id (primary key)
  • isin_id (varchar(12) not null) - logically references isins table (but no FK present)
  • ...

When we import the "Orders", we only create reference-Isin-beans to get the correct ISIN inserted in the order. We cannot guarantee, that the ISIN would really exist in this step, because ISINs are imported in a later step (delay can be hours or even days or sometimes we event won't get any detail information, because the 'value paper' is so exotic)

On the one side, the ISIN in the "orders" must be "not null". It is not possible, to have an order without an ISIN. On the other side, we cannot guarantee the consistency between "orders" and "isins" table, so we've to add @DbForeignKey(noConstraint = true) If we don't find the related ISIN, we'll show the user only the 12-digit isin and a message, that no detail information is present in the system. Unfortunately the use of join instead of left join will not find the order at all now.

I understand the argument, that @DbForeignKey(noConstraint = true) may only generate DDL generation and should not affect the query generation, but we will need a left join in this particular use case.

So how can we achieve to get a left-join here? Idea 1: @DbForeignKey(noConstraint = true) should only affect DDL @DbForeignKey(noConstraint = true, assumeConsistency = false) should use `left join'

Idea 2: Add an additional @JoinType() annotation to override the used join type. This may be also useful in other situations, where ebean would generate a left join and you want a join.

@rbygrave What do you think?

  1. Should we stick to the definition, that noConstraint only affect DDL generation (for performance reasons - maybe add to javadoc)?
  2. If yes, I see no chance to reuse existing annotations, should we add assumeConsistency = false to DbForeignKey or should we introduce a new @JoinType(INNER/OUTER) annotation (which would be our favorite, if we keep on 1.)

Cheers Alex & Roland

rPraml avatar Aug 01 '22 09:08 rPraml

The default join type ebean will use is based on cardinality + optionality. For the *ToOne case it then just comes down to optionality - which means in this case it's all down to whether the foreign key column is nullable.

Also note that this is the default join type and LEFT JOIN needs to "cascade" to any subsequent "sub path/sub tree". What that means is that INNER JOIN get automatically converted to LEFT JOIN if a higher part of the path is LEFT JOIN.

Using a @JoinType might be more general per say but I think it would be more confusing. As far as I am aware, the only case where we really want to change the "default" join type is this case where we have:

  • a non-null foreign key column
  • it is known to have invalid values / values that will eventually be valid but perhaps not until hours later

So I think we should go with Idea 1.

Idea 1:

@DbForeignKey(noConstraint = true) - Means the constraint is not enforced via the DB. Only affects DDL. @DbForeignKey(noConstraint = true, assumeConsistency = false) - Means use left join regardless of the non-null status of the foreign key column.

I am wondering about the naming. If assumeConsistency=false would be better as forceLeftJoin=true or allowInconsistency=true

rbygrave avatar Aug 03 '22 02:08 rbygrave

+1 for assumeConsistency. I'll create a PR for ebean-annotation

edit: sry, I misunderstand your last sentence. The more I think, the better I like forceLeftJoin as it expresses what it exactly does.

https://www.reddit.com/r/ProgrammerHumor/comments/vs5jb2/naming_things/ ;)

rPraml avatar Aug 03 '22 05:08 rPraml

Ha ha, happy to go with forceLeftJoin ... but the other 2 names seem good too. Naming is hard :)

rbygrave avatar Aug 03 '22 08:08 rbygrave

@AlexWagner we need to review this. I've adjusted/reverted the forceLeftJoin option, as this is the code we have in production since ~1 year

rPraml avatar Aug 09 '23 15:08 rPraml

Closing as resolved by #2759

rbygrave avatar Aug 14 '23 02:08 rbygrave