Skip to content

Commit c9826d9

Browse files
committed
fix(pgwire): respect identifier boundaries when stripping RETURNING
The keyword scanner treated `_` as a word boundary, so an identifier like `orders_returning` could be misread as containing the RETURNING keyword, splitting the statement at the wrong offset. Underscore is a valid identifier character in SQL, so boundary checks now test for any identifier byte (alnum or `_`) rather than alnum alone.
1 parent 07b271f commit c9826d9

1 file changed

Lines changed: 30 additions & 4 deletions

File tree

nodedb/src/control/server/pgwire/handler/returning.rs

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,16 @@ fn is_valid_column_name(name: &str) -> bool {
155155
.all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '.')
156156
}
157157

158+
/// Whether `b` can appear inside a SQL identifier (letter, digit, or `_`).
159+
///
160+
/// Underscore is an identifier character, so a keyword match must NOT treat a
161+
/// neighbouring `_` as a word boundary — otherwise `RETURNING` is falsely found
162+
/// inside an identifier such as `orders_returning`, splitting the statement at
163+
/// the wrong place.
164+
fn is_ident_byte(b: u8) -> bool {
165+
b.is_ascii_alphanumeric() || b == b'_'
166+
}
167+
158168
/// Find the byte offset of `word` in `upper_text` respecting word boundaries.
159169
fn find_word_boundary(upper_text: &str, word: &str) -> Option<usize> {
160170
let bytes = upper_text.as_bytes();
@@ -164,8 +174,8 @@ fn find_word_boundary(upper_text: &str, word: &str) -> Option<usize> {
164174
let mut i = 0;
165175
while i + wlen <= bytes.len() {
166176
if &bytes[i..i + wlen] == wbytes {
167-
let before_ok = i == 0 || !bytes[i - 1].is_ascii_alphanumeric();
168-
let after_ok = i + wlen >= bytes.len() || !bytes[i + wlen].is_ascii_alphanumeric();
177+
let before_ok = i == 0 || !is_ident_byte(bytes[i - 1]);
178+
let after_ok = i + wlen >= bytes.len() || !is_ident_byte(bytes[i + wlen]);
169179
if before_ok && after_ok {
170180
return Some(i);
171181
}
@@ -204,8 +214,8 @@ fn find_returning_keyword(upper: &str) -> Option<usize> {
204214

205215
if i + kw_len <= bytes.len()
206216
&& &bytes[i..i + kw_len] == keyword
207-
&& (i == 0 || !bytes[i - 1].is_ascii_alphanumeric())
208-
&& (i + kw_len >= bytes.len() || !bytes[i + kw_len].is_ascii_alphanumeric())
217+
&& (i == 0 || !is_ident_byte(bytes[i - 1]))
218+
&& (i + kw_len >= bytes.len() || !is_ident_byte(bytes[i + kw_len]))
209219
{
210220
return Some(i);
211221
}
@@ -283,6 +293,22 @@ mod tests {
283293
assert_eq!(sql, "UPDATE products SET stock = 0 WHERE id = 'p1'");
284294
}
285295

296+
#[test]
297+
fn returning_inside_identifier_not_treated_as_keyword() {
298+
// A collection/table whose name embeds "returning" (with `_` as an
299+
// identifier boundary) must NOT match the RETURNING keyword inside the
300+
// name — the real keyword is the trailing one after WHERE.
301+
let (sql, spec) =
302+
strip_returning("DELETE FROM orders_returning WHERE id = 'p1' RETURNING *").unwrap();
303+
assert_eq!(sql, "DELETE FROM orders_returning WHERE id = 'p1'");
304+
assert_eq!(spec.unwrap().columns, ReturningColumns::Star);
305+
306+
// Same identifier with no trailing RETURNING clause → no spec, unchanged.
307+
let (sql, spec) = strip_returning("DELETE FROM orders_returning WHERE id = 'p1'").unwrap();
308+
assert!(spec.is_none());
309+
assert_eq!(sql, "DELETE FROM orders_returning WHERE id = 'p1'");
310+
}
311+
286312
#[test]
287313
fn returning_in_string_literal_ignored() {
288314
let (sql, spec) =

0 commit comments

Comments
 (0)