Add Import-DbaParquet command#10309
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…to development
…ardrails (do Import-DbaParquet) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jovanpop-msft
left a comment
There was a problem hiding this comment.
[WIP] Added Import-DbaParquet
jovanpop-msft
left a comment
There was a problem hiding this comment.
Adding Import-DbaParquet
|
hum, not sure we want to add 1MB of added dependency here (also, dependency hell is already ... hellish). |
|
yes, this is a great candidate to be externalized like we did with SqlPackage. I'll take a closer look soon! Thank you so much @jovanpop-msft 🙇🏼 |
|
Still appreciate this, just gotta get some time to take a close look and treat it well 🙏🏼 |
|
working on this now 👍🏼 |
(do Install-DbaParquet, Get-DbaParquetPath, Import-DbaParquet)
Code Review: Add Import-DbaParquet commandThanks for this substantial contribution! Parquet import is a highly requested capability and the overall architecture is solid. Here's my review: OverviewThis PR adds two new public commands ( Strengths
Issues and SuggestionsCritical / Bugs1. Stop-Function -Message $message -Target "Parquet.NET"
if ($EnableException) {
throw "Parquet.NET not found"
}
Stop-Function -Message $message -Target "Parquet.NET" -EnableException $EnableException
return $null2. Write-Message -Level Verbose -Message "Successfully created table $schema.$table with the following column definitions:`n $($sqldatatypes -join "`n ")"Backtick inside a double-quoted string for 3. } catch {
# callers report back the error if $result is empty
}The catch block is entirely empty. Callers only check if 4. $sqlcmd = New-Object Microsoft.Data.SqlClient.SqlCommand($query, $sqlconn, $transaction)This inner function is called from 5. 6. Final end {
try {
if (-not $startedWithAnOpenConnection) {
$null = $sqlconn.Close() # $sqlconn is set inside process foreachWhen processing multiple files or instances, Style / Convention Issues7. 8. 9. 10. Parallel parameters exposed but not implemented 11. Missing commit message Performance12. Row-by-row 13. Minor
SummaryThe core functionality is well-implemented and the test coverage is good. The main things to address before merge:
Great addition to dbatools! |
(do Install-DbaParquet, Import-DbaParquet)
Rename the AutoCreateTable string flag from StoreStringAsUtf8 to a clearer switch NoUtf8 (invert logic by setting StoreStringAsUtf8 = -not $NoUtf8). Update public docs, parameter help, and tests to reflect the new switch. Remove unused parallel-related parameters and associated tests, tighten error handling (pass EnableException to Stop-Function, propagate ErrorRecord), fix naming/casing and quoting inconsistencies, and improve SQL parameter usage and switch literal consistency. Also add a transaction parameter to GetTableDefinition and update authorship/copyright metadata to 2026. (do Install-DbaParquet, Import-DbaParquet)
(do Install-DbaParquet, Import-DbaParquet)
Introduce the Import-DbaParquet command along with enhancements for schema management, error handling, and stability. Implement exact size logic for varchar and varbinary types, and add a parameter to store strings as UTF-8. Fix issues related to binary type mapping and automatic table creation.