/polls/voters.json gibt doppelte Benutzer über seitenweise Anfragen zurück

Fehler
, ,
1

Wenn der Endpunkt /polls/voters.json mit Paginierung aufgerufen wird, haben wir festgestellt, dass der erste Aufruf wie erwartet 25 Benutzer zurückgibt, die 2. Seite jedoch 26 Benutzer zurückgibt, von denen einer im ersten Aufruf zurückgegeben wurde. Dies ist konsistent, wenn es über 25 Benutzer gibt.

Das Problem liegt hier im Poll-Plugin für Discourse:

Der Offset wird unter der Annahme berechnet, dass PostgreSQL exklusiv mit einer Grenze der Zeile mittels BETWEEN ist, wenn es inklusiv ist.

Die erste Abfrage gibt tatsächlich 25 zurück, da die berechneten Zeilen bei 1 und nicht bei 0 beginnen.

Mein vorgeschlagener Fix ist so einfach wie:

   params = {
      offset: offset + 1,
      offset_plus_limit: offset + limit,
      option_digest: opts[:option_id].presence,
    }

Oder eine elegantere Lösung könnte die Verwendung von PostgreSQL LIMIT und OFFSET sein:

    params = {
      limit: limit,
      offset: offset,
      option_digest: opts[:option_id].presence,
    }

          WHERE pv.poll_id IN (:poll_ids)
                /* where */
        ) v
        ORDER BY digest, CASE WHEN rank = 'Abstain' THEN 1 ELSE CAST(rank AS integer) END, username
        LIMIT :limit OFFSET :offset
      SQL
2 „Gefällt mir“
2

Guter Fang, ich füge hier einen pr-welcome hinzu, falls jemand versuchen möchte, dies zu beheben.

Wir müssen bestätigen, dass wir bestehende Dinge in Umfragen nicht kaputt machen, falls das Frontend fälschlicherweise von falschen Annahmen hier ausgeht.

1 „Gefällt mir“
3

Danke für die schnelle Antwort!
Ich werde versuchen, eine Umgebung einzurichten, ich wollte keinen PR ohne Tests erstellen. Jeder kann mich gerne überholen. Ich bin überhaupt kein Ruby-Entwickler. Ich weiß auch nichts über das Ändern von Spezifikationen oder Tests.

Komischerweise gibt es in der Benutzeroberfläche Code, der die Ergebnisse in ein Set ausgibt, um diesen Fehler zu umgehen.

2 „Gefällt mir“
4

Für alle anderen, die dies sehen: Wir haben eine ähnliche UI-Korrektur für unsere Verlosungssoftware hier vorgenommen.

Ein Hoch auf @lukecyca von VHS für die Identifizierung des Problembereichs.

5

Es gibt auch einen verwandten Bericht über das Laden weiterer Wähler in Umfragen vom Januar.

1 „Gefällt mir“
7

Guter Fang.

Es ist sogar noch lustiger. Ich habe vor einiger Zeit ein paar sehr große PRs eingereicht, um das Frontend zu modernisieren und Ranked Choice hinzuzufügen.

Der riesige Umfang dieser PRs hat definitiv dazu beigetragen, dass ich das übersehen habe, und Backend-Korrekturen waren nicht wirklich im Geltungsbereich (obwohl ich bei der Durchführung dieser PRs tatsächlich viele Probleme gelöst habe, die nicht streng genommen für diese PRs bestimmt waren).

Ich gebe zu, dass ich diesem Frontend-Präzedenzfall der 2018 aufgestellt wurde für Ranked Choice gefolgt bin, ohne wirklich darüber nachzudenken :sweat_smile:. (Was ihr übrigens auch gerade befolgt habt, wenn auch wissentlich :laughing:)

Erstaunlich, wie Workarounds bestehen bleiben können, ohne dass das zugrunde liegende Problem identifiziert wird.

3 „Gefällt mir“
8

Ich dachte, ich versuche mich mal an meinem ersten Beitrag, aber es scheint, als wäre es doch etwas komplizierter als erwartet. :wink:

Ich habe einen Draft-PR hochgeladen: FIX: do not return duplicates from /polls/voters.json by clechasseur · Pull Request #1 · clechasseur/discourse · GitHub

In diesem PR habe ich zuerst Tests hinzugefügt, die das Problem reproduzieren, dann habe ich Robs einfache Lösung angewendet und die Tests haben bestanden.

Ich habe auch die elegantere Lösung ausprobiert (die, die ich bevorzugt hätte), aber obwohl sie doppelte Wähler verhindert, ändert sie auch, welcher Wähler auf welcher Seite zurückgegeben wird, einschließlich der ersten, was potenziell als Breaking Change angesehen werden könnte (abhängig davon, wie das Frontend damit umgeht - ich habe es noch nicht nachgesehen).

Wenn ich jedoch einen Schritt zurücktrete, frage ich mich, was die wahre Bedeutung des limit-Parameters ist, wenn man diesen Endpunkt aufruft - er begrenzt nicht wirklich die Anzahl der insgesamt zurückgegebenen Wähler, sondern nur die Anzahl der Wähler, die für jede Umfrageoption zurückgegeben werden. Diesen Effekt sehen Sie in dem Test, den ich für die Multiple-Choice-Umfrage hinzugefügt habe hier - die erste Seite ist tatsächlich auf 2 Wähler pro Option begrenzt, aber insgesamt werden drei verschiedene Wähler zurückgegeben (verteilt auf die Optionen). Wenn man zur eleganten Lösung wechselt (d. h. LIMIT :limit OFFSET :offset verwendet), wird das limit auf die Gesamtzahl der Stimmen und nicht der Wähler angewendet. Ich bin mir nicht zu 100 % sicher, ob das besser oder intuitiver ist.

Wie auch immer, ich bin neu hier und vielleicht denke ich zu viel nach. Die einfache Lösung entfernt doppelte Wähler und richtet nicht zu viel Schaden an, daher könnte sie der richtige Weg sein. Ich warte auf Input, bevor ich einen PR an das Haupt-Repository sende.

–

Nebenbei bemerkt, glaube ich, dass es in diesem Teil des Codes einen weiteren Fehler gibt. Die Abfrage zum Laden der Wähler ist nach Digest, Rang und Benutzername sortiert - aber beim Sortieren nach Rang wird diese Bedingung verwendet:

CASE WHEN rank = 'Abstain' THEN 1 ELSE CAST(rank AS integer) END

Allerdings entspricht 'Abstain' tatsächlich Rang 0 und nicht 1 - Rang 1 kann auch als '1' zurückgegeben werden. Dies macht die Sortierung potenziell nicht deterministisch über Abfragen hinweg, was bedeutet, dass es je nach Anzahl der Wähler und dem verwendeten limit-Wert möglich sein könnte, Wähler bei paginierten Abfragen tatsächlich zu übersehen. In meinen neuen Tests musste ich die zurückgegebenen Wähler sortieren, um die nicht-deterministische Natur zu umgehen. (Da sie nicht deterministisch ist, gehe ich davon aus, dass sie in einem Test nicht leicht reproduzierbar ist, aber ich kann es versuchen…)

1 „Gefällt mir“
9

Ich habe dem obigen Link zu einem früheren Bericht gefolgt, der einen Link zu einem früheren noch früheren Bericht enthält, der etwas enthält, das ebenfalls auf diese Möglichkeit hindeutet:

Das ist jedoch schon lange her, daher könnte der Code damals völlig anders gewesen sein. (Ich habe versucht, mit Git in der Zeit zurückzugehen, aber die Historie endete 2021, daher gehe ich davon aus, dass der Code irgendwann verschoben wurde.)

10

Nach einiger Experimentiererei konnte ich keinen Test erstellen, der die nicht-deterministische Reihenfolge zu einem Problem mit zurückgegebenen Wählern führte. Ich denke, das liegt an der Art und Weise, wie die Abfrage erstellt wird und wie die Umfragestimmen erzeugt werden, aber ich bin mir nicht zu 100 % sicher.

Ich habe einen PR mit meinem Test und den von mir vorgeschlagenen Korrekturen erstellt: FIX: Vermeiden Sie die Rückgabe doppelter Wähler vom Endpunkt /polls/voters.json von clechasseur · Pull Request #34433 · discourse/discourse

3 „Gefällt mir“
12

Vielen Dank für die Korrektur und Ihre Geduld bei unserer Überprüfung.

Ich habe sie genehmigt und zusammengeführt. :partying_face:

3 „Gefällt mir“
13

Dieses Thema wurde nach 3 Tagen automatisch geschlossen. Neue Antworten sind nicht mehr möglich.