fix for #38585: Signed primary columns changed value #38920

KrasnoshchokBohdan · 2024-07-12T08:24:51Z

Description (*)

changed "unsigned" attribute for "value_id" column for tables from original list (This is applicable to the following tables:... from author post) + catalog_category_entity_datetime, customer_address_entity_datetime

Related Pull Requests

Fixed Issues (if relevant)

Fixes Database tables with primary column having signed instead of unsigned definition which is loss of usable values #38585

Manual testing scenarios (*)

mysql -u root -p
use (your magento2 db name)
desc catalog_product_entity_text (or any table from the list)
check "value_id" type, should be unsigned

Questions or comments

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
All automated tests passed successfully (all builds are green)

…ead of unsigned definition which is loss of usable values - changed unsigned attribute for value_id column for tables from original list + catalog_category_entity_datetime customer_address_entity_datetime

m2-assistant · 2024-07-12T08:24:55Z

Hi @KrasnoshchokBohdan. Thank you for your contribution!
Here are some useful tips on how you can test your changes using Magento test environment.

Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento give me test instance - deploy test instance based on PR changes
@magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

@magento run all tests - run or re-run all required tests against the PR changes
@magento run <test-build(s)> - run or re-run specific test build(s)
For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:

Database Compare
Functional Tests CE
Functional Tests EE
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests
Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.

KrasnoshchokBohdan · 2024-07-12T08:25:47Z

@magento run all tests

KrasnoshchokBohdan · 2024-07-12T12:45:53Z

@magento run Functional Tests B2B Functional Tests CE Functional Tests EE

magento-automated-testing · 2024-07-12T12:46:00Z

Failed to run the builds. Please try to re-run them later.

KrasnoshchokBohdan · 2024-07-12T13:37:44Z

@magento run Functional Tests B2B

KrasnoshchokBohdan · 2024-07-15T06:36:49Z

@magento run Functional Tests B2B

KrasnoshchokBohdan · 2024-07-15T09:59:35Z

@magento run Functional Tests CE

KrasnoshchokBohdan · 2024-07-15T12:50:56Z

@magento run Functional Tests EE

KrasnoshchokBohdan · 2024-07-15T17:31:27Z

Failed tests seems flaky to me

engcom-Hotel · 2024-07-23T06:17:28Z

@magento run all tests

engcom-Hotel · 2024-07-24T14:26:48Z

We are moving this PR On Hold as some internal discussion is going on related to it.

engcom-Hotel · 2024-09-10T06:04:11Z

@magento run all tests

KrasnoshchokBohdan · 2024-09-18T06:28:18Z

@magento run Unit Tests Static Tests WebAPI Tests

magento-automated-testing · 2024-09-18T06:28:25Z

Failed to run the builds. Please try to re-run them later.

KrasnoshchokBohdan · 2024-09-18T06:29:55Z

@magento run Unit Tests

KrasnoshchokBohdan · 2024-09-18T07:49:27Z

@magento run WebAPI Tests

KrasnoshchokBohdan · 2024-09-18T10:13:03Z

@magento run Magento Health Index

KrasnoshchokBohdan · 2024-09-18T12:12:21Z

@magento run Static Tests Semantic Version Checker

magento-automated-testing · 2024-09-18T12:12:28Z

Failed to run the builds. Please try to re-run them later.

KrasnoshchokBohdan · 2024-09-18T12:13:02Z

@magento run Static Tests, Semantic Version Checker

KrasnoshchokBohdan · 2024-09-18T13:20:19Z

@magento run Functional Tests B2B, Functional Tests CE, Functional Tests EE

KrasnoshchokBohdan · 2024-09-18T17:02:17Z

@ihor-sviziev @engcom-Charlie

Hi! Now it seems to look better, what do you think?

engcom-Charlie · 2024-09-19T06:25:22Z

@ihor-sviziev @engcom-Charlie

Hi! Now it seems to look better, what do you think?

Hi @KrasnoshchokBohdan,

Thank you for the updation!

@ihor-sviziev, as suggessted by you in here also in other comments like here, it seems that @KrasnoshchokBohdan have made the changes accordingly.

Requesting you for the review.

Thank you!

ihor-sviziev

@KrasnoshchokBohdan, I just reviewed this PR again, and see that by adjusting type of the columns we were getting issues like this:

  Column definition "product_tax_class_id" and reference column definition "c
  lass_id" are different in tables "tax_calculation" and "tax_class"
  Column definition "customer_tax_class_id" and reference column definition "
  class_id" are different in tables "tax_calculation" and "tax_class"
  Column definition "tax_calculation_rate_id" and reference column definition
   "tax_calculation_rate_id" are different in tables "tax_calculation" and "t
  ax_calculation_rate"
  Column definition "tax_calculation_rule_id" and reference column definition
   "tax_calculation_rule_id" are different in tables "tax_calculation" and "t
  ax_calculation_rule"
  Column definition "tax_calculation_rate_id" and reference column definition
   "tax_calculation_rate_id" are different in tables "tax_calculation_rate_ti
  tle" and "tax_calculation_rate"

You've reverted these changes, which is great, thanks!

I am wondering if people might have similar issues with other tables that you've changed just because they have foreign keys to these tables in custom extensions, and Magento doesn't have those custom extensions.

I believe the number of potentially affected people will be higher than the number of people getting to the issue you're trying to resolve. Those who need it can apply these changes as a patch and verify that they do not affect their Magento installation.

Having that in mind, I don't know if we should accept this pull request.
Please give us your thoughts on this.

engcom-Hotel · 2024-09-23T05:48:29Z

@magento run all tests

rostilos · 2024-09-27T07:04:50Z

@ihor-sviziev
Perhaps to maintain compatibility there is an option to create a datapatch instead of directly modifying the schema DB?
I'm not sure that's possible using the M2 framework toolkit, but maybe you can give me more hints.

ihor-sviziev · 2024-09-27T16:07:36Z

@rostilos, unfortunately it won’t help because of dependencies in 3rt party extensions. These extensions still will have a db schema files, which declare the current state. I don’t think it’s possible to make backward compatible version at all, while benefit from this change will be for pretty small amount for installations

engcom-Charlie · 2024-10-01T06:31:05Z

Hi @engcom-Hotel,

Can you please go through @ihor-sviziev's comment and review this PR again to decide whether we can proceed ahead with this PR or not.

Thank you!

engcom-Hotel · 2024-10-04T13:05:29Z

Hello @KrasnoshchokBohdan,

I agree with @ihor-sviziev's input here. This PR may impact some third-party extension that uses these tables as foreign keys.

I think we can close this PR.

Please suggest @KrasnoshchokBohdan @rostilos @ihor-sviziev.

Thanks

engcom-Hotel · 2024-10-10T12:22:16Z

We are closing this PR as the fix may impact the 3rd party extensions.

hostep · 2024-10-10T15:22:58Z

I heard some whispers a few months ago about a potential upcoming Magento 2.5.0 version, so maybe if that actually happens, we can open this PR again, because it's a new major version where backwards compatibility breaking is allowed

rostilos · 2024-10-18T11:18:43Z

@engcom-Hotel
Perhaps, given the post above, we can reopen it and assign an appropriate label?

ihor-sviziev · 2024-10-19T10:12:46Z

I don't mind re-opening it for 2.5, but I didn't hear any news about it

m2-assistant · 2024-10-19T10:12:49Z

Hi @KrasnoshchokBohdan. Thank you for your contribution!
Here are some useful tips on how you can test your changes using Magento test environment.
❗ Automated tests can be triggered manually with an appropriate comment:

@magento run all tests - run or re-run all required tests against the PR changes
@magento run <test-build(s)> - run or re-run specific test build(s)
For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:

Database Compare
Functional Tests CE
Functional Tests EE
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests
Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.

Priyakshic · 2024-10-22T14:57:43Z

Hi @KrasnoshchokBohdan ,

Thanks for the contribution!
Currently, we are not processing the PRs targeted for 2.5-develop. Therefore, we are removing the "Community Picked" label for this PR.
We will revisit and pick this PR in the future whenever we start working on 2.5-develop.

Thanks

magento#38585: Database tables with primary column having signed inst…

4db96bd

…ead of unsigned definition which is loss of usable values - changed unsigned attribute for value_id column for tables from original list + catalog_category_entity_datetime customer_address_entity_datetime

m2-community-project bot added the Priority: P3 May be fixed according to the position in the backlog. label Jul 12, 2024

m2-github-services added Partner: Perspective Team partners-contribution Pull Request is created by Magento Partner labels Jul 12, 2024

m2-community-project bot added the Progress: pending review label Jul 12, 2024

sinhaparul added the Project: Community Picked PRs upvoted by the community label Jul 17, 2024

engcom-Hotel self-requested a review July 23, 2024 06:16

m2-community-project bot added Progress: review and removed Progress: pending review labels Jul 23, 2024

Merge branch '2.4-develop' into fix-for-issue-38585

8d517a6

engcom-Hotel requested a review from ihor-sviziev July 23, 2024 12:29

m2-community-project bot added Progress: on hold and removed Progress: review labels Jul 24, 2024

Merge branch '2.4-develop' into fix-for-issue-38585

dad5335

engcom-Hotel added Progress: review and removed Progress: on hold labels Sep 10, 2024

engcom-Hotel approved these changes Sep 10, 2024

View reviewed changes

m2-community-project bot assigned engcom-Hotel Sep 10, 2024

ihor-sviziev requested changes Sep 19, 2024

View reviewed changes

Merge branch '2.4-develop' into fix-for-issue-38585

926caa0

engcom-Hotel closed this Oct 10, 2024

m2-community-project bot removed the Progress: needs update label Oct 10, 2024

ihor-sviziev added the Release Line: 2.5 label Oct 19, 2024

ihor-sviziev reopened this Oct 19, 2024

m2-community-project bot added the Progress: needs update label Oct 19, 2024

Priyakshic removed the Project: Community Picked PRs upvoted by the community label Oct 22, 2024

fix for #38585: Signed primary columns changed value #38920

Are you sure you want to change the base?

fix for #38585: Signed primary columns changed value #38920

Uh oh!

Conversation

KrasnoshchokBohdan commented Jul 12, 2024

Description (*)

Related Pull Requests

Fixed Issues (if relevant)

Manual testing scenarios (*)

Questions or comments

Contribution checklist (*)

Uh oh!

m2-assistant bot commented Jul 12, 2024

Uh oh!

KrasnoshchokBohdan commented Jul 12, 2024

Uh oh!

KrasnoshchokBohdan commented Jul 12, 2024

Uh oh!

magento-automated-testing bot commented Jul 12, 2024

Uh oh!

KrasnoshchokBohdan commented Jul 12, 2024

Uh oh!

KrasnoshchokBohdan commented Jul 15, 2024

Uh oh!

KrasnoshchokBohdan commented Jul 15, 2024

Uh oh!

KrasnoshchokBohdan commented Jul 15, 2024

Uh oh!

KrasnoshchokBohdan commented Jul 15, 2024

Uh oh!

engcom-Hotel commented Jul 23, 2024

Uh oh!

engcom-Hotel commented Jul 24, 2024

Uh oh!

engcom-Hotel commented Sep 10, 2024

Uh oh!

KrasnoshchokBohdan commented Sep 18, 2024

Uh oh!

magento-automated-testing bot commented Sep 18, 2024

Uh oh!

KrasnoshchokBohdan commented Sep 18, 2024

Uh oh!

KrasnoshchokBohdan commented Sep 18, 2024

Uh oh!

KrasnoshchokBohdan commented Sep 18, 2024

Uh oh!

KrasnoshchokBohdan commented Sep 18, 2024

Uh oh!

magento-automated-testing bot commented Sep 18, 2024

Uh oh!

KrasnoshchokBohdan commented Sep 18, 2024

Uh oh!

KrasnoshchokBohdan commented Sep 18, 2024

Uh oh!

KrasnoshchokBohdan commented Sep 18, 2024

Uh oh!

engcom-Charlie commented Sep 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ihor-sviziev left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

engcom-Hotel commented Sep 23, 2024

Uh oh!

rostilos commented Sep 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ihor-sviziev commented Sep 27, 2024

Uh oh!

engcom-Charlie commented Oct 1, 2024

Uh oh!

engcom-Hotel commented Oct 4, 2024

Uh oh!

engcom-Hotel commented Oct 10, 2024

Uh oh!

hostep commented Oct 10, 2024

Uh oh!

rostilos commented Oct 18, 2024

engcom-Charlie commented Sep 19, 2024 •

edited

Loading

ihor-sviziev left a comment •

edited

Loading

rostilos commented Sep 27, 2024 •

edited

Loading