Problem/Motivation

When a Views table display has the field “Views result counter” (row counter) and that column is made sortable (click-sort in table header), clicking the header triggers an SQL query with ORDER BY "unknown", which fails with SQLSTATE[42S22]: Column not found: 1054 Unknown column 'unknown' in 'order clause'.

Steps to reproduce

  1. Go to /admin/structure/views and edit the core view People (display id: user_admin_people) or create a new View listing users.
  2. Set the display format to Table.
  3. Add the field Global: Views result counter (or the Views “result counter” field).
  4. In the Table settings, enable the column as sortable (click-sort).
  5. Visit the view page (e.g. /admin/people) and click the table header for that counter column.

Proposed resolution

Update the Views data definition for the built-in “counter” field to explicitly mark it as non-sortable by adding 'click sortable' => FALSE

Additionally, introduce a post-update hook (views_post_update_counter_field_not_sortable()) so that the updated Views data definition is picked up on existing sites.

Expected result

Either:

  • The UI should not allow enabling click-sort for the result counter field, or
  • Clicking the header should not add an invalid ORDER BY and should fail gracefully / be ignored.

Actual result

A query is generated with an invalid ORDER BY and the page errors:

SQLSTATE[42S22]: Column not found: 1054 Unknown column 'unknown' in 'order clause'
... ORDER BY "unknown" ASC ...

Issue fork drupal-3574506

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mike-michal created an issue. See original summary.

mike-michal’s picture

Attached patch sets views.counter field as 'click sortable' = FALSE in views_views_data().
This prevents enabling click-sort for Views result counter and avoids ORDER BY "unknown" SQL errors.

Tested:
- Core: 11.2.10
- Views: /admin/people (People[user_admin_people]) table + Views result counter sortable previously caused SQLSTATE[42S22].
- After patch: counter is not click-sortable; error no longer occurs.

quietone’s picture

Version: 11.2.x-dev » main

Hi, Issues for Drupal core should be targeted to the 'main' branch, our primary development branch. Changes are made on the main branch first, and are then back ported as needed according to the Core change policies. The version the problem was discovered on should be stated in the issue summary Problem/Motivation section. Thanks.

quietone’s picture

Issue tags: -bug, -views

mike-michal’s picture

Hi @quietone

Thanks for the guidance.

The patch has been applied to `main` and an MR has been created/updated accordingly.

Please let me know if there is something else I can do.

Kind regards,
Michał, Mike Kozłowski.

lendude’s picture

Priority: Normal » Major
Status: Active » Reviewed & tested by the community

Actually major since it throws an exception.

This fixes it, nice.

No point in trying to clean up existing config, the format settings are never updated with any changes made in the UI anyway so they are a mess anyway. Tested that the click sorting disappears from the View even if the config still says it is click sortable, so this fixes existing Views too.

sivaji_ganesh_jojodae’s picture

The patch affects the Table sort settings of View result counter.

However the View page is broken with the following SQL error once View result counter is added.

SQLSTATE[42S22]: Column not found: 1054 Unknown column 'unknown' in 'ORDER BY': SELECT "users_field_data"."uid" AS "uid" FROM "users_field_data" "users_field_data" WHERE ("users_field_data"."default_langcode" = :db_condition_placeholder_0) AND ("users_field_data"."uid" != :db_condition_placeholder_1) ORDER BY "unknown" ASC LIMIT 50 OFFSET 0; Array ( [:db_condition_placeholder_0] => 1 [:db_condition_placeholder_1] => 0 )

sivaji_ganesh_jojodae’s picture

I tested this on a fresh installation, and the patch works as expected.

Please see the screenshots below for reference.

Views table settings

Views settings page

Admin people listing with View result counter

Admin people

quietone’s picture

Title: Views: Sorting by Views result counter causes ORDER BY unknown » Sorting by Views result counter causes ORDER BY unknown
alexpott’s picture

Status: Reviewed & tested by the community » Needs work

I agree that we should fix this first as it is an easy way to cause an SQL error. But I also think we should file a follow-up to fix existing views as the view configuration is only fixed if you return to the table settings and press save.

Also in order for the view to be fixed we need to rebuild views data so we should be adding an empty post update function so we can release this fix in a bug release.

@lendude I don't really understand what you mean by

No point in trying to clean up existing config, the format settings are never updated with any changes made in the UI anyway so they are a mess anyway.

The format settings were definitely updated when I tested going back to the table settings and pressing update after applying this MR.

alexpott’s picture

Issue tags: +Needs followup

Re #13 - I think the only thing we need to do here is to add an empty post update function to views.post_update.php

sivaji_ganesh_jojodae’s picture

Status: Needs work » Needs review

Added an empty post-update hook to views.post_update.php.

mike-michal’s picture

Wow. One line of code and such a long journey? ;)

smustgrave’s picture

This one still appears to need a follow up so leaving in review.

smustgrave’s picture

Status: Needs review » Needs work

This one needs a reroll now, also summary could use an update as it's missing the required proposed solution section.

mike-michal’s picture

Reroll?

sivaji_ganesh_jojodae’s picture

Issue summary: View changes
Status: Needs work » Needs review

Updated the Issue summary and fixed the merge conflict.

mike-michal’s picture

Sorry guys for my confusion. I am new to this process.
I thought that this simple and obvious, imho, change is a great opportunity to understand how to contribute. But if this one, simple line of code takes so long way I can't imagine how it goes with something a little bit more complex ;)

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs followup +Bug Smash Initiative, +Needs Review Queue Initiative

Opened #3588188: Fix existing views with click sortable per #3574506 so removing follow up tag.

@mike-michal this issue was waiting on a follow up ticket to be created per #14,.

Feedback has been address with the empty hook.

mike-michal’s picture

@smustgrave it would be nice if someone could explain me in details what is going on here. Maybe I could try to contribute more frequently. Crossed fingers for happy end of this one ;) Thanks a lot for all your efforts. Mike.

alexpott’s picture

Version: main » 11.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed 71f4885 and pushed to main. Thanks!
Committed 76ce09f and pushed to 11.x. Thanks!
Committed 326940f and pushed to 11.3.x. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • alexpott committed 76ce09fa on 11.x
    fix: #3574506 Sorting by Views result counter causes ORDER BY unknown...

  • alexpott committed 71f4885c on main
    fix: #3574506 Sorting by Views result counter causes ORDER BY unknown...
alexpott’s picture

@mike-michal the thing that makes a change like this hard is that there are existing views configs that need fixing - that's been descoped for a follow-up in #3588188: Fix existing views with click sortable per #3574506. So the fix is simple but the updating of existing views is not that easy and should only happen in the minor update whereas this can go in a patch release.

  • alexpott committed 1a1999dd on 11.3.x
    fix: #3574506 Sorting by Views result counter causes ORDER BY unknown...

Status: Fixed » Closed (fixed)

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