Make COPY statement options compliant to Postgres#247
Conversation
|
|
||
| namespace hsql { | ||
|
|
||
| // Name unchanged for compatibility. |
There was a problem hiding this comment.
I don't get this comment.
There was a problem hiding this comment.
Dates back to when we added the COPY statement and, with it, the ability to export tables. Even though we use these file types for import and export, we kept the name so people with forks don't have to rename stuff in their code.
There was a problem hiding this comment.
This thinking is also the reson why I added the encoding as another member of the statement rather than just passing the options object with both file type and encoding.
There was a problem hiding this comment.
What name would you have chosen?
There was a problem hiding this comment.
For the enum? In the grammar, we simply call it file type (so FileType as enum name):
sql-parser/src/parser/bison_parser.y
Line 457 in 9a67d24
I remember that it was a big discussion back then and some insisted on compatibility. Anyway, I think it's kinda late for that now.
There was a problem hiding this comment.
Can you briefly mention that we call it FileyType in the grammar, but won't adapt it here?
Not stating what the alternative is (nobody is aware of the probably old discussion) does not give away why the current name might not be perfect. :)
There was a problem hiding this comment.
Feel free to zünd afterwards.
While fiddling around with a Postgres instance, I noticed that our syntax for
COPYstatements is not 100% compliant to Postgres. In detail, we usedCOPY table_a FROM 'path/to/tbl' WITH FORMAT CSV;, whereas Postgres usesCOPY table_a FROM 'path/to/tbl' WITH (FORMAT CSV);– there are brackets around the options. Furthermore, theWITHkeyword is optional [1].The incorrect syntax was introduced in #139 (my bad 😕) and it slipped through the review back then. This PR makes the
COPYstatement compliant to Postgres again. Furthermore, I used the opportunity to add the encoding as another option. This allows us to apply any specified encoding for imported and exported tables in Hyrise.[1] https://www.postgresql.org/docs/16/sql-copy.html