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

fix: wrong argument type passed when creating foreign key #14037

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@philly-php

philly-php commented Feb 22, 2018

we were passing a string into the function to create sql to create a foreign key constraint. This function expects an array, since foreign keys can reference multiple columns.

Fixes Issue #13941

Signed-off-by: Leonid Sklyut lsklyut@kbra.com

Before submitting pull request, please check that every commit:

  • [x ] Has proper Signed-Off-By
  • Has commit message which describes it
  • Is needed on it's own, if you have just minor fixes to previous commits, you can squash them
  • Any new functionality is covered by tests
fix: wrong argument type passed when creating foreign key
we were passing a string into the function to create sql to create a foreign key constraint. This function expects an array, since foreign keys can reference multiple columns.

Fixes Issue #13941

Signed-off-by: Leonid Sklyut <lsklyut@kbra.com>
@nijel

I think this should be addressed elsewhere, similarly as #14010

refactor: cast the foreign field to an array prior to using it
right now, there is no way to select multiple columns for an index, even though mysql allows it. Since the destination_foreign_column will always be a string at the moment, I am casting it to an array prior to using it, as the code expects it to be one.

we were passing a string into the function to create sql to create a foreign key constraint. This function expects an array, since foreign keys can reference multiple columns.

Fixes Issue #13941

Signed-off-by: Leonid Sklyut <lsklyut@kbra.com>
@codecov

This comment has been minimized.

codecov bot commented Feb 23, 2018

Codecov Report

Merging #14037 into master will increase coverage by <.01%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master   #14037      +/-   ##
============================================
+ Coverage     55.44%   55.44%   +<.01%     
  Complexity    14364    14364              
============================================
  Files           494      494              
  Lines         70513    70522       +9     
============================================
+ Hits          39097    39104       +7     
- Misses        31416    31418       +2
@lsklyut

This comment has been minimized.

lsklyut commented Feb 23, 2018

@nijel , I've pulled the array casting up to where we grab the $foreign_field from $destination_foreign_column. As it stands, it does not look like this input would ever be anything but a string, even though the code further down and in _getSQLToCreateForeignKey relies on it being an array. I do think the array logic makes sense, since mysql does allow multi-column foreign keys (although it doesn't look like the UI allows you to do that yet)

@lsklyut

This comment has been minimized.

lsklyut commented Feb 23, 2018

this would also fix #14009

@mauriciofauth mauriciofauth self-assigned this Mar 27, 2018

mauriciofauth added a commit that referenced this pull request Mar 27, 2018

Fixes error in foreign key sql generation + drop foreign key contraint (
#14031)

* Fixes error in foreign key sql genration, issue #14030
Changed:
	templates/table/relation/foreign_key_row.twig
The destination_column_names were getting passed as a 1d array, the names when extracted gave a string. Passing it as a 2d array (see templates/table/relation/foreign_key_row.twig lines 123,133) fixes the issue.

Sign-Off-By: Lakshay arora (b16060@students.iitmandi.ac.in)

* Fixes error in foreign key sql generation + drop foreign key contraint, issue #14030
Changed:
	templates/table/relation/foreign_key_row.twig

Fix #13941
Fix #14009
Close #14037
Close #14010

Signed-off-by: Lakshay arora <b16060@students.iitmandi.ac.in>
(cherry picked from commit 00b5962)
Signed-off-by: Maurício Meneghini Fauth <mauriciofauth@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment