Skip to content

Commit eabb1cb

Browse files
authored
Dont rewrite anon prepared statements by default (#556)
* Dont rewrite anon prepared statements by default * Fix test
1 parent 101d0ce commit eabb1cb

10 files changed

Lines changed: 136 additions & 20 deletions

File tree

integration/go/go_pq/go_pq_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,13 @@ func TestAuthenticationWithPassthrough(t *testing.T) {
8181
}
8282

8383
func TestPqCrud(t *testing.T) {
84+
adminConn, err := sql.Open("postgres", "postgres://admin:pgdog@127.0.0.1:6432/admin?sslmode=disable")
85+
assert.Nil(t, err)
86+
defer adminConn.Close()
87+
88+
_, err = adminConn.Exec("SET prepared_statements TO 'extended_anonymous'")
89+
assert.Nil(t, err)
90+
8491
conns := PqConnections()
8592

8693
for _, conn := range conns {

integration/pgdog.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ read_write_strategy = "aggressive"
1111
openmetrics_port = 9090
1212
openmetrics_namespace = "pgdog_"
1313
prepared_statements_limit = 500
14+
prepared_statements = "extended"
1415
expanded_explain = true
1516
# dns_ttl = 15_000
1617
query_cache_limit = 500

pgdog/src/admin/set.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,10 @@ impl Command for Set {
9191
.close_unused(config.config.general.prepared_statements_limit);
9292
}
9393

94+
"prepared_statements" => {
95+
config.config.general.prepared_statements = Self::from_json(&self.value)?;
96+
}
97+
9498
"cross_shard_disabled" => {
9599
config.config.general.cross_shard_disabled = Self::from_json(&self.value)?;
96100
}

pgdog/src/config/core.rs

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ use std::fs::read_to_string;
44
use std::path::PathBuf;
55
use tracing::{info, warn};
66

7+
use crate::config::PreparedStatements;
8+
79
use super::database::Database;
810
use super::error::Error;
911
use super::general::General;
@@ -82,12 +84,12 @@ impl ConfigAndUsers {
8284
}
8385

8486
/// Prepared statements are enabled.
85-
pub fn prepared_statements(&self) -> bool {
87+
pub fn prepared_statements(&self) -> PreparedStatements {
8688
// Disable prepared statements automatically in session mode
8789
if self.config.general.pooler_mode == PoolerMode::Session {
88-
false
90+
PreparedStatements::Disabled
8991
} else {
90-
self.config.general.prepared_statements.enabled()
92+
self.config.general.prepared_statements
9193
}
9294
}
9395

@@ -378,31 +380,34 @@ column = "tenant_id"
378380
// Test transaction mode (default) - prepared statements should be enabled
379381
config.config.general.pooler_mode = PoolerMode::Transaction;
380382
config.config.general.prepared_statements = PreparedStatements::Extended;
381-
assert!(
383+
assert_eq!(
382384
config.prepared_statements(),
385+
PreparedStatements::Extended,
383386
"Prepared statements should be enabled in transaction mode"
384387
);
385388

386389
// Test session mode - prepared statements should be disabled
387390
config.config.general.pooler_mode = PoolerMode::Session;
388391
config.config.general.prepared_statements = PreparedStatements::Extended;
389-
assert!(
390-
!config.prepared_statements(),
392+
assert_eq!(
393+
config.prepared_statements(),
394+
PreparedStatements::Disabled,
391395
"Prepared statements should be disabled in session mode"
392396
);
393397

394398
// Test session mode with full prepared statements - should still be disabled
395399
config.config.general.pooler_mode = PoolerMode::Session;
396400
config.config.general.prepared_statements = PreparedStatements::Full;
397-
assert!(
398-
!config.prepared_statements(),
401+
assert_eq!(
402+
config.prepared_statements(),
403+
PreparedStatements::Disabled,
399404
"Prepared statements should be disabled in session mode even when set to Full"
400405
);
401406

402407
// Test transaction mode with disabled prepared statements - should remain disabled
403408
config.config.general.pooler_mode = PoolerMode::Transaction;
404409
config.config.general.prepared_statements = PreparedStatements::Disabled;
405-
assert!(!config.prepared_statements(), "Prepared statements should remain disabled when explicitly set to Disabled in transaction mode");
410+
assert_eq!(config.prepared_statements(), PreparedStatements::Disabled, "Prepared statements should remain disabled when explicitly set to Disabled in transaction mode");
406411
}
407412

408413
#[test]

pgdog/src/config/pooling.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
use serde::{Deserialize, Serialize};
22
use std::str::FromStr;
33

4-
#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq)]
4+
#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Copy)]
55
#[serde(rename_all = "snake_case")]
66
pub enum PreparedStatements {
77
Disabled,
88
#[default]
99
Extended,
10+
ExtendedAnonymous,
1011
Full,
1112
}
1213

@@ -18,6 +19,10 @@ impl PreparedStatements {
1819
pub fn enabled(&self) -> bool {
1920
!matches!(self, PreparedStatements::Disabled)
2021
}
22+
23+
pub fn rewrite_anonymous(&self) -> bool {
24+
matches!(self, PreparedStatements::ExtendedAnonymous)
25+
}
2126
}
2227

2328
impl FromStr for PreparedStatements {
@@ -27,6 +32,7 @@ impl FromStr for PreparedStatements {
2732
match s.to_lowercase().as_str() {
2833
"disabled" => Ok(Self::Disabled),
2934
"extended" => Ok(Self::Extended),
35+
"extended_anonymous" => Ok(Self::ExtendedAnonymous),
3036
"full" => Ok(Self::Full),
3137
_ => Err(format!("Invalid prepared statements mode: {}", s)),
3238
}

pgdog/src/frontend/client/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,7 @@ impl Client {
482482
// Check config once per request.
483483
let config = config::config();
484484
// Configure prepared statements cache.
485-
self.prepared_statements.enabled = config.prepared_statements();
485+
self.prepared_statements.level = config.prepared_statements();
486486
self.prepared_statements.capacity = config.config.general.prepared_statements_limit;
487487
self.timeouts = Timeouts::from_config(&config.config.general);
488488

pgdog/src/frontend/client/query_engine/prepared_statements.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use crate::config::PreparedStatements;
2+
13
use super::*;
24

35
impl QueryEngine {
@@ -7,8 +9,15 @@ impl QueryEngine {
79
context: &mut QueryEngineContext<'_>,
810
) -> Result<(), Error> {
911
for message in context.client_request.iter_mut() {
10-
if message.extended() && context.prepared_statements.enabled {
11-
context.prepared_statements.maybe_rewrite(message)?;
12+
if message.extended() {
13+
let level = context.prepared_statements.level;
14+
match (level, message.anonymous()) {
15+
(PreparedStatements::ExtendedAnonymous, _)
16+
| (PreparedStatements::Extended, false) => {
17+
context.prepared_statements.maybe_rewrite(message)?
18+
}
19+
_ => (),
20+
}
1221
}
1322
}
1423
Ok(())

pgdog/src/frontend/client/test/mod.rs

Lines changed: 76 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@ use bytes::{Buf, BufMut, BytesMut};
1010

1111
use crate::{
1212
backend::databases::databases,
13-
config::{config, load_test, load_test_replicas, set, Role},
13+
config::{config, load_test, load_test_replicas, set, PreparedStatements, Role},
1414
frontend::{
1515
client::{BufferEvent, QueryEngine},
16-
Client,
16+
prepared_statements, Client,
1717
},
1818
net::{
1919
bind::Parameter, Bind, Close, CommandComplete, DataRow, Describe, ErrorResponse, Execute,
@@ -307,8 +307,8 @@ async fn test_client_with_replicas() {
307307
client.run().await.unwrap();
308308
});
309309
let buf = buffer!(
310-
{ Parse::new_anonymous("SELECT * FROM test_client_with_replicas") },
311-
{ Bind::new_statement("") },
310+
{ Parse::named("test", "SELECT * FROM test_client_with_replicas") },
311+
{ Bind::new_statement("test") },
312312
{ Execute::new() },
313313
{ Sync }
314314
);
@@ -769,3 +769,75 @@ async fn test_client_login_timeout_does_not_affect_queries() {
769769
conn.write_all(&buffer!({ Terminate })).await.unwrap();
770770
handle.await.unwrap();
771771
}
772+
773+
#[tokio::test]
774+
async fn test_anon_prepared_statements() {
775+
crate::logger();
776+
load_test();
777+
778+
let (mut conn, mut client, _) = new_client!(false);
779+
780+
let mut c = (*config()).clone();
781+
c.config.general.prepared_statements = PreparedStatements::ExtendedAnonymous;
782+
set(c).unwrap();
783+
784+
let handle = tokio::spawn(async move {
785+
client.run().await.unwrap();
786+
});
787+
788+
conn.write_all(&buffer!(
789+
{ Parse::new_anonymous("SELECT 1") },
790+
{ Bind::new_params("", &[]) },
791+
{ Execute::new() },
792+
{ Sync }
793+
))
794+
.await
795+
.unwrap();
796+
797+
let _ = read!(conn, ['1', '2', 'D', 'C', 'Z']);
798+
799+
{
800+
let cache = prepared_statements::PreparedStatements::global();
801+
let read = cache.read();
802+
assert!(!read.is_empty());
803+
}
804+
805+
conn.write_all(&buffer!({ Terminate })).await.unwrap();
806+
handle.await.unwrap();
807+
}
808+
809+
#[tokio::test]
810+
async fn test_anon_prepared_statements_extended() {
811+
crate::logger();
812+
load_test();
813+
814+
let (mut conn, mut client, _) = new_client!(false);
815+
816+
let mut c = (*config()).clone();
817+
c.config.general.prepared_statements = PreparedStatements::Extended;
818+
set(c).unwrap();
819+
820+
let handle = tokio::spawn(async move {
821+
client.run().await.unwrap();
822+
});
823+
824+
conn.write_all(&buffer!(
825+
{ Parse::new_anonymous("SELECT 1") },
826+
{ Bind::new_params("", &[]) },
827+
{ Execute::new() },
828+
{ Sync }
829+
))
830+
.await
831+
.unwrap();
832+
833+
let _ = read!(conn, ['1', '2', 'D', 'C', 'Z']);
834+
835+
{
836+
let cache = prepared_statements::PreparedStatements::global();
837+
let read = cache.read();
838+
assert!(read.is_empty());
839+
}
840+
841+
conn.write_all(&buffer!({ Terminate })).await.unwrap();
842+
handle.await.unwrap();
843+
}

pgdog/src/frontend/prepared_statements/mod.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use once_cell::sync::Lazy;
66
use parking_lot::RwLock;
77

88
use crate::{
9+
config::PreparedStatements as PreparedStatementsLevel,
910
frontend::router::parser::RewritePlan,
1011
net::{Parse, ProtocolMessage},
1112
stats::memory::MemoryUsage,
@@ -26,7 +27,7 @@ static CACHE: Lazy<PreparedStatements> = Lazy::new(PreparedStatements::default);
2627
pub struct PreparedStatements {
2728
pub(super) global: Arc<RwLock<GlobalCache>>,
2829
pub(super) local: HashMap<String, String>,
29-
pub(super) enabled: bool,
30+
pub(super) level: PreparedStatementsLevel,
3031
pub(super) capacity: usize,
3132
pub(super) memory_used: usize,
3233
}
@@ -35,7 +36,7 @@ impl MemoryUsage for PreparedStatements {
3536
#[inline]
3637
fn memory_usage(&self) -> usize {
3738
self.local.memory_usage()
38-
+ self.enabled.memory_usage()
39+
+ std::mem::size_of::<PreparedStatementsLevel>()
3940
+ self.capacity.memory_usage()
4041
+ std::mem::size_of::<Arc<RwLock<GlobalCache>>>()
4142
}
@@ -46,7 +47,7 @@ impl Default for PreparedStatements {
4647
Self {
4748
global: Arc::new(RwLock::new(GlobalCache::default())),
4849
local: HashMap::default(),
49-
enabled: true,
50+
level: PreparedStatementsLevel::Extended,
5051
capacity: usize::MAX,
5152
memory_used: 0,
5253
}

pgdog/src/net/protocol_message.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,17 @@ impl ProtocolMessage {
3131
)
3232
}
3333

34+
pub fn anonymous(&self) -> bool {
35+
use ProtocolMessage::*;
36+
37+
match self {
38+
Bind(bind) => bind.anonymous(),
39+
Parse(parse) => parse.anonymous(),
40+
Describe(describe) => describe.anonymous(),
41+
_ => false,
42+
}
43+
}
44+
3445
pub fn len(&self) -> usize {
3546
match self {
3647
Self::Bind(bind) => bind.len(),

0 commit comments

Comments
 (0)