Skip to content

sql: allow " as an escape character for COPY#52044

Closed
otan wants to merge 1 commit intocockroachdb:masterfrom
otan-cockroach:copy_allow_qt
Closed

sql: allow " as an escape character for COPY#52044
otan wants to merge 1 commit intocockroachdb:masterfrom
otan-cockroach:copy_allow_qt

Conversation

@otan
Copy link
Copy Markdown
Contributor

@otan otan commented Jul 28, 2020

PostgreSQL allows \\\" as an escape for \". However, we seem to not
allow it at all. This was a blocker for osm2pgsql.

In general, I think we should be allowing any non-captured escape to be
used as is, as PostgreSQL does:

otan=# \copy copytest from stdin
Enter data to be copied followed by a newline.
End with a backslash and a period on a line by itself, or an EOF signal.
>> \1\2
>> COPY 1
otan=# select * from copytest
;
    t
----------
 \"
 \x01\x02
(2 rows)

otan=# \copy copytest from stdin
Enter data to be copied followed by a newline.
End with a backslash and a period on a line by itself, or an EOF signal.
>> \a
>> COPY 1
otan=# select * from copytest
;
    t
----------
 \"
 \x01\x02
 a
(3 rows)

otan=# \copy copytest from stdin
Enter data to be copied followed by a newline.
End with a backslash and a period on a line by itself, or an EOF signal.
>> \b
>> COPY 1
otan=# select * from copytest
;
    t
----------
 \"
 \x01\x02
 a
 \x08
(4 rows)

If you think we should change our code to follow this, let me know.

Release note: None

@otan otan requested review from a team and madelynnblue July 28, 2020 23:38
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

PostgreSQL allows `\\\"` as an escape for `\"`. However, we seem to not
allow it at all. This was a blocker for osm2pgsql.

In general, I think we should be allowing any non-captured escape to be
used as is, as PostgreSQL does:
```
otan=# \copy copytest from stdin
Enter data to be copied followed by a newline.
End with a backslash and a period on a line by itself, or an EOF signal.
>> \1\2
>> COPY 1
otan=# select * from copytest
;
    t
----------
 \"
 \x01\x02
(2 rows)

otan=# \copy copytest from stdin
Enter data to be copied followed by a newline.
End with a backslash and a period on a line by itself, or an EOF signal.
>> \a
>> COPY 1
otan=# select * from copytest
;
    t
----------
 \"
 \x01\x02
 a
(3 rows)

otan=# \copy copytest from stdin
Enter data to be copied followed by a newline.
End with a backslash and a period on a line by itself, or an EOF signal.
>> \b
>> COPY 1
otan=# select * from copytest
;
    t
----------
 \"
 \x01\x02
 a
 \x08
(4 rows)
```

If you think we should change our code to follow this, let me know.

Release note: None
@otan otan removed the request for review from a team July 29, 2020 03:28
@madelynnblue
Copy link
Copy Markdown
Contributor

From https://www.postgresql.org/docs/10/sql-copy.html:

Any other backslashed character that is not mentioned in the above table will be taken to represent itself.

This is the actual problem. Even though your PR would fix the \" case, it wouldn't fix the \a case you also have in your terminal example above. The correct fix would be to do the above sentence (and the rest of that paragraph which deals with priority of special escaped sequences).

@otan
Copy link
Copy Markdown
Contributor Author

otan commented Jul 29, 2020

sg!

@otan otan closed this Jul 29, 2020
@madelynnblue
Copy link
Copy Markdown
Contributor

I'm working on COPY on my Fridays so I'll fix this.

@otan
Copy link
Copy Markdown
Contributor Author

otan commented Jul 29, 2020

well, saw this too late :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants