fix: Update Copy parser to handle ',' separated option list #49
Conversation
single char regex to wider set of characters.
Vizerai
changed the title
…o expand single char regex to wider set of characters.
Vizerai
changed the title
| @@ -242,6 +241,8 @@ private void parseCopyStatement() throws Exception { | |||
| parse(sql, this.options); | |||
| } catch (Exception e) { | |||
| throw new SQLException("Invalid COPY statement syntax: " + e.toString()); | |||
| } catch (TokenMgrError e) { | |||
There was a problem hiding this comment.
nit: you can combine the two catch clauses into one like this:
} catch (Exception | TokenMgrError e) {
throw new SQLException("Invalid COPY statement syntax: " + e.toString());
}
| assertThat(options.getDelimiter(), is(equalTo(value))); | ||
| assertThat(options.getQuote(), is(equalTo(value1))); | ||
| assertThat(options.getEscape(), is(equalTo(value2))); | ||
| } |
There was a problem hiding this comment.
Can we add the following assertion (here and also for the other tests):
| } | |
| assertEquals(StatementParser.parseCommand(StatementParser.removeCommentsAndTrim(sql)), "COPY"); | |
| } |
This assertion currently fails for ESCAPE '\', as our parser (correctly?) sees '' as an unclosed string literal.
There was a problem hiding this comment.
Yes, removeCommentsAndTrim() does not allow a single '\' in the statement. Copy expects the delimiter, escape, and quote options to be single characters.
Le'ts drop ', ", and \ for now. The logic in removeCommentsAndTrim() would break if we allowed those. We could add them back in at a later point after we rework removeCommentsAndTrim().
There was a problem hiding this comment.
yeah, that sounds reasonable.
Update Copy parser to handle ',' separated option list. Also expand single char regex to wider set of characters [! - ~].