-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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-2989: ORM should allow custom index names for foreign associations. #3753
Comments
Comment created by jsuggs: Here is a full example I think this mainly has to do with the doctrine auto-generated indexes. |
Comment created by @Ocramius: IIRC, index names cannot currently be assigned, and are computed automatically - that's why the ORM is behaving like that. I don't think this needs a fix right now - lowering priority |
Comment created by @deeky666: [~jsuggs] I also could not reproduce this bug in DBAL. I wonder if that is related to foreign key declarations (ORM relations) only. In fact, you cannot define index names for relations (foreign keys) in ORM at the moment. Therefore ORM always generates index names automatically. If you use custom names for those indexes in your online schema, the comparator will surely output the index renaming statement. I guess defining the index at table level in your mapping manually won't help here either. |
Comment created by jsuggs: I guess I get it, but its kinda a big deal and I'll have to old off upgrading as a result. I personally find this more of a regression due to the unintended consequences. The schema diff is now 100% useless (to me) as I rename all of my indexes to a more semantic name. My off the cuff thought on ORM index naming is that you expand the naming strategy to include support for the indexes. Given the full set of criteria you should be able to come up with some sort of semantic naming and only on namespace collision to you "fallback" to a hash of the columns (solves your issues and mine). |
Comment created by jsuggs: To state differently, I (personally) value the schema diff tool much more than custom index (re)naming since the (overall) ability to use custom index names is not complete (due to the ORM issues Steve outlined above). If the support to (re)name foreign key indexes can't go out in parallel to this, I'd like to offer a middle ground of making the index renaming configurable (ie ability to opt-out of functionality). Sorry to be negative towards a new feature, but its really an inconvenience for me, and I suspect others since Strate also expressed concerns in the PR. Let me know how I can be of help in working towards a resolution. |
Comment created by @beberlei: [~jsuggs] What do you mean with "just upgraded"? Which to which version? The index renaming and coverage feature is very old already, not sure what is happening. Do you define an index for a relation column? |
Comment created by @deeky666: [beberlei] This definitely has to do with the comparator now comparing index names also. |
Comment created by jsuggs: Sorry for the confusion. Here is the PR that introduced the functionality in question. Benjamin, I had just upgraded DBAL to the latest 2.5 master. This is only indirectly related to the ORM functionality as the ORM SchemaTool uses the DBAL Comparator to generate is schema diff. Steve, I see three potential solutions (in my personal order of preference).
I realize that #1 would be a bit more work, but I think offers enhanced functionality. #2 is a good, but would require additional annotations/mappings. #3 is probably the least dev effort but just doesn't feel right (to me). Again, I don't want to seem critical of the dev effort, but its an unintended consequence that will keep me from being able to upgrade DBAL to 2.5. |
Comment created by @deeky666: [~jsuggs] You are right the comparator is somewhat responsible for the "unnecesarry" schema diffs it now creates. However its functionality is completely working IMO. Try creating and comparing your schema example on DBAL level only (without utilizing ORM). You will see that it works. I see and understand that those index name updates are annoying but they are not WRONG. If you have a mapping for a relation with an unnamed index and rename this index in your online schema manually, I would even expect the schema tool to generate this diff. Because this is the whole point of the index rename feature. The problem we have here is that there currently is no way in ORM to define the index name on association mappings. There the following happens in your example when comparing the online and offline schema with the schema tool: (abbreviated to the important steps, chronological order)
Just to clarify what happens under the hood and that it is an ORM issue, not DBAL! |
Comment created by jsuggs: Here is a first shot at basic support for allowing the mapping of the index name. |
Comment created by @deeky666: [jsuggs] I have worked on a PR for this already using the exact same approach you just mentioned. However I am not quite sure about ManyToMany yet (because you would have to be able to define both index names there) and also I talked to [beberlei] and he does not seem to be happy with this approach. So I stopped work on this for the moment. |
Comment created by @deeky666: Moved this to ORM issues. |
Comment created by jsuggs: Steve I both agree and disagree. I think my underlying issue is your step 1.1. The ORM creates the index named "IDX_76FF8CAA8E48560F" and there is NO extension point for being able to override that behavior. FWIW, I've never been fond of the way that the ORM uses the AbstractAsset::_generateIdentifierName for generating the index and foreign key. So I definitely agree that its is an ORM issue not DBAL issue :). However, as I stated previously, until the ORM is updated/patched to allow for index naming, this (in my opinion) is a regression despite it working the way it currently does (correct or not). I can close out this issue and open one for ORM. Do you have a old branch and/or ORM ticket that I can reference? Thanks for engaging in the dialog! I hope to be of assistance in helping come up with a solution that gets everyone happy. |
Comment created by @deeky666: [~jsuggs] Thanks for updating the description and thanks for assisting on this issue. We will find a solution for this before the final release of 2.5. If we cannot find a good solution until then we might also consider reverting the index renaming feature on DBAL (but I would like to avoid that). I will discuss this issue with the other core devs again and see what we can come up with. |
Comment created by jsuggs: No problem, here is the start of a PR to maybe address the index names |
Comment created by @doctrinebot: |
Comment created by @doctrinebot: |
Issue was closed with resolution "Fixed" |
At this moment is possible to add names on foreign keys? [Doctrine\Common\Annotations\AnnotationException] |
Defining an index name is possible (ORM simply skips on already defined indexes). FK names are not yet allowed to be customized. |
Are there any plans to make FK names customisable? |
No, as it is not important functionality. Customising every detail of the On 1 Sep 2016 11:23, "Mantas Urnieža" notifications@github.com wrote:
|
Hi, I have some fields that are not used in mapping as FK/PK, I just need to index them manually because of performanace as I use them in searching queries, these are not FK. I was doing the same as you said: just adding them in top of entity with its column name and doctrine was not generating hash for them. It works fine with MySQL. But if I have a same column name in two different tables and I want to index both, when I was trying to dump mapping into a PostGre database with command tool, it has problem with PostGre for duplicate index name, so I thought to find the method how doctrine generates index hash, and generate hash for them manually with the same format instead of hashing it on my own. I found the method, generated hash, and now it works fine with PostGre. Please advice am I wrong somewhere or is there any better way? Thanks, |
This IS important functionaly and it's also VERY BASIC functionality of every database. I spent several hours investigating why on every diff all FK CONSTRAINTS are removing and creating again. Problem is here: AbstractAsset.php#L208. Doctrine makes FK intentifiers in uppercase while in database are in lowecase so I have to modify DBAL source if I want to use migrations diff. Can anyone explain to me why I'm able to define UNIQUE constraints but not FK constraints? |
@seticzech Which database is changing the name back to lowercase? Might be related to doctrine/dbal#2974. |
FK constraints are defined via associations, and "owned" by the ORM mappings. You are free to ise schema migrations and DDL to manage them yourself though. |
Again: customising your DB schema is up to you: the ORM provides superficial tooling for managing the schema, but it won't go fine-grained. It is OK to write database migrations to make things look pretty with your own definition of "pretty". FKs on association names are one example of that. Meanwhile, since everyone is taking the opportunity to 👎 on this, I'm locking this as "resolved". |
Jira issue originally created by user jsuggs:
Now that the DBAL allows/checks for renamed indexes, the ORM implementation needs to be enhanced to allow custom naming of the indexes for foreign associations.
The text was updated successfully, but these errors were encountered: