I'm working a version of the Views Unionize module for D7, with a different UI. I've hit a show stopper issue here #1786828: Views join condition placeholders incrementing incorrectly that needs a small change in the views_join::build_join method. The net effect is the same :views_join_condition(s) placeholders are being re-used by each of the UNIONed Views' queries.

The issue I'm trying to solve is the build_join method does not keep a running log of the SelectQuery::$nextplaceholder property across multiple UNIONed queries as the placeholders are assigned when the build_join method is called as opposed to writing them when the entire SelectQuery object is passed off to DBTNG (as with conditions).

CommentFileSizeAuthor
#13 maintain_views_join-1787072-13.patch1.25 KBbigjim
#5 views_join_condition_increment_5.patch1.11 KBbigjim
#4 views_join_condition_increment_2.patch1.33 KBbigjim
views_join_condition_increment.patch1.33 KBbigjim
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

What about using views_plugin_query_default::placeholder() instead?

bigjim’s picture

bigjim CreditAttribution: bigjim commented

Ha, learn something new everyday. So making that change throws an error from DBTNG:

SQLSTATE[HY093]: Invalid parameter number: number of bound variables does not match number of tokens

Same error called statically or as $view_query->placeholder(). Any ideas why there would be a mismatch? The SQL looks okay. I'll dig in a little more.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Needs work 
+++ b/includes/handlers.incundefined
@@ -1575,6 +1576,7 @@ class views_join {
               $placeholder_i = ':views_join_condition_' . $select_query->nextPlaceholder();
+              $placeholder_i = ':views_join_condition_' . $join_count;

This probably causes the problem ... in general i would really really really vote for a dbtng solution.

+++ b/includes/handlers.incundefined
@@ -1585,9 +1587,11 @@ class views_join {
             $placeholder = ':views_join_condition_' . $select_query->nextPlaceholder();
+            $placeholder_i = ':views_join_condition_' . $join_count;

This change doesn't look right

bigjim’s picture

bigjim CreditAttribution: bigjim commented
FileSize
views_join_condition_increment_2.patch1.33 KB

You're right on the typo, updated patched attached here (based on originally proposed method).

though to be clear:

$placeholder_i = ':views_join_condition_' . $join_count;

this works fine

$placeholder_i = ':views_join_condition_' . $views_query->placeholder();

and

$placeholder_i = ':views_join_condition_' . views_plugin_query_default::placeholder();

Both throw the "SQLSTATE[HY093]" error

I'm 100% with you on the DBTNG fix, though the way DBTNG handles join conditions makes that quite difficult. The most significant limitation being that you have to assign the join condition placeholder key before you hand the SelectQuery object off to DBTNG.

bigjim’s picture

bigjim CreditAttribution: bigjim commented
Status: Needs work » Needs review
FileSize
views_join_condition_increment_5.patch1.11 KB

Okay patch using the placeholder() function attached here.

bigjim’s picture

bigjim CreditAttribution: bigjim commented
Title: Increment Views Join condition with static variable » Maintain Views Join condition counter in Views object
Status: Needs review » Reviewed & tested by the community

We've been using this in production for a couple months no with no issues. While I totally agree with the desire to get this fixed in DBTNG I don't see tat happening anytime soon and it's doubtful any such fix would get back ported to D7 in a reasonably timely manner.

Moving to RTBC, changed

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

Sure i totally see why you mark this issue as RTBC, and i think this is RBC, but you should not do this
with your own patches.

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Status: Reviewed & tested by the community » Needs review
Darren Oh’s picture

Darren Oh
Primary language English
Location Lakeland, Florida
CreditAttribution: Darren Oh commented

5: views_join_condition_increment_5.patch queued for re-testing.

Darren Oh’s picture

Darren Oh
Primary language English
Location Lakeland, Florida
CreditAttribution: Darren Oh commented
Category: Feature request » Bug report
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

This was reviewed a long time ago and still passes testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Project: Views (for Drupal 7) » Drupal core
Version: 7.x-3.x-dev » 8.x-dev
Component: Views Data » views.module
Status: Reviewed & tested by the community » Patch (to be ported)

Committed and pushed to 7.x

This needs something similar in Drupal 8.

bigjim’s picture

bigjim CreditAttribution: bigjim commented
Assigned: Unassigned » bigjim

I can work on this one

bigjim’s picture

bigjim CreditAttribution: bigjim commented
FileSize
maintain_views_join-1787072-13.patch1.25 KB

Drupal 8 patch here for test bot.

bigjim’s picture

bigjim CreditAttribution: bigjim commented
Status: Patch (to be ported) » Needs review

forgot to mark needs review

Status: Needs review » Needs work

The last submitted patch, 13: maintain_views_join-1787072-13.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Issue tags: +Needs tests

Thanks for posting the patch!

I guess we need some dedicated test coverage here as well

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford at OpenConcept Consulting Inc. commented
Version: 8.0.x-dev » 8.1.x-dev
Assigned: bigjim » Unassigned
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Version: 8.1.x-dev » 8.0.x-dev

... its a bug,

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Darren Oh’s picture

Darren Oh
Primary language English
Location Lakeland, Florida
CreditAttribution: Darren Oh for Estée Lauder Companies commented
Issue tags: +fldc19

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev
Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude as a volunteer and at iO commented
Project: Drupal core » Views (for Drupal 7)
Status: Needs work » Fixed

I have no idea if this is still relevant, it feels outdated and more like a feature request now than a bug if it is. I understand this got changed in D7, but ¯\_(ツ)_/¯

If this is needed for D8 please feel free to open this back up and provide arguments why this needs to be changed in core and can't handled in contrib.

For now I'm moving this back to D7 and marking it fixed.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude as a volunteer and at iO commented
Version: 9.3.x-dev » 8.x-3.x-dev
Component: views.module » Miscellaneous
Issue tags: +Bug Smash Initiative

Forgot to tag

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude as a volunteer and at iO commented
Version: 8.x-3.x-dev » 7.x-3.x-dev

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.