importccl: support IMPORT INTO for DELIMITED and PGCOPY#52628
importccl: support IMPORT INTO for DELIMITED and PGCOPY#52628craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
59d38db to
5ccfde9
Compare
|
Some notes:
|
pbardea
left a comment
There was a problem hiding this comment.
I'd prefer supporting these formats when they are fully supported (e.g. properly populate target cols). Maybe we can support one at a time in the same change that fixes the targeted column fix?
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @Anzoteh96 and @pbardea)
pkg/ccl/importccl/import_stmt.go, line 152 at r1 (raw file):
// File formats supported for IMPORT INTO var allowedIntoFormats = []string{"CSV", "AVRO", "DELIMITED", "PGCOPY"}
If we define this as a map, then we don't need the isSupportedImportInto, correct?
Anzoteh96
left a comment
There was a problem hiding this comment.
Yeah that makes sense for the targeted columns to be fixed too (though note that the targeted columns for AVRO will be less straightforward since it's extracted from the schema instead of the statement). I've marked it as WIP and will come to it again when that's being fixed.
Reviewable status:
9f661e2 to
c2cbba6
Compare
Anzoteh96
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/ccl/importccl/import_stmt_test.go, line 3176 at r2 (raw file):
Quoted 17 lines of code…
{ name: "delimited", data: "1\t2\n3\t4", create: "a INT, b INT DEFAULT 42, c INT", targetCols: "c, a", format: "DELIMITED", expectedResults: [][]string{{"2", "42", "1"}, {"4", "42", "3"}}, }, { name: "pgcopy", data: "1,2\n3,4", create: "a INT, b INT DEFAULT 42, c INT", targetCols: "c, a", with: `delimiter = ","`, format: "PGCOPY", expectedResults: [][]string{{"2", "42", "1"}, {"4", "42", "3"}}, },
Some update: I've fixed the targeted column problem and included test case when some columns are not targeted (as per the default column example here). Feel free to suggest any additional test case.
(Also, the target cols above are done in such a way that tests the validity of this import when target columns are reordered).
d3f9c23 to
2bb4485
Compare
Anzoteh96
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/ccl/importccl/import_stmt.go, line 152 at r1 (raw file):
Previously, pbardea (Paul Bardea) wrote…
If we define this as a map, then we don't need the
isSupportedImportInto, correct?
Done.
pkg/ccl/importccl/import_stmt_test.go, line 3176 at r2 (raw file):
Previously, Anzoteh96 (Anzo Teh) wrote…
{ name: "delimited", data: "1\t2\n3\t4", create: "a INT, b INT DEFAULT 42, c INT", targetCols: "c, a", format: "DELIMITED", expectedResults: [][]string{{"2", "42", "1"}, {"4", "42", "3"}}, }, { name: "pgcopy", data: "1,2\n3,4", create: "a INT, b INT DEFAULT 42, c INT", targetCols: "c, a", with: `delimiter = ","`, format: "PGCOPY", expectedResults: [][]string{{"2", "42", "1"}, {"4", "42", "3"}}, },Some update: I've fixed the targeted column problem and included test case when some columns are not targeted (as per the default column example here). Feel free to suggest any additional test case.
(Also, the target cols above are done in such a way that tests the validity of this import when target columns are reordered).
Update: added test cases for when targeted columns are reordered.
pkg/ccl/importccl/import_stmt_test.go, line 1176 at r4 (raw file):
verifyQuery: "SELECT * FROM t ORDER BY a", expected: [][]string{{"hello", "hello"}, {"hi", "hi"}}, },
Note: we had this for PGDUMP and MYSQLDUMP as added by Rohan, but it was removed when we enforce the restriction of support to CSV and AVRO. I'm now adding DELIMITED and PGCOPY.
Previously, IMPORT INTO is supported only for CSV and AVRO. This PR extends the support of IMPORT INTO for DELIMITED and PGCOPY, in the hope that this will be useful when supporting default columns for imports from these files. This involves populating the targeted columns from import processor when creating new input reader and row converter for DELIMITED and PGCOPY, when the targeted columns are specified in IMPORT INTO. Fixes cockroachdb#52405 Release note (general): IMPORT INTO is now supported for DELIMITED and PGCOPY file formats.
2bb4485 to
dd30ffd
Compare
|
TFTR! bors r=pbardea |
|
Build succeeded: |
Previously, IMPORT INTO is supported only for CSV and AVRO. This PR extends the support of IMPORT INTO for DELIMITED and PGCOPY, in the hope that this will be useful when supporting default columns for imports from these files.
This involves populating the targeted columns from import processor when creating new input reader and row converter for DELIMITED and PGCOPY, when the targeted columns are specified in
IMPORT INTO.Fixes #52405
Release note (general): IMPORT INTO is now supported for DELIMITED and PGCOPY file formats.