-
Notifications
You must be signed in to change notification settings - Fork 468
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
GroceryCrud search fails if ->where() or ->set_relation() are used - FIXED #341
Comments
|
👍 |
👍 Perfect, please include this in the next version of GC! |
Hi, this problem still persists when you select a table which has a self referencing field. For example I have a table named Company, with a field name headquarter which is a foreign key to Company. Error is: Column in where clause is ambiguous. |
Only for clarification: The code above fixes two things for me which are broken in stock GC 1.5.3:
|
@multimulti I'm also using GC 1.5.3 and ajax_list_info throws this error for me, with the updated function:
Maybe I did something wrong? I just replaced the original function with the updated one from above... |
GC does throw errors if you use set_relation() and two tables contain the same field/column name. Can you add the table name to your query so that the SQL comes out as "... AND company.name LIKE....."? Otherwise, I work round it by making sure I use unique field names - so change the 'name' field in the 'company' table to 'company_name' or similar. Something at the back of my mind is telling me that this "ambiguous clause" problem has been fixed in another thread, either here on Github or on the GC forum. If I'm right and I can find it I will post it here. A further clarification: My function also includes code from Amit which stops "dummy columns" causing search errors. |
What do you mean with add the table name? The error is in |
My code doesn't address the self-referencing problem, which I think is a known issue with GC. I suggest that you check out the GC forum - for example this post addresses the problem and proposes changes to two GC functions to fix it (you need to scroll down a bit to find the proposed code): http://www.grocerycrud.com/forums/topic/921-ajax-list-info-ambiguous-field-in-search/ Here is the key code: 1- Modify the get_columns method. 1.1- Add these lines before the first foreach: $ci =& get_instance(); 1.2- Add the next code inner the second foreach: else foreach ($data->columns as $column) if ( ! empty($new_column_name)) This work for me, Try with a test environment or separate example. |
I already tried this... It doesn't work unfortunately. Since you wrote the new |
I believe this is how GC implements the set_relation() calls - this is to make search work with the "target" table of the set_relation() call. e.g. "company" is stored as an id in another table, but you can get search results for, say, the company name when searching that first table thanks to the JOIN. GC is built on top of CodeIgniter and makes calls to CI's Query Builder Class, which is where the actual SQL will be generated. Good luck with drilling down into that! PS Have you actually tried using unique column names for all the tables in the database? (Or at least for the .name field in different tables) It may just fix the ambiguity problem. |
Hmm okay, well my other approach would be to give the company where I select from a name. Any guess where this comes from? |
Don't quite follow I'm afraid, but the answer is likely to be "deep inside CodeIgniter" :-( |
Hello @Doctor-Paul do you mind to send me a pull request of your code change in order to review the changes and maybe merge it with the grocery CRUD master code? If you don't know how to do it please let me know so I can help you with that :) I think this bug mentioned is fixed at the version 1.5.3 but not sure though! Thanks |
Haven't done this before but I can see the pull request button - I'll give it a go and shout if I need help. |
OK, I'm shouting :-) Pull request asks me to compare two versions of the code - can you talk me through the process? |
@Doctor-Paul I fixed the problem myself, I modified your code and added a
|
This message was created automatically by mail delivery software. A message that you sent could not be delivered to one or more of its [email protected] ------- This is a copy of the message, including all the headers. ------ Content preview: @Doctor-Paul I fixed the problem myself, I modified your code Content analysis details: (-4.1 points, 5.0 required) pts rule name description -5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/, high ----==_mimepart_568aa47e72738_a3a3f95c48432a0270948 @Doctor-Paul I fixed the problem myself, I modified your code and added a
Reply to this email directly or view it on GitHub: @Doctor-Paul I fixed the problem myself, I modified your code and added a
— ----==_mimepart_568aa47e72738_a3a3f95c48432a0270948-- |
Excellent! I will certainly add that to my code, many thanks. EDIT: This may solve your problem, but I think it will break the search if set_relation() is used to link to another table. |
Do you have an example where it might break? It seems to work for me! |
It's just a suspicion at the moment, will try to look into it further although it will be a few days before I can do that. |
@Doctor-Paul I have found one bug in the code in your second post here: When no explicit column ("Search all") is selected in the drop-down right to the search text, the search always returns no results. The expected behavior is to then search in ALL columns and return the results found. But, again, thank you a lot for your code, apart from that it seems to work perfectly. |
Is there workaround for this "Search all" problem? |
I am using the version 1.5.4 of GC and ci 3.0.4. with original "employees_management" example. I use "Search all" option to "mu" string, i get mysql error: Sql is: |
Hi, i may have a clue for this. Actually, when you have many table connected together (relation), proceeding a search, i'll use temp relation table and all col name of the main table defined with "set_table". So to have this fixed, it's required for all thoses col name to be prefixed with the main table name like this "exemple.ID" if ID is the ambiguous field. First thing to have it fixed, create a new method on Grocery_Crud_Model.php
Then, edit the Grocery_Crud.php library file and reach the method "set_ajax_list_queries". Within this method, to the last "else" statement, change this :
Into this : $table_name = $this->basic_model->get_table_name(); Hope it will help. |
the title are market as "FIXED" well this break the preview of the images! this patch are incomplete and buggy |
grocery crude not working with search all at the time of using where |
Hi pips! im having the same problem with i would like to know how to fix for the search box / and search filter for columns where should i start please reply. |
the patch provided in this issue works, but break the image previews! |
Cancel
Sent by Android Phone
…On 19 Jul 2017 5:02 pm, "PICCORO Lenz McKAY" ***@***.***> wrote:
the patch provided in this issue works, but break the image previews!
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#341 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAuS8_qdO8wy9vM9EOupFnOF_Fao5j_dks5sPgxYgaJpZM4GvhZd>
.
|
* scoumbourdis/grocery-crud#341 * pendiinete soporte en el ajax fix
… busqueda ajax * scoumbourdis/grocery-crud#341 * el codigo original esta comentado, para si en futuro hay problemas alternar
* bug was mentioned in scoumbourdis/grocery-crud#341
Using GC 1.5.2 and CI 3.0
As others have noted, "search all" fails completely if a ->where() statement has been used.
This is because GC is chaining calls to ->or_like() in addition to ->where() and the resulting SQL is simply wrong.
Search also fails for the same reason if made against a single column which is the subject of a call to ->set_relation()
I have produced a modified version of the set_ajax_list_queries() function which fixes this problem.
If a "where" exists, it generates a query string of chained OR LIKE statements and then feeds it to the ->where() function, which CI automatically ANDs with the existing where statement.
Happy to share this code, work in progress shown below:
The text was updated successfully, but these errors were encountered: