Field "Comment Statistics: Updated/commented date" is build correctly:
GREATEST(node_field_data.changed, comment_entity_statistics.last_comment_timestamp)

Filter Criteria seems to use the old/wrong row "node.changed":
GREATEST(node.changed, comment_entity_statistics.last_comment_timestamp)

So this SQL-Error arises:
SQLSTATE[42S22]: Column not found: 1054 Unknown column 'node.changed' in 'where clause': SELECT

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

kle created an issue. See original summary.

Lendude’s picture

Component: views.module » comment.module
Status: Active » Needs review
Issue tags: +VDC, +Needs tests
FileSize
756 bytes

@kle thanks for the bug report. And yes this is broken.

Here is a fix, this still needs tests.

Moving to the right component, the code for this lives in the comment module.

dawehner’s picture

I'm wondering whether we can search the codebase for other instances of it, and as such fix the failures earlier than needed.

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.

jonathanshaw’s picture

This issue appears to be a subset of #2086125: Last read comment field/filter/argument uses still the node.changed instead of node_field_data.changed column which has a more ambitious patch. Any chance you could review that?

andypost’s picture

Cellar Door’s picture

This patch works wonderfully and isn't a large distraction from the larger work. Any harm in opening this back up and getting this committed while waiting for the larger rewrite to occur?

I'd mark RTBC if it were proper to do so, as I've tested and it works as written.

Cellar Door’s picture

After playing with this patch more, this solves the issue with the query but the config still has an issue where it throws the following exception: InvalidArgumentException: The configuration property display.[display_name].display_options.filters.last_updated.value.min doesn't exist.

I believe the Views Schema also needs to be updated to allow the config to be stored properly.

jonathanshaw’s picture

This needs written tests, so can't be RTBC :(

Cellar Door’s picture

FileSize
2.11 KB

Here's an updated patch with the config fixed and to dawehner's point I fixed the sort which was also using the wrong column

There was talk in the other thread of pulling in the tests from there to cover this, how do we want to handle that? I'd say let's write minimal test cases just to cover these simple updates and get this committed while the rest of the re-write occurs, which may take a while.

jonathanshaw’s picture

Agreed, the tests in the other patch are probably more ambitious than this needs. I'm not sure what the easiest way to test a views filter and sort is.