Skip to content

Commit 8a3ada5

Browse files
authored
fix(prepared_statements): full rewrites extended as well (#971)
fixes #604
1 parent 1457c4c commit 8a3ada5

19 files changed

Lines changed: 502 additions & 49 deletions

File tree

integration/ci/apt.sh

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
#!/usr/bin/env bash
2-
# Thin `apt-get install` wrapper that no-ops on non-Linux platforms (macOS
3-
# developers running these scripts locally already have the deps installed
4-
# via their own package manager).
2+
# Thin `apt-get install` wrapper used in CI only.
3+
# No-ops when CI is not set (local dev) or apt-get is unavailable.
54
#
65
# Usage: integration/ci/apt.sh <pkg1> <pkg2> ...
76
set -euo pipefail
87

9-
if [[ "$(uname -s)" != "Linux" ]]; then
8+
if [[ -z "${CI:-}" ]] || ! command -v apt-get &>/dev/null; then
109
exit 0
1110
fi
1211

integration/prepared_statements_full/users.toml

Lines changed: 0 additions & 4 deletions
This file was deleted.

integration/ruby/common.sh

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
#!/bin/bash
2+
#
3+
# Shared helpers for ruby integration suites.
4+
# Source this file; do not execute directly.
5+
#
6+
RUBY_COMMON_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
7+
source "${RUBY_COMMON_DIR}/../common.sh"
8+
9+
# Install system packages and the bundler gem. Call once per CI run.
10+
function install_deps() {
11+
# Native gem extensions (psych, pg) need yaml + libpq headers.
12+
bash ${RUBY_COMMON_DIR}/../ci/apt.sh ruby-dev libyaml-dev libpq-dev build-essential
13+
command -v bundle >/dev/null || sudo gem install bundler --no-document
14+
}
15+
16+
# Run bundle install and rspec in TARGET_DIR using the shared Gemfile.
17+
# Defaults to RUBY_COMMON_DIR when called with no argument.
18+
function dev_suite() {
19+
local target_dir="${1:-$RUBY_COMMON_DIR}"
20+
21+
export BUNDLE_GEMFILE="${RUBY_COMMON_DIR}/Gemfile"
22+
export GEM_HOME=~/.gem
23+
mkdir -p ${GEM_HOME}
24+
25+
pushd "${target_dir}"
26+
bundle install
27+
bundle exec rspec *_spec.rb
28+
popd
29+
}
30+
31+
# Full CI cycle for a single suite: start pgdog, run tests, stop.
32+
# CONFIG_DIR is optional — omit to use the default integration/ config.
33+
# Call install_deps before the first run_suite in a process.
34+
function run_suite() {
35+
local config_dir="${1:-}"
36+
37+
if [ -n "$config_dir" ]; then
38+
run_pgdog "$config_dir"
39+
else
40+
run_pgdog
41+
fi
42+
wait_for_pgdog
43+
44+
dev_suite "$config_dir"
45+
46+
stop_pgdog
47+
}

integration/ruby/dev.sh

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,5 @@
11
#!/bin/bash
22
set -e
33
SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
4-
5-
pushd ${SCRIPT_DIR}
6-
7-
export GEM_HOME=~/.gem
8-
mkdir -p ${GEM_HOME}
9-
bundle install
10-
bundle exec rspec *_spec.rb
11-
12-
popd
4+
source "${SCRIPT_DIR}/common.sh"
5+
dev_suite
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
#!/bin/bash
2+
set -e
3+
SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
4+
source "${SCRIPT_DIR}/../common.sh"
5+
dev_suite "$SCRIPT_DIR"
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
[general]
2+
prepared_statements = "disabled"
3+
default_pool_size = 2
4+
5+
[[databases]]
6+
name = "pgdog"
7+
host = "127.0.0.1"
8+
9+
[admin]
10+
password = "pgdog"
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
# frozen_string_literal: true
2+
3+
require_relative '../rspec_helper'
4+
5+
# With prepared_statements = "disabled" pgdog forwards protocol messages as-is
6+
# without rewriting or caching.
7+
describe 'prepared_statements = disabled' do
8+
after { ensure_done }
9+
10+
# Anonymous statements (empty name) are a single Parse+Bind+Execute+Sync
11+
# cycle on one backend — no state needs to survive across cycles.
12+
it 'executes anonymous parameterized queries' do
13+
conn = connect
14+
10.times do |i|
15+
res = conn.exec_params('SELECT $1::bigint * 2 AS val', [i])
16+
expect(res[0]['val'].to_i).to eq(i * 2)
17+
end
18+
conn.close
19+
end
20+
21+
# Session mode pins one backend for the connection lifetime; pass-through
22+
# is sufficient because prepare and execute always reach the same backend.
23+
it 'passes named statements through in session mode' do
24+
conn = connect('pgdog', 'pgdog_session')
25+
conn.prepare('session_stmt', 'SELECT $1::bigint AS val')
26+
10.times do |i|
27+
res = conn.exec_prepared('session_stmt', [i])
28+
expect(res[0]['val'].to_i).to eq(i)
29+
end
30+
conn.close
31+
end
32+
33+
# Session mode gives each client its own dedicated backend, so two session
34+
# connections are guaranteed to land on different backends. Without a global
35+
# cache the execute on conn2 reaches a backend that never saw the prepare.
36+
it 'does not share statements across connections' do
37+
conn1 = connect('pgdog', 'pgdog_session')
38+
conn2 = connect('pgdog', 'pgdog_session')
39+
conn1.prepare('cross_stmt', 'SELECT $1::bigint AS val')
40+
expect do
41+
conn2.exec_prepared('cross_stmt', [7])
42+
end.to raise_error(PG::Error)
43+
conn1.close
44+
conn2.close
45+
end
46+
47+
# In transaction pool mode each query can land on a different backend.
48+
# disabled mode forwards Parse and Bind as-is with no global cache, so a
49+
# Bind that arrives on a backend that never saw the Parse fails.
50+
#
51+
# Sequential tests cannot force a crossing: a single connection always
52+
# returns the backend to the LIFO top between queries, so the next query
53+
# gets the same backend. Concurrent threads make the pool hand out both
54+
# backends simultaneously. With 5 threads and pool_size = 2, the pigeonhole
55+
# principle guarantees that at least 3 threads will attempt PREPARE on a
56+
# backend that already holds the statement, producing 'already exists'
57+
# errors — regardless of timing.
58+
#
59+
# Mirror: full/'executes named extended-protocol statements in
60+
# transaction pool mode' — same structure, opposite expectation.
61+
it 'fails named extended-protocol statements in transaction pool mode' do
62+
errors = []
63+
mutex = Mutex.new
64+
65+
threads = 5.times.map do
66+
Thread.new do
67+
conn = connect
68+
begin
69+
conn.prepare('ext_stmt', 'SELECT $1::bigint AS val')
70+
20.times { conn.exec_prepared('ext_stmt', [42]) }
71+
rescue PG::Error => e
72+
mutex.synchronize { errors << e.message }
73+
ensure
74+
conn.close rescue nil
75+
end
76+
end
77+
end
78+
79+
threads.each(&:join)
80+
expect(errors).not_to be_empty
81+
end
82+
83+
# Same mechanism as the extended-protocol test above, but for the
84+
# simple-protocol PREPARE/EXECUTE path. disabled mode forwards the SQL
85+
# statement text as-is, so EXECUTE on a backend that never saw the
86+
# PREPARE fails with 'prepared statement does not exist' or, if two
87+
# threads hit the same backend, 'already exists'.
88+
#
89+
# Mirror: full/'rewrites simple-protocol PREPARE / EXECUTE in
90+
# transaction pool mode' — same structure, opposite expectation.
91+
it 'fails SQL PREPARE/EXECUTE in transaction pool mode' do
92+
errors = []
93+
mutex = Mutex.new
94+
95+
threads = 5.times.map do
96+
Thread.new do
97+
conn = connect
98+
begin
99+
conn.exec('PREPARE sql_stmt AS SELECT $1::bigint * 2')
100+
20.times { |i| conn.exec("EXECUTE sql_stmt(#{i})") }
101+
rescue PG::Error => e
102+
mutex.synchronize { errors << e.message }
103+
ensure
104+
conn.close rescue nil
105+
end
106+
end
107+
end
108+
109+
threads.each(&:join)
110+
expect(errors).not_to be_empty
111+
end
112+
end
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
#!/bin/bash
2+
set -e
3+
SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
4+
source "${SCRIPT_DIR}/../common.sh"
5+
install_deps
6+
run_suite "$SCRIPT_DIR"
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
[[users]]
2+
database = "pgdog"
3+
name = "pgdog"
4+
password = "pgdog"
5+
6+
7+
[[users]]
8+
name = "pgdog_session"
9+
database = "pgdog"
10+
password = "pgdog"
11+
server_user = "pgdog"
12+
pooler_mode = "session"
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
#!/bin/bash
2+
set -e
3+
SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
4+
source "${SCRIPT_DIR}/../common.sh"
5+
dev_suite "$SCRIPT_DIR"

0 commit comments

Comments
 (0)