Skip to content

feat: side-effect SELECT #158

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

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
feat: side-effect SELECT
  • Loading branch information
denysvitali committed May 12, 2025
commit b9a0098a727860533e6396e36633c3509f1d68b0
49 changes: 49 additions & 0 deletions pgdog/src/frontend/router/parser/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,24 @@ use pg_query::{
use regex::Regex;
use tracing::{debug, trace};

static SIDE_EFFECT_FUNCTIONS: Lazy<HashSet<String>> = Lazy::new(|| {
let mut set = HashSet::new();
// Advisory lock functions
set.insert("pg_advisory_lock".to_lowercase());
set.insert("pg_advisory_xact_lock".to_lowercase());
set.insert("pg_advisory_lock_shared".to_lowercase());
set.insert("pg_advisory_xact_lock_shared".to_lowercase());
set.insert("pg_try_advisory_lock".to_lowercase());
set.insert("pg_try_advisory_xact_lock".to_lowercase());
set.insert("pg_try_advisory_lock_shared".to_lowercase());
set.insert("pg_try_advisory_xact_lock_shared".to_lowercase());
// Sequence functions
set.insert("nextval".to_lowercase());
set.insert("setval".to_lowercase());
// Add other functions with side-effects here
set
});

static REPLICATION_REGEX: Lazy<Regex> = Lazy::new(|| {
Regex::new(
"(CREATE_REPLICATION_SLOT|IDENTIFY_SYSTEM|DROP_REPLICATION_SLOT|READ_REPLICATION_SLOT|ALTER_REPLICATION_SLOT|TIMELINE_HISTORY).*",
Expand Down Expand Up @@ -226,6 +244,13 @@ impl QueryParser {
let mut command = match root.node {
// SELECT statements.
Some(NodeEnum::SelectStmt(ref stmt)) => {
// Check for side-effect functions in the target list
if Self::has_side_effect_function(stmt) {
debug!("Query contains side-effect function, routing to primary.");
return Ok(Command::Query(Route::write(None)));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes it a cross-shard query. I don't think we want that, necessarily, especially for advisory locks. You'd probably want to pick a shard that handles it, since advisory locks on different shards won't actually mutually exclude queries.

}

// If no side-effect functions found, proceed with normal SELECT routing
if matches!(shard, Shard::Direct(_)) {
return Ok(Command::Query(Route::read(shard)));
}
Expand Down Expand Up @@ -701,6 +726,30 @@ impl QueryParser {
fn delete(_stmt: &DeleteStmt) -> Result<Command, Error> {
Ok(Command::Query(Route::write(None)))
}

/// Check if a SELECT statement contains any functions with side effects.
fn has_side_effect_function(stmt: &SelectStmt) -> bool {
for target_item_wrapper_node in &stmt.target_list {
if let Some(NodeEnum::ResTarget(res_target)) = target_item_wrapper_node.node.as_ref() {
if let Some(val_node_wrapper) = &res_target.val {
if let Some(NodeEnum::FuncCall(func_call)) = val_node_wrapper.node.as_ref() {
if let Some(name_part_node) = func_call.funcname.last() {
if let Some(NodeEnum::String(func_name_node_val)) =
name_part_node.node.as_ref()
{
if SIDE_EFFECT_FUNCTIONS
.contains(&func_name_node_val.sval.to_lowercase())
{
return true;
}
}
}
}
}
}
}
false
}
}

#[cfg(test)]
Expand Down