#21853: Allow mview indexers to use different entity columns. #21857

nikunjkotecha · 2019-03-20T12:49:09Z

Related Pull Requests

https://github.com/magento/partners-magento2ee/pull/413

This is not final expected change and only here to show the issue and proposed direction for the fix.

magento-cicd2 · 2019-03-20T12:49:12Z

All committers have signed the CLA.

magento-engcom-team · 2019-03-20T12:49:13Z

Hi @nikunjkotecha. Thank you for your contribution
Here is some useful tips 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-engcom-team give me test instance - deploy test instance based on PR changes
@magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

orlangur · 2019-03-20T23:04:55Z

@akaplya could you check if current behavior is intentional?

aleron75 · 2019-07-08T16:28:21Z

Hello, I just experiencing the same issue and IMO it's not an intentional behavior since the result changes depending on the way you set the indexing mode on all the indexes or on a single one.

josefbehr · 2019-08-23T10:38:51Z

@magento-engcom-team How shall we proceed with this? Merge this proposed change for the time being, or is further work required?

ihor-sviziev · 2020-01-24T13:17:35Z

@sdzhepa @slavvka,
This PR got stuck for almost 1 year.
Could you check what's wrong with it? Should we get approval?

ihor-sviziev · 2020-02-14T06:48:32Z

@slavvka you moved this PR to "on hold", could you describe why and what we're waiting for?

slavvka · 2020-03-03T16:21:34Z

@ihor-sviziev sorry for not having added the explanation. We are waiting for the review from Magento architect team

ihor-sviziev · 2020-03-03T16:28:40Z

@nikunjkotecha while we're waiting for Architecture team review - could you sign Adobe CLA?

nikunjkotecha · 2020-03-04T05:29:46Z

done

lenaorobei

Could you please change the logic in order to read the correct column name in the buildStatement method instead of create and delete.

The issue is caused because thе changelog is missing subscription column name.

lenaorobei · 2020-03-27T20:06:08Z

lib/internal/Magento/Framework/Mview/View/Subscription.php

@@ -117,8 +117,19 @@ public function create()

            // Add statements for linked views
            foreach ($this->getLinkedViews() as $view) {
+                // Store current column name for reverting back later.
+                $originalColumnName = $this->getColumnName();


This solution looks like we are trying to hack the subscription logic by changing object property and misuse the columnName .
What we actually need is subscription column name, not the changelog column name (which is all the time the same).

@lenaorobei I looked at the code again and your comment and it's a bit confusing
1/ It seems we both agree we need to use the subscription column name
2/ I'm not sure why you mention not the changelog column name, we need two column names - changelog column is for the changelog table and that's not going to change (even you mentioned same). What we need is to change how we read the subscription column name

My understanding, Subscription is created once for all the subscribers together and whichever subscription we modify last - columnName is used from that and re-used for all the triggers.

I've tried to simplify the code to make it look not (or less) hacky but to me it seems changes are required in the same direction as I've taken or a major rework would be required in how the subscriptions are created (refactor how the statement is built for linked views)

You are right, we need both changelog and subscription column names, but $this->getColumnName() is changelog column, not the subscription one.
I recommended to move the logic of getting its name to the buildStatement method in order to cover both cases - create and delete.

We don't have any object / data available as of now in buildStatement to get the column name of the linked view for which buildStatement is called.

We can pass full view object or the way I've done to pass the subscription name, please advice

I thought we can get view from the changelog.

not at-least as per the interface

https://github.com/magento/magento2/blob/2.4-develop/lib/internal/Magento/Framework/Mview/View/ChangelogInterface.php

I may be wrong, please guide if I'm missing something here

Got it.
Let's pass the view object then.

# Conflicts: # lib/internal/Magento/Framework/Mview/Test/Unit/View/SubscriptionTest.php # lib/internal/Magento/Framework/Mview/View/Subscription.php

engcom-Charlie · 2020-12-28T13:26:43Z

@magento run all tests

magento-engcom-team · 2020-12-29T05:43:48Z

Hi @ihor-sviziev, thank you for the review.
ENGCOM-8474 has been created to process this Pull Request

engcom-Bravo · 2020-12-29T08:40:33Z

✔️ QA Passed

The result is the same as in the report above #21857 (comment)

gabrieldagama · 2021-01-07T11:14:02Z

@magento run all tests

gabrieldagama · 2021-01-07T12:34:57Z

@magento run Functional Tests EE

gabrieldagama · 2021-01-07T13:29:32Z

@magento run Functional Tests B2B

gabrieldagama · 2021-01-07T14:13:33Z

@magento run Functional Tests EE

gabrieldagama · 2021-01-07T15:07:45Z

@magento run Functional Tests B2B

gabrieldagama · 2021-01-08T09:57:29Z

@magento run Functional Tests EE

gabrieldagama · 2021-01-11T09:49:20Z

@magento run all tests

…umns. #21857

m2-assistant · 2021-01-24T12:14:50Z

Hi @nikunjkotecha, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

magento#21853: Allow mview indexers to use different entity columns.

891198c

magento-engcom-team added Component: Mview Release Line: 2.3 labels Mar 20, 2019

aleron75 marked this pull request as ready for review August 19, 2019 07:28

josefbehr self-assigned this Aug 23, 2019

sidolov changed the base branch from 2.3-develop to 2.4-develop December 5, 2019 17:21

sidolov added Release Line: 2.4 and removed Release Line: 2.3 labels Dec 5, 2019

ghost added the Progress: pending approval label Jan 24, 2020

ihor-sviziev added the Requires status update label Jan 24, 2020

Merge branch '2.4-develop' into 21853

7da4940

slavvka requested a review from akaplya as a code owner January 24, 2020 16:27

ghost added Progress: on hold and removed Progress: pending approval labels Feb 10, 2020

ihor-sviziev self-assigned this Mar 3, 2020

ihor-sviziev mentioned this pull request Mar 3, 2020

January 8, 2020 magento/architecture#349

Open

lenaorobei requested changes Mar 27, 2020

View reviewed changes

ghost assigned lenaorobei Mar 27, 2020

ghost added Progress: needs update and removed Progress: on hold labels Mar 27, 2020

chalov-anton mentioned this pull request Dec 15, 2020

Database table triggers creation using mview multiple subscriptions with different entity columns for one table #31192

Closed

5 tasks

m2-community-project bot added Progress: extended testing and removed Progress: accept labels Dec 23, 2020

engcom-Charlie added 2 commits December 28, 2020 12:13

Merge branch '2.4-develop' into 21853

094decc

# Conflicts: # lib/internal/Magento/Framework/Mview/Test/Unit/View/SubscriptionTest.php # lib/internal/Magento/Framework/Mview/View/Subscription.php

fixing conflict merge; fixed unit test

910eb41

m2-community-project bot added Progress: pending review and removed Progress: extended testing labels Dec 28, 2020

engcom-Charlie requested review from ihor-sviziev and gabrieldagama December 28, 2020 18:05

ihor-sviziev approved these changes Dec 29, 2020

View reviewed changes

m2-community-project bot added Progress: ready for testing and removed Progress: pending review labels Dec 29, 2020

m2-community-project bot added Progress: testing in progress and removed Progress: ready for testing labels Dec 29, 2020

m2-community-project bot added Progress: accept and removed Progress: testing in progress labels Dec 29, 2020

Merge branch '2.4-develop' into 21853

be3f84b

magento-engcom-team pushed a commit that referenced this pull request Jan 24, 2021

ENGCOM-8474: #21853: Allow mview indexers to use different entity col…

e5d0279

…umns. #21857

magento-engcom-team merged commit 7327dc0 into magento:2.4-develop Jan 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#21853: Allow mview indexers to use different entity columns. #21857

#21853: Allow mview indexers to use different entity columns. #21857

nikunjkotecha commented Mar 20, 2019 •

edited by engcom-Charlie

Loading

magento-cicd2 commented Mar 20, 2019 •

edited

Loading

magento-engcom-team commented Mar 20, 2019

orlangur commented Mar 20, 2019

aleron75 commented Jul 8, 2019

josefbehr commented Aug 23, 2019

ihor-sviziev commented Jan 24, 2020

ihor-sviziev commented Feb 14, 2020

slavvka commented Mar 3, 2020

ihor-sviziev commented Mar 3, 2020

nikunjkotecha commented Mar 4, 2020

lenaorobei left a comment •

edited

Loading

lenaorobei Mar 27, 2020 •

edited

Loading

nikunjkotecha Apr 12, 2020

lenaorobei Apr 13, 2020

nikunjkotecha Apr 13, 2020 •

edited

Loading

lenaorobei Apr 13, 2020

nikunjkotecha Apr 13, 2020

lenaorobei Apr 13, 2020

nikunjkotecha Apr 14, 2020

engcom-Charlie commented Dec 28, 2020

magento-engcom-team commented Dec 29, 2020

engcom-Bravo commented Dec 29, 2020

gabrieldagama commented Jan 7, 2021

gabrieldagama commented Jan 7, 2021

gabrieldagama commented Jan 7, 2021

gabrieldagama commented Jan 7, 2021

gabrieldagama commented Jan 7, 2021

gabrieldagama commented Jan 8, 2021

gabrieldagama commented Jan 11, 2021

m2-assistant bot commented Jan 24, 2021

#21853: Allow mview indexers to use different entity columns. #21857

#21853: Allow mview indexers to use different entity columns. #21857

Conversation

nikunjkotecha commented Mar 20, 2019 • edited by engcom-Charlie Loading

Related Pull Requests

magento-cicd2 commented Mar 20, 2019 • edited Loading

magento-engcom-team commented Mar 20, 2019

orlangur commented Mar 20, 2019

aleron75 commented Jul 8, 2019

josefbehr commented Aug 23, 2019

ihor-sviziev commented Jan 24, 2020

ihor-sviziev commented Feb 14, 2020

slavvka commented Mar 3, 2020

ihor-sviziev commented Mar 3, 2020

nikunjkotecha commented Mar 4, 2020

lenaorobei left a comment • edited Loading

Choose a reason for hiding this comment

lenaorobei Mar 27, 2020 • edited Loading

Choose a reason for hiding this comment

nikunjkotecha Apr 12, 2020

Choose a reason for hiding this comment

lenaorobei Apr 13, 2020

Choose a reason for hiding this comment

nikunjkotecha Apr 13, 2020 • edited Loading

Choose a reason for hiding this comment

lenaorobei Apr 13, 2020

Choose a reason for hiding this comment

nikunjkotecha Apr 13, 2020

Choose a reason for hiding this comment

lenaorobei Apr 13, 2020

Choose a reason for hiding this comment

nikunjkotecha Apr 14, 2020

Choose a reason for hiding this comment

engcom-Charlie commented Dec 28, 2020

magento-engcom-team commented Dec 29, 2020

engcom-Bravo commented Dec 29, 2020

gabrieldagama commented Jan 7, 2021

gabrieldagama commented Jan 7, 2021

gabrieldagama commented Jan 7, 2021

gabrieldagama commented Jan 7, 2021

gabrieldagama commented Jan 7, 2021

gabrieldagama commented Jan 8, 2021

gabrieldagama commented Jan 11, 2021

m2-assistant bot commented Jan 24, 2021

nikunjkotecha commented Mar 20, 2019 •

edited by engcom-Charlie

Loading

magento-cicd2 commented Mar 20, 2019 •

edited

Loading

lenaorobei left a comment •

edited

Loading

lenaorobei Mar 27, 2020 •

edited

Loading

nikunjkotecha Apr 13, 2020 •

edited

Loading