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

[6.x] Use qualified column names in pivot query #36720

Merged
merged 1 commit into from
Mar 26, 2021
Merged

[6.x] Use qualified column names in pivot query #36720

merged 1 commit into from
Mar 26, 2021

Conversation

vdlp-mw
Copy link
Contributor

@vdlp-mw vdlp-mw commented Mar 23, 2021

Use qualified column names in pivot query. This allows a user to extend the query with a join that may include ambiguous column names.

Old extended query example:

# SQLSTATE[23000]: Integrity constraint violation: 1052
# Column 'rule_id' in WHERE clause is ambiguous

SELECT `cmp_rule`.*
FROM   `cmp_rule`
       RIGHT JOIN `reg_rule`
               ON `reg_rule`.`rule_id` = `cmp_rule`.`rule_id`
WHERE  `base_id` = 1
       AND `rule_id` IN ( 5 ) 

New extended query example:

SELECT `cmp_rule`.*
FROM   `cmp_rule`
       RIGHT JOIN `reg_rule`
               ON `reg_rule`.`rule_id` = `cmp_rule`.`rule_id`
WHERE  `cmp_rule`.`base_id` = 1
       AND `cmp_rule`.`rule_id` IN ( 5 ) 

I also created a issue for this PR here: #36718

@driesvints driesvints changed the title Use qualified column names in pivot query [6.x] Use qualified column names in pivot query Mar 23, 2021
@vdlp-mw
Copy link
Contributor Author

vdlp-mw commented Mar 23, 2021

Two similar PRs that also introduce the usage of qualified columns that may be worth looking into.
PR: Fix ambiguous UPDATED_AT columns #26031
PR: Fix qualified column in withAggregate #35061

@driesvints
Copy link
Member

Can you also try this on PostgreSQL? Seems like one of the linked prs was reverted because it didn't work on that.

@vdlp-mw
Copy link
Contributor Author

vdlp-mw commented Mar 23, 2021

I've re-ran the tests with the pgsql driver and all tests are still passing. Some modifications to the docker-composer.yaml and test.sh files were needed to test with postgresql. You can find the changes in here: vdlp-mw@e0b7a7b or on this branch vdlp-mw/framework@qualified-foreign-pivot-keyname-pgsql-test.

For some odd reason some unrelated tests hanged after switching the database driver. I've re-ran the tests with the following command:

$ ./bin/test.sh --filter '/(Illuminate\\Tests\\Database\\DatabaseEloquentMorphToManyTest)/'
Creating network "framework_default" with the default driver
Creating framework_redis_1     ... done
Creating postgres              ... done
Creating framework_memcached_1 ... done
Waiting for services to boot   ...
PHPUnit 9.5.4 by Sebastian Bergmann and contributors.

Runtime:       PHP 7.4.16
Configuration: /data/phpunit.xml.dist

....                                                                4 / 4 (100%)

I also re-ran the tests with the following filter with no reported issues:
./bin/test.sh --filter '/(Illuminate\\Tests\\Database)/'.

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.

4 participants