Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make set_transaction_id use the PK index for its lookup #104

Merged
merged 1 commit into from
Jun 27, 2024

Conversation

maltoe
Copy link
Collaborator

@maltoe maltoe commented Jun 27, 2024

This patch fixes a longstanding (yet undiscovered) bug in the set_transaction_id procedure that caused it to not use the provided index on id spanning xact_id. Reason was the use of CURRVAL in the WHERE clause which caused the query to evaluate every row against a new function call, turning the should-be-fast lookup into a seq scan of the transactions table. Consequently, INSERTs scaled linearly with the size of the transactions table, instead of logarithmically.

Huge 🤦

Function diff as its difficult to see otherwise:

--- old 2024-06-27 12:23:40.078326172 +0200
+++ new 2024-06-27 12:23:36.834933301 +0200
@@ -5,9 +5,15 @@
         /* verify that no previous INSERT within current transaction (with same id) */
         IF
           EXISTS(
+            WITH constants AS (
+              SELECT
+                COALESCE(NEW.id, CURRVAL('#{prefix}.transactions_id_seq')) AS id,
+                COALESCE(NEW.xact_id, pg_current_xact_id()) AS xact_id
+            )
             SELECT 1 FROM #{prefix}.transactions
-            WHERE id = COALESCE(NEW.id, CURRVAL('#{prefix}.transactions_id_seq'))
-            AND xact_id = COALESCE(NEW.xact_id, pg_current_xact_id())
+            JOIN constants
+            ON constants.id = transactions.id
+            AND constants.xact_id = transactions.xact_id
           )
         THEN
           NEW.id = COALESCE(NEW.id, CURRVAL('#{prefix}.transactions_id_seq'));

CHANGELOG.md Show resolved Hide resolved
This patch fixes a longstanding (yet undiscovered) bug in the `set_transaction_id`
procedure that caused it to not use the provided index on `id` spanning `xact_id`.
Reason was the use of `CURRVAL` in the `WHERE` clause which caused the query to
evaluate every row against a new function call, turning the should-be-fast lookup
into a seq scan of the `transactions` table. Consequently, `INSERT`s scaled linearly
with the size of the `transactions` table, instead of logarithmically.

Huge 🤦
@maltoe maltoe enabled auto-merge (squash) June 27, 2024 11:49
@maltoe maltoe merged commit c4a579d into main Jun 27, 2024
7 checks passed
@maltoe maltoe deleted the fix-index-use-in-exists branch June 27, 2024 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants