sabir-akhadov-localstack commented on code in PR #2428:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/2428#discussion_r4316346310
##########
src/parser/mod.rs:
##########
@@ -12548,40 +12548,50 @@ impl<'a> Parser<'a> {
Ok(s.chars().next().unwrap())
}
- /// Parse a tab separated values in
- /// COPY payload
- pub fn parse_tsv(&mut self) -> Vec<Option<String>> {
- self.parse_tab_value()
- }
-
- /// Parse a single tab-separated value row used by `COPY` payload parsing.
- pub fn parse_tab_value(&mut self) -> Vec<Option<String>> {
- let mut values = vec![];
- let mut content = String::new();
- while let Some(t) = self.next_token_no_skip().map(|t| &t.token) {
- match t {
- Token::Whitespace(Whitespace::Tab) => {
- values.push(Some(core::mem::take(&mut content)));
- }
- Token::Whitespace(Whitespace::Newline) => {
- values.push(Some(core::mem::take(&mut content)));
- }
+ /// Parse the inline payload of `COPY ... FROM STDIN` as written.
+ ///
+ /// The payload is data rather than SQL, so it is captured uninterpreted.
Review Comment:
The payload is re-rendered from tokens rather than sliced from the source,
so it is not byte-for-byte: `0x1F` comes back as `X'1F'`, `E'\x41'` as `E'A'`,
CRLF as LF, and `'a''b'` as `'a'b'`, which no longer re-parses. Worth softening
"as written" here and in the `Statement::Copy` doc unless the tokenizer hands
over the raw text.
##########
src/parser/mod.rs:
##########
@@ -12548,40 +12548,50 @@ impl<'a> Parser<'a> {
Ok(s.chars().next().unwrap())
}
- /// Parse a tab separated values in
- /// COPY payload
- pub fn parse_tsv(&mut self) -> Vec<Option<String>> {
- self.parse_tab_value()
- }
-
- /// Parse a single tab-separated value row used by `COPY` payload parsing.
- pub fn parse_tab_value(&mut self) -> Vec<Option<String>> {
- let mut values = vec![];
- let mut content = String::new();
- while let Some(t) = self.next_token_no_skip().map(|t| &t.token) {
- match t {
- Token::Whitespace(Whitespace::Tab) => {
- values.push(Some(core::mem::take(&mut content)));
- }
- Token::Whitespace(Whitespace::Newline) => {
- values.push(Some(core::mem::take(&mut content)));
- }
+ /// Parse the inline payload of `COPY ... FROM STDIN` as written.
+ ///
+ /// The payload is data rather than SQL, so it is captured uninterpreted.
+ /// It starts on the line following the command and runs up to the `\.`
+ /// terminator, which is consumed but not returned. Field
+ /// separators, escapes and the `\N` null marker are left to the caller.
+ ///
+ /// Returns `None` when nothing but the statement delimiter follows, so a
+ /// bare `COPY t FROM STDIN;` is not given a payload it never carried.
+ fn parse_copy_payload(&mut self) -> Option<String> {
+ use core::fmt::Write;
+
+ let mut payload = String::new();
+ // The newline ending the command line is a delimiter, not data.
+ if self.peek_nth_token_no_skip_ref(0).token ==
Token::Whitespace(Whitespace::Newline) {
+ let _ = self.next_token_no_skip();
+ }
+ while let Some(token) = self.next_token_no_skip().map(|t| &t.token) {
+ match token {
Token::Backslash => {
- if self.consume_token(&Token::Period) {
- return values;
- }
- if let Token::Word(w) = self.next_token().token {
- if w.value == "N" {
- values.push(None);
- }
+ // `\.` terminates the payload, every other backslash is
data.
+ if self.peek_nth_token_no_skip_ref(0).token ==
Token::Period {
+ let _ = self.next_token_no_skip();
+ return Some(payload);
}
+ payload.push('\\');
}
- _ => {
- content.push_str(&t.to_string());
+ // `Display` for a single line comment appends a newline that
the
+ // tokenizer left in the stream as a separate token, so render
it
+ // here instead to avoid duplicating it.
+ Token::Whitespace(Whitespace::SingleLineComment { prefix,
comment }) => {
+ payload.push_str(prefix);
+ payload.push_str(comment);
+ }
+ // Rendering into the buffer avoids a temporary `String` per
+ // token, and a payload can hold an entire table.
+ token => {
+ let _ = write!(payload, "{token}");
}
}
}
- values
+ // An unterminated payload keeps whatever data it holds, but an empty
one
+ // means the statement had no inline data at all.
+ (!payload.is_empty()).then_some(payload)
Review Comment:
`COPY t FROM STDIN;\n\n` yields `Some("\n")` and prints a `\.`, while `;\n`
yields `None`; a whitespace-only unterminated trail should probably be `None`
too.
##########
src/parser/mod.rs:
##########
@@ -12548,40 +12548,50 @@ impl<'a> Parser<'a> {
Ok(s.chars().next().unwrap())
}
- /// Parse a tab separated values in
- /// COPY payload
- pub fn parse_tsv(&mut self) -> Vec<Option<String>> {
- self.parse_tab_value()
- }
-
- /// Parse a single tab-separated value row used by `COPY` payload parsing.
- pub fn parse_tab_value(&mut self) -> Vec<Option<String>> {
- let mut values = vec![];
- let mut content = String::new();
- while let Some(t) = self.next_token_no_skip().map(|t| &t.token) {
- match t {
- Token::Whitespace(Whitespace::Tab) => {
- values.push(Some(core::mem::take(&mut content)));
- }
- Token::Whitespace(Whitespace::Newline) => {
- values.push(Some(core::mem::take(&mut content)));
- }
+ /// Parse the inline payload of `COPY ... FROM STDIN` as written.
+ ///
+ /// The payload is data rather than SQL, so it is captured uninterpreted.
+ /// It starts on the line following the command and runs up to the `\.`
+ /// terminator, which is consumed but not returned. Field
+ /// separators, escapes and the `\N` null marker are left to the caller.
+ ///
+ /// Returns `None` when nothing but the statement delimiter follows, so a
+ /// bare `COPY t FROM STDIN;` is not given a payload it never carried.
+ fn parse_copy_payload(&mut self) -> Option<String> {
+ use core::fmt::Write;
+
+ let mut payload = String::new();
+ // The newline ending the command line is a delimiter, not data.
+ if self.peek_nth_token_no_skip_ref(0).token ==
Token::Whitespace(Whitespace::Newline) {
+ let _ = self.next_token_no_skip();
+ }
+ while let Some(token) = self.next_token_no_skip().map(|t| &t.token) {
+ match token {
Token::Backslash => {
- if self.consume_token(&Token::Period) {
- return values;
- }
- if let Token::Word(w) = self.next_token().token {
- if w.value == "N" {
- values.push(None);
- }
+ // `\.` terminates the payload, every other backslash is
data.
+ if self.peek_nth_token_no_skip_ref(0).token ==
Token::Period {
Review Comment:
PostgreSQL only ends the data at a line containing just `\.`, so
`1\tfoo\.bar` (literal `foo.bar` in text format, plain data in CSV) terminates
the payload here and then fails on `bar`. Requiring the previous token to be a
newline (or payload start) and the next to be newline/EOF would match the docs;
the same spot could also treat `\.` as the statement terminator, since
`\.\nSELECT 1;` currently fails with "Expected: end of statement".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
