Skip to content

fix: prevent duplicate table name errors #3216

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

Merged
merged 2 commits into from
Feb 26, 2025

Conversation

Seb33300
Copy link
Contributor

@Seb33300 Seb33300 commented Feb 21, 2025

Fixes #2871, #880 (maybe all issues with Self Join tag )

This PR fixes the duplicate table name SQL error by adding aliases to joined tables.
It also supports multi levels joins:
Eg:

- article.author.name
- article.category.created_by.name
- article.comments[].user.name

⚠️ This PR includes a BC break

As you can see with the test case I modified, this PR comes with a small BC break.
If a column has defined a custom order by using orderColumn(), the table is now using an alias that will not match with the table name.
A new parameter has been added to the orderColumn() callable to pass the column name with alias.

DataTables::eloquent($model)
    ->orderColumn('relation.name', function ($query, $order, $column) {
        // $column = alias.name of the relation.name column
        $query->orderBy($column, $order);
    });

@Seb33300 Seb33300 force-pushed the fix-duplicate-table-names branch 2 times, most recently from e5dd906 to f643dcb Compare February 21, 2025 09:29
@Seb33300 Seb33300 marked this pull request as draft February 21, 2025 09:32
@Seb33300
Copy link
Contributor Author

I will check failing test next week

@Seb33300 Seb33300 force-pushed the fix-duplicate-table-names branch from 8e5e367 to c8af15e Compare February 24, 2025 01:45
@Seb33300 Seb33300 marked this pull request as ready for review February 24, 2025 02:33
@Seb33300
Copy link
Contributor Author

Hi @yajra, PR is now ready for review 🙏

@obyajra
Copy link

obyajra commented Feb 24, 2025

The changes look good. Will hold off for now and review again in v12 (Laravel 12) release since there is a BC.

Will try to complete the v12 within this or earlier next week.

Thanks!

@yajra
Copy link
Owner

yajra commented Feb 26, 2025

@Seb33300 can you provide an example of the table aliases generated in this PR? One concern I have is the Oracle limitation that only accepts up to 30 chars for object/table name length.

Initial Laravel 12 support merged on the master branch.

Thanks!

@Seb33300
Copy link
Contributor Author

Seb33300 commented Feb 26, 2025

@yajra It concatenates all parent relation names to create a unique alias name:

updated_by.name => _updated_by.name
created_by.company.name => _created_by_company.name

Here is an example of query generated (on MySQL):

select `users`.* 
from `users` 
left join `users` as `_updated_by` on `users`.`updated_user_id` = `_updated_by`.`id` 
where `_updated_by`.`name` LIKE '%%seb%%' 
order by `_updated_by`.`name` asc 
limit 10 
offset 0

@Seb33300
Copy link
Contributor Author

Seb33300 commented Feb 26, 2025

One concern I have is the Oracle limitation that only accepts up to 30 chars for object/table name length.

Please note that starting Oracle Database 12c Release 2 (12.2) released in 2017, the maximum size is now 128 chars.
https://oracle-base.com/articles/12c/long-identifiers-12cr2

And all Oracle versions prior that one are already no longer supported by Oracle:
https://en.wikipedia.org/wiki/Oracle_Database

@yajra
Copy link
Owner

yajra commented Feb 26, 2025

One concern I have is the Oracle limitation that only accepts up to 30 chars for object/table name length.

Please note that starting Oracle Database 12c Release 2 (12.2) released in 2017, the maximum size is now 128 chars. https://oracle-base.com/articles/12c/long-identifiers-12cr2

And all Oracle versions prior that one are already no longer supported by Oracle: https://en.wikipedia.org/wiki/Oracle_Database

Yes, unfortunately - I still have projects that uses old Oracle version >.<

@Seb33300
Copy link
Contributor Author

Seb33300 commented Feb 26, 2025

Unless you have columns with multiple joins, It's unlikely to happen, but still theoretically possible.

A workaround I can suggest is to use md5 to generate a hash of the alias in case it's more than 30 chars.

@yajra
Copy link
Owner

yajra commented Feb 26, 2025

Thanks and will review first on an actual project. I just recalled that we rarely use relation and opt to use join statements for more stable results.

@yajra
Copy link
Owner

yajra commented Feb 26, 2025

Tested on a new Laravel 12 app and works as expected. Tested self-join SQL and seems to be fixed:

SQL Generated:

select * from "users" left join "users" as "_creator" on "users"."created_by" = "_creator"."id" order by "_creator"."name" desc limit 10 offset 0

Is the _ on alias (_creator) intended?

@yajra yajra merged commit d0baf8c into yajra:master Feb 26, 2025
8 of 9 checks passed
@Seb33300
Copy link
Contributor Author

Is the _ on alias (_creator) intended?

Yes, this is to prevent a potential conflict with a table name if the relation name is the same as the table name.

@yajra
Copy link
Owner

yajra commented Feb 26, 2025

Thanks for the feedback. Released on v12 🚀

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

Successfully merging this pull request may close these issues.

Can't use column searches in same table relations
3 participants