Skip to content

Commit

Permalink
[#25577] YSQL: Support unnamed prepared statements bind+execute
Browse files Browse the repository at this point in the history
Summary:
**Issue**

Some databases drivers by libpq (C driver) have functionality to prepare an unnamed prepare statement by sending the parse packet only with statement name as empty string. The execution can be delayed. An example mentioning this:

```
PGresult *res = PQprepare(conn, "", "SELECT * FROM my_table WHERE id = $1", 1, NULL); // only prepare the query and send parse message only

const char *paramValues[1] = {"1"};
res = PQexecPrepared(conn, "", 1, paramValues, NULL, NULL, 0); // bind + execute
```

With connection manager involved, the server would be detached after `PQprepare` and during detach connection manager issues internal queries like `RESET` and `RESET ALL`. Postgres backend interpret them as unnamed statements and drops the already existing prepared statement plan for `SELECT * FROM my_table WHERE id = $1`.

Due to this, when `PQexecPrepared` is done to execute the query, postgres can't find unnamed prepared statement as it was dropped in detach step.

```
Execution failed: ERROR:  unnamed prepared statement does not exist
```

**Fix**

  # We need some way to identify what was the last unnamed prepared statement is issue by client to be prepared via parse message. We can then use that to re-send the parse message it if required.

  # Whether it is required or not would depend upon if ther server which is routed to perform the transaction have that unnamed prepared statement stored for that client.

For point 1, this diff adds these fields in od_client structure.

```
	/*
	 * This stores the last pasre message issued by this logical connection
	 * corresponding to the unnamed prepared statement.
	*/
	char* yb_unnamed_prep_stmt;
	int yb_unnamed_prep_stmt_size;
```

Whenever a parse message corresponding to unnamed prepared statement comes, just store the entire message and it's size. This will then be re-sent when processing the bind message if required.

For point 2, this diff adds these field in od_server structure.

```
	/*
	 * The ID of the logical client that has sent parse message to the backend
	 * corresponding to this server object for an unnamed prepared statement.
	 * This would be cleared after server gets detached.
	*/
	od_id_t yb_unnamed_prep_stmt_client_id;
```

During processing the bind packet, we will check if `client->id` is same as `server->yb_unnamed_prep_stmt_client_id`.
If true, then no need to resend the parse message. Otherwise it is required.
Jira: DB-14831

Test Plan: Jenkins: enable connection manager, all tests

Reviewers: skumar, stiwary, vpatibandla, rbarigidad

Reviewed By: rbarigidad

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D41078
  • Loading branch information
Devansh Saxena committed Jan 13, 2025
1 parent 2baf0ea commit 54cb8cb
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 0 deletions.
8 changes: 8 additions & 0 deletions src/odyssey/sources/client.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,12 @@ struct od_client {
* after successful authentication via auth backend.
*/
int64_t logical_client_version;

/*
* This stores the last unnamed prepared statement, otherwise NULL.
*/
char* yb_unnamed_prep_stmt;
int yb_unnamed_prep_stmt_size;
};

static const size_t OD_CLIENT_DEFAULT_HASHMAP_SZ = 420;
Expand Down Expand Up @@ -150,6 +156,8 @@ static inline void od_client_init(od_client_t *client)
client->yb_is_authenticating = false;
client->yb_external_client = NULL;
client->logical_client_version = 0;
client->yb_unnamed_prep_stmt = NULL;
client->yb_unnamed_prep_stmt_size = 0;
}

static inline od_client_t *od_client_allocate(void)
Expand Down
46 changes: 46 additions & 0 deletions src/odyssey/sources/frontend.c
Original file line number Diff line number Diff line change
Expand Up @@ -1336,6 +1336,22 @@ static od_frontend_status_t od_frontend_remote_client(od_relay_t *relay,
if (desc.operator_name[0] == '\0') {
/* no need for odyssey to track unnamed prepared statements */
prev_named_prep_stmt = 0;

if (client->yb_unnamed_prep_stmt) {
free(client->yb_unnamed_prep_stmt);
client->yb_unnamed_prep_stmt = NULL;
}

client->yb_unnamed_prep_stmt = malloc(size);
if (client->yb_unnamed_prep_stmt == NULL) {
return OD_EOOM;
}

memcpy(client->yb_unnamed_prep_stmt, data, size);
client->yb_unnamed_prep_stmt_size = size;

server->yb_unnamed_prep_stmt_client_id = client->id;

break;
}

Expand Down Expand Up @@ -1519,6 +1535,36 @@ static od_frontend_status_t od_frontend_remote_client(od_relay_t *relay,

/* unnamed prepared statement, ignore processing of the packet */
if (operator_name[0] == '\0') {
assert(client->yb_unnamed_prep_stmt);

if (od_id_cmp(&client->id, &server->yb_unnamed_prep_stmt_client_id)) {
od_debug(&instance->logger, "rewrite parse", client, server,
"server already has the cache plan from previous parse");
break;
}

machine_msg_t *msg_new = NULL;
msg_new = kiwi_fe_copy_msg(msg_new, client->yb_unnamed_prep_stmt,
client->yb_unnamed_prep_stmt_size);

if (instance->config.log_query ||
route->rule->log_query) {
od_frontend_log_parse(
instance, client,
"rewrite parse",
machine_msg_data(msg_new),
machine_msg_size(msg_new));
}

od_stat_parse(&route->stats);
rc = od_write(&server->io, msg_new);
if (rc == -1) {
od_error(&instance->logger,
"rewrite parse", NULL, server,
"write error: %s",
od_io_error(&server->io));
return OD_ESERVER_WRITE;
}
break;
}

Expand Down
9 changes: 9 additions & 0 deletions src/odyssey/sources/router.c
Original file line number Diff line number Diff line change
Expand Up @@ -998,6 +998,15 @@ void od_router_detach(od_router_t *router, od_client_t *client)

client->server = NULL;
server->client = NULL;
/*
* When the server detaches after completing a transaction,
* some queries are issued during the reset phase, which causes the
* unnamed prepared statement plan cache to be dropped.
* As a result, it is necessary to remove the saved state
* on Connection Manager.
*/
memset(&server->yb_unnamed_prep_stmt_client_id, 0,
sizeof(server->yb_unnamed_prep_stmt_client_id));
/*
* Drop the server connection if:
* a. Server gets OFFLINE.
Expand Down
9 changes: 9 additions & 0 deletions src/odyssey/sources/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,13 @@ struct od_server {
* eventually cleaned by cron job
*/
bool marked_for_close;

/*
* The ID of the logical client that has sent parse message to the backend
* corresponding to this server object for an unnamed prepared statement.
* This would be cleared after server gets detached.
*/
od_id_t yb_unnamed_prep_stmt_client_id;
};

static const size_t OD_SERVER_DEFAULT_HASHMAP_SZ = 420;
Expand Down Expand Up @@ -115,6 +122,8 @@ static inline void od_server_init(od_server_t *server, int reserve_prep_stmts)
od_relay_init(&server->relay, &server->io);
od_list_init(&server->link);
memset(&server->id, 0, sizeof(server->id));
memset(&server->yb_unnamed_prep_stmt_client_id, 0,
sizeof(server->yb_unnamed_prep_stmt_client_id));

if (reserve_prep_stmts) {
server->prep_stmts =
Expand Down

0 comments on commit 54cb8cb

Please sign in to comment.