[SPARK-20731][SQL] Add ability to change or omit .csv file extension in CSV Data Source #17973
Conversation
mikkokupsu
changed the title
|
ok to test |
| cars.coalesce(1).write | ||
| .option("header", "true") | ||
| .option("fileExtension", ".tsv") | ||
| .option("delimiter", "\t") |
There was a problem hiding this comment.
When delimiter is set to \t, is it still a CSV? : )
There was a problem hiding this comment.
Sorry, I have really hard time understanding what you asking here. Could you be more specific?
There was a problem hiding this comment.
I guess the whole API and implementation is called "CSV" still. Of course, you can already read/write files with different names and different delimiters. Does this matter enough to make a new option? what if I delimit with, eh, null characters?
There was a problem hiding this comment.
@srowen and @gatorsmile, the reason why I pushed this pull request is actually to omit the file extension completely. I guess we can discuss the semantics of different delimiters and file formats but the whole point of this pull request was to give users the option to change a hard coded value.
There was a problem hiding this comment.
I would like to suggest to leave this out if there is no better reason for now. Downside of this is, it looks this allows arbitrary name and it does not gurantee the extention is, say, tsv when the delmiter is a tab. It is purely up to the user.
I added those extentions long ago and one of the motivation was auto detection of datasource like Haddop does (which we ended up with not adding it yet due to the cost of listing files and etc).
There was a problem hiding this comment.
What is the reason why Hive introduced the conf hive.output.file.extension?
There was a problem hiding this comment.
Also, what is your usage scenario? It sounds like you want to omit the extension?
There was a problem hiding this comment.
@gatorsmile, Yes, my use case is to omit the extension, but I decided to make the implementation flexible i.e. .option("fileExtension", "")
There was a problem hiding this comment.
Just curious what is the reason you need to omit the extension?
There was a problem hiding this comment.
@gatorsmile, We provide data files to our clients and specify the file format to TAB separated. I want to avoid all confusion where someone receives just the dataset and confuses the data to be COMMA separated.
|
Test build #76896 has finished for PR 17973 at commit
|
|
@mikkokupsu We do not plan to support this after an offline discussion with the other Committers. Thanks for your contribution. Could you close this PR please? |
|
Can one of the admins verify this patch? |
What changes were proposed in this pull request? The existing CSV DataSource allows one to set the delimiter/separator but does not allow the changing of the file extension. This means that a file can have values separated by tabs but me marked as a ".csv" file. This change allows one to change the file extension to match the delimiter/separator (e.g. ".tsv" for a tab separated value file). Why are the changes needed? This PR adds an additional option to set the fileExtension. The end result is that when a separator is set that is not a comma that the output file has a file extension that matches the separator (e.g. file.tsv, file.psv, etc...). Notes on Previous Pull Request apache#17973 A pull request adding this option was discussed 7 years ago. One reason it wasn't added was: "I would like to suggest to leave this out if there is no better reason for now. Downside of this is, it looks this allows arbitrary name and it does not gurantee the extention is, say, tsv when the delmiter is a tab. It is purely up to the user." I don't believe this is a good reason to not let the user set the extension. If we let them set the delimiter/separator to an arbitrary string/char then why not let the user also set the file extension to specify the separator that the file uses (e.g. tsv, psv, etc...). This addition keeps the "csv" file extension as the default and has the benefit of allowing other separators to match the file extension. Does this PR introduce any user-facing change? Yes. This PR adds one row to the options table for the CSV DataSource documentation to include the "fileExtension" option. How was this patch tested? One unit test was added to validate a file is written with the new extension. Was this patch authored or co-authored using generative AI tooling? No
### What changes were proposed in this pull request? The existing CSV DataSource allows one to set the delimiter/separator but does not allow the changing of the file extension. This means that a file can have values separated by tabs but me marked as a ".csv" file. This change allows one to change the file extension to match the delimiter/separator (e.g. ".tsv" for a tab separated value file). ### Why are the changes needed? This PR adds an additional option to set the fileExtension. The end result is that when a separator is set that is not a comma that the output file has a file extension that matches the separator (e.g. file.tsv, file.psv, etc...). Notes on Previous Pull Request #17973 A pull request adding this option was discussed 7 years ago. One reason it wasn't added was: "I would like to suggest to leave this out if there is no better reason for now. Downside of this is, it looks this allows arbitrary name and it does not gurantee the extention is, say, tsv when the delmiter is a tab. It is purely up to the user." I don't believe this is a good reason to not let the user set the extension. If we let them set the delimiter/separator to an arbitrary string/char then why not let the user also set the file extension to specify the separator that the file uses (e.g. tsv, psv, etc...). This addition keeps the "csv" file extension as the default and has the benefit of allowing other separators to match the file extension. ### Does this PR introduce _any_ user-facing change? Yes. This PR adds one row to the options table for the CSV DataSource documentation to include the "fileExtension" option. ### How was this patch tested? One unit test was added to validate a file is written with the new extension. ### Was this patch authored or co-authored using generative AI tooling? No Closes #49233 from jabbaugh/jbaugh-add-csv-file-ext. Authored-by: Jim Baugh <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
Closes apache#17422 Closes apache#17619 Closes apache#18034 Closes apache#18229 Closes apache#18268 Closes apache#17973 Closes apache#18125 Closes apache#18918 Closes apache#19274 Closes apache#19456 Closes apache#19510 Closes apache#19420 Closes apache#20090 Closes apache#20177 Closes apache#20304 Closes apache#20319 Closes apache#20543 Closes apache#20437 Closes apache#21261 Closes apache#21726 Closes apache#14653 Closes apache#13143 Closes apache#17894 Closes apache#19758 Closes apache#12951 Closes apache#17092 Closes apache#21240 Closes apache#16910 Closes apache#12904 Closes apache#21731 Closes apache#21095 Added: Closes apache#19233 Closes apache#20100 Closes apache#21453 Closes apache#21455 Closes apache#18477 Added: Closes apache#21812 Closes apache#21787 Author: hyukjinkwon <[email protected]> Closes apache#21781 from HyukjinKwon/closing-prs.
7 participants
What changes were proposed in this pull request?
Add new option to CSV Data Source to make changing hard coded ".csv" file extension to something else.
https://issues.apache.org/jira/browse/SPARK-20731
How was this patch tested?
Unit tests.