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

BUG: can't call parameterized distributed procedures outside an explicit transaction in Citus 12.1.3 (but not 12.1.0?) #7597

Open
DS-AdamMilazzo opened this issue May 10, 2024 · 3 comments

Comments

@DS-AdamMilazzo
Copy link

DS-AdamMilazzo commented May 10, 2024

In Citus 12.1.3, when calling a distributed procedure in a batch of SQL statements, the CALL statement appears to execute in a different transaction from the rest of the statements in the batch. Also, it is generally impossible to call distributed procedures with batch parameters as arguments to the procedure. This behavior is not observed in Citus 12.1.0.

Repro steps

First, perform this setup:

CREATE TABLE t (p INT NOT NULL, i INT NOT NULL, PRIMARY KEY (p, i));
SELECT create_distributed_table('t', 'p');
CREATE PROCEDURE f(_p INT, _i INT) LANGUAGE plpgsql AS
    $$ BEGIN SELECT i / 0 FROM t WHERE p = _p AND i = _i; END; $$;
SELECT create_distributed_function('f(INT,INT)', distribution_arg_name := '_p', colocate_with := 't');

Second, call the procedure in a parameterized batch of SQL statements:

INSERT INTO t (p, i) VALUES (@0, @1);
CALL f(1, @1);

In this batch, I used parameters @0 = 1 and @1 = 2, but the specific values don't matter. The batch was sent with the Npgsql library, but any library that allows sending parameterized batches should be able to reproduce this bug. (Don't try putting the statements into a SQL procedure and calling it. That's not likely to reproduce the problem. Use a client library to send the parameterized batch.) Example Npgsql code:

NpgsqlCommand cmd = connection.CreateCommand();
cmd.CommandText = "INSERT INTO t (p, i) VALUES (@0, @1); CALL f(1, @1);";
cmd.Parameters.Add(new NpgsqlParameter("@0", 1));
cmd.Parameters.Add(new NpgsqlParameter("@1", 2));
cmd.ExecuteNonQuery();

Expected behavior:

  1. A tuple should be inserted into table t.
  2. Procedure f(1, 2) should be called, and fail with a division by zero error.
  3. The transaction should roll back, removing the inserted tuple.

Actual behavior:

  1. A tuple is inserted into table t.
  2. Procedure f is not called. Instead, an error occurs: "42P02: there is no parameter $1"
  3. The INSERT statement is still committed (i.e. the inserted tuple is not removed).

Additional Comments

  • The problem happens in Citus 12.1.3 but not 12.1.0, I believe.
  • You don't even need the INSERT statement to trigger the error. Any CALL to a distributed procedure with batch parameters (outside of an explicit transaction) will fail! E.g. if the batch is simply CALL f(1, @0); it will fail.
  • The problem only happens if it's a distributed function. If you don't call create_distributed_function, the problem does not occur.
  • The problem does not occur when an explicit transaction is used, e.g. BEGIN; INSERT ...; CALL ...; COMMIT;
  • The problem does not occur if the CALL does not have batch parameters, e.g. CALL f(1, 2);
  • It doesn't matter whether the function f is supposed to succeed or fail. It never gets called in the first place.
  • I previously reported Citus segfaults and crashes Postgres when calling distributed procedure with a parameterized distribution argument #7242 which was also related to parameterized distributed functions called outside explicit transactions. It may be related.
  • This is a fairly serious problem that I hope gets fixed soon...
@DS-AdamMilazzo DS-AdamMilazzo changed the title BUG: transaction lost calling a distributed procedure in Citus 12.1.3 (but 12.1.0 works) BUG: transaction lost calling a distributed procedure in Citus 12.1.3 (but not 12.1.0?) May 10, 2024
@DS-AdamMilazzo DS-AdamMilazzo changed the title BUG: transaction lost calling a distributed procedure in Citus 12.1.3 (but not 12.1.0?) BUG: can't call parameterized distributed procedures outside an explicit transaction in Citus 12.1.3 (but not 12.1.0?) May 10, 2024
@emelsimsek
Copy link
Contributor

I have tried to reproduce this using the below python code. However on both 12.1.0 and 12.1.3, procedure f is called, it throws division by 0 exception and nothing is inserted to the table t. Is anything missing in this code?

    cur = conn.cursor()

    sql = "CREATE TABLE t (p INT NOT NULL, i INT NOT NULL, PRIMARY KEY (p, i));"
    cur.execute(sql);

    sql = "SELECT create_distributed_table('t', 'p');"
    cur.execute(sql);

    conn.commit();

    sql = """ CREATE PROCEDURE f(_p INT, _i INT) LANGUAGE plpgsql AS
    $$ BEGIN SELECT i / 0 FROM t WHERE p = _p AND i = _i; END; $$;"""

    cur.execute(sql);

    conn.commit();

    sql = "SELECT create_distributed_function('f(INT,INT)', distribution_arg_name := '_p', colocate_with := 't');"
    cur.execute(sql);

    conn.commit();

    sql = "INSERT INTO t (p, i) VALUES (%s, %s); CALL f(1, %s);"

    cur.execute(sql,(1,2,2))
    conn.commit();

@DS-AdamMilazzo
Copy link
Author

DS-AdamMilazzo commented May 10, 2024

I am not familiar with the python client, but I think there are two problems with that code.

  1. I think that by default, the python client wraps your commands in a transaction. You should switch it to autocommit mode. As mentioned in the post, the problem doesn't occur inside a transaction.
  2. Ideally you would use two parameters, as in INSERT INTO t (p, i) VALUES (@0, @1); CALL f(1, @1); rather than three. Using three may result in a different behavior, though I'm not sure.

@DS-AdamMilazzo
Copy link
Author

Any luck reproducing this?

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

No branches or pull requests

2 participants