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

DDC-551: Consider adding ability to specify additional join conditions on a @JoinTable / @JoinColumn #5059

Closed
doctrinebot opened this issue Apr 28, 2010 · 20 comments
Assignees
Milestone

Comments

@doctrinebot
Copy link

Jira issue originally created by user mjh_ca:

Per discussion with beberlei and romanb in #doctrine-dev yesterday, opening this ticket as a "feature request" to support migrating legacy schemas with a special many-to-many mapping to Doctrine.

Consider the following schema:

CREATE TABLE categories (
    category*id BIGINT UNSIGNED NOT NULL AUTO*INCREMENT,
    content_type ENUM('posts', 'videos'),
    /** ... **/
    PRIMARY KEY (category_id)
) ENGINE=InnoDB;

CREATE TABLE content*category*association (
    content_id BIGINT UNSIGNED NOT NULL,
    category_id BIGINT UNSIGNED NOT NULL,
    content_type ENUM('posts', 'videos'),
    PRIMARY KEY (content*id, category_id, content*type),
    FOREIGN KEY (category*id, content_type) REFERENCES categories(category_id, content*type) ON DELETE CASCADE ON UPDATE CASCADE
) ENGINE=InnoDB;

CREATE TABLE posts (
    post*id BIGINT UNSIGNED NOT NULL AUTO*INCREMENT,
    /** ... **/
    PRIMARY KEY (post_id)
) ENGINE=InnoDB;

CREATE TABLE videos (
    video*id BIGINT UNSIGNED NOT NULL AUTO*INCREMENT,
    /** ... **/
) ENGINE=InnoDB;

There is a Many-To-Many relationship between each of the posts and videos table (via the content_category_association table) to the categories table. The difference from a standard many-to-many relationship is there is an extra column in the association table (content_type) which must be included in the join condition to return correct results. Since both the videos and posts table have their own autonumber primary keys, a join against the association table must include an extra condition (i.e. INNER JOIN ... ON ... AND content_category_association.content_type = 'posts').

Perhaps you could allow passing of additional properties to @jointable / joinColumns to specify the additional join condition .. i.e.:

/*** @Entity **/
class Video
{
  /****
   * @ManyToMany(targetEntity="Category")
   * @JoinTable(name="content*category*association",
   *      joinColumns={@JoinColumn(name="content*id", referencedColumnName="video*id")},
   *      inverseJoinColumns={@JoinColumn(name="category*id", referencedColumnName="video*id")},
   *      extraJoinTerms={@JoinTerm(content_type="video")}
   *      )
   */
  private $categories;

  // ...
}

/*** @Entity **/
class Category
{
    // ...
}

Certainly this schema is not ideal from a pure OO perspective. Class inheritance with a discriminator column may have been a better way to do this, thereby allowing a globally unique "content_id" for all types of content, negating the need for the extra column in the association table. However, it would nonetheless be helpful to have this additional capability within Doctrine to avoid having to re-factor such a legacy schema.

@doctrinebot
Copy link
Author

Comment created by yaroslav:

Would be great to get this functionality

@doctrinebot
Copy link
Author

Comment created by @beberlei:

Assigned to asm89

@doctrinebot
Copy link
Author

Comment created by @beberlei:

Scheduled for 2.2

@doctrinebot
Copy link
Author

Comment created by @asm89:

I've been working on this ticket over here:
https://github.com/asm89/doctrine2/tree/[DDC-551](http://www.doctrine-project.org/jira/browse/DDC-551)

Latest thing I added was the state of the collection of filters, because this is needed for parsing (and sometimes not parsing) the queries to generate SQL. I'd like some feedback about the state keeping. More information at the commit:
asm89@2653d73

At this point the EntityManager keeps track of this state, but maybe it would be nice to have a separate FilterCollection keep track of the state/hashes etc?

@doctrinebot
Copy link
Author

Comment created by @beberlei:

This issue is referenced in Github Pull-Request GH-224
#224

@doctrinebot
Copy link
Author

Comment created by @beberlei:

Related Pull Request was closed: #210

@doctrinebot
Copy link
Author

Comment created by @beberlei:

Related Pull Request was closed: #224

@doctrinebot
Copy link
Author

Comment created by @beberlei:

Implemented

@doctrinebot
Copy link
Author

Issue was closed with resolution "Fixed"

@doctrinebot
Copy link
Author

Comment created by @beberlei:

This issue is referenced in Github Pull-Request GH-225
#225

@doctrinebot
Copy link
Author

Comment created by @beberlei:

Related Pull Request was closed: #225

@doctrinebot
Copy link
Author

Comment created by @beberlei:

This issue is referenced in Github Pull-Request GH-227
#227

@doctrinebot
Copy link
Author

Comment created by @beberlei:

Related Pull Request was closed: #227

@doctrinebot
Copy link
Author

Comment created by darkangel:

Alex mentioned on IRC that filters do not provide the functionality that the OP requires, so this issue should really re-opened, unless I'm missing something?

@doctrinebot
Copy link
Author

Comment created by @beberlei:

This issue is referenced in Github Pull-Request GH-237
#237

@doctrinebot doctrinebot added this to the 2.2 milestone Dec 6, 2015
@superdav42
Copy link

@asm89 I don't see how this resolves the original issue. It requires adding conditions on the related join and filters don't seem to have access to that kind of information.

@benface
Copy link

benface commented Jan 19, 2020

I don't think the original issue is resolved. By looking at the source code, there is simply no hook that allows us to add conditions to the first JOIN generated by a many-to-many relationship.

@oojacoboo
Copy link
Contributor

@benface You are actually mostly correct here, it does not. But, with annotations hooked into your filters, you can add your own custom decorators and then add these in the where clause.

It's a nasty hack IMO and support for custom/additional join conditions should be supported. We just hit a similar issue with many:one relationship and soft-delete (actually archive, similar idea) on the one side of the relationship. If we had the ability to define an additional join condition, we could simply check that archived_at IS NULL and we'd be all good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants