diff --git a/CHANGELOG.md b/CHANGELOG.md index 98aff1bc72..40b14ebd00 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Explain Analyze running write statements on Read-Only connections and skipping the Alert and Safe Mode confirmation. - Remote deletions of connections, groups, tags, SSH profiles and table favorites applied with their sync category off. - **Local only** connections taking edits and deletions made on another device. +- Oracle, Snowflake and Dameng `NUMBER` rounded or left empty, and `DECIMAL` losing digits, in Parquet exports. +- Oracle `BINARY_FLOAT` and `BINARY_DOUBLE` columns written as text in Parquet exports. +- PostgreSQL `money` values written as null in Parquet exports. +- Files left behind when a multi-table Parquet export is stopped between tables. - Filter-bar BETWEEN refused on Typesense and Weaviate, and given the wrong lower bound on BigQuery. - Cassandra filter error telling MCP clients to use a Match All control they do not have. - Japanese, Chinese and Korean text in CSV, TSV and SQL files opening as garbled characters. diff --git a/Plugins/ParquetExportPlugin/ParquetExportPlugin.swift b/Plugins/ParquetExportPlugin/ParquetExportPlugin.swift index 3db4d270c4..b374e65d25 100644 --- a/Plugins/ParquetExportPlugin/ParquetExportPlugin.swift +++ b/Plugins/ParquetExportPlugin/ParquetExportPlugin.swift @@ -61,21 +61,9 @@ final class ParquetExportPlugin: ObservableObject, ExportFormatPlugin, SettableP ) async throws -> ExportFormatResult { guard !tables.isEmpty else { return ExportFormatResult() } var warnings: [String] = [] - var written: [URL] = [] - for (index, table) in tables.enumerated() { - try progress.checkCancellation() - progress.setCurrentTable(table.qualifiedName, index: index + 1) - let fileURL = tables.count == 1 - ? destination - : ParquetFileNaming.perTableURL(destination: destination, table: table.name) - do { - try await writeTable(table, dataSource: dataSource, to: fileURL, progress: progress) - written.append(fileURL) - } catch { - for url in written { try? FileManager.default.removeItem(at: url) } - throw error - } + try await ParquetTableFiles.write(tables, destination: destination, progress: progress) { table, fileURL in + try await writeTable(table, dataSource: dataSource, to: fileURL, progress: progress) } if tables.count > 1 { @@ -109,7 +97,8 @@ final class ParquetExportPlugin: ObservableObject, ExportFormatPlugin, SettableP case .header(let header): columns = header.columns let types = columns.map { column in - ParquetTypeMapper.duckDBType(forColumnType: declaredTypes[column] ?? "") + ParquetTypeMapper.duckDBType( + forColumnType: declaredTypes[column] ?? "", databaseTypeId: dataSource.databaseTypeId) } try staging.createTable(columns: columns, types: types) created = true diff --git a/Plugins/ParquetExportPlugin/ParquetTableFiles.swift b/Plugins/ParquetExportPlugin/ParquetTableFiles.swift new file mode 100644 index 0000000000..0afa9bc9ce --- /dev/null +++ b/Plugins/ParquetExportPlugin/ParquetTableFiles.swift @@ -0,0 +1,35 @@ +// +// ParquetTableFiles.swift +// ParquetExportPlugin +// + +import Foundation +import TableProPluginKit + +public enum ParquetTableFiles { + public static func write( + _ tables: [PluginExportTable], + destination: URL, + progress: PluginExportProgress, + writeTable: (PluginExportTable, URL) async throws -> Void + ) async throws { + var written: [URL] = [] + do { + for (index, table) in tables.enumerated() { + try progress.checkCancellation() + progress.setCurrentTable(table.qualifiedName, index: index + 1) + let fileURL = fileURL(for: table, tableCount: tables.count, destination: destination) + try await writeTable(table, fileURL) + written.append(fileURL) + } + } catch { + for url in written { try? FileManager.default.removeItem(at: url) } + throw error + } + } + + private static func fileURL(for table: PluginExportTable, tableCount: Int, destination: URL) -> URL { + guard tableCount > 1 else { return destination } + return ParquetFileNaming.perTableURL(destination: destination, table: table.name) + } +} diff --git a/Plugins/ParquetExportPlugin/ParquetTypeMapper.swift b/Plugins/ParquetExportPlugin/ParquetTypeMapper.swift index 927db5174c..d505a4c0a1 100644 --- a/Plugins/ParquetExportPlugin/ParquetTypeMapper.swift +++ b/Plugins/ParquetExportPlugin/ParquetTypeMapper.swift @@ -11,17 +11,18 @@ import TableProPluginKit /// Parquet is a typed format, so writing every column as a string would produce a file that reads /// back with no numbers, no dates and no booleans. The source engine's own type name is the only /// thing that says what a column holds, because a streamed value arrives as text either way. -/// -/// The mapping is deliberately coarse. Getting a width or a precision wrong writes a file that -/// silently truncates, and Parquet's own type set is small: matching families is right, matching -/// exact declarations is not achievable across twenty engines. public enum ParquetTypeMapper { /// The DuckDB type for a column, or `VARCHAR` when nothing better is known. A value that fails /// to cast becomes null rather than failing the export, which is what `TRY_CAST` gives. - public static func duckDBType(forColumnType typeName: String) -> String { + public static func duckDBType(forColumnType typeName: String, databaseTypeId: String) -> String { let base = baseName(typeName) + if let moneyType = fixedPointMoneyTypes[databaseTypeId]?[base] { return moneyType } if integerTypes.contains(base) { return "BIGINT" } - if decimalTypes.contains(base) { return "DOUBLE" } + if exactNumericTypes.contains(base) { + guard !enginesIgnoringDeclaredPrecision.contains(databaseTypeId) else { return "DOUBLE" } + return exactNumericType(declaredAs: typeName) + } + if approximateNumericTypes.contains(base) { return "DOUBLE" } if booleanTypes.contains(base) { return "BOOLEAN" } if dateTypes.contains(base) { return "DATE" } if timestampTypes.contains(base) { return "TIMESTAMP" } @@ -43,13 +44,45 @@ public enum ParquetTypeMapper { .map(String.init) ?? withoutArgs } + private static func exactNumericType(declaredAs typeName: String) -> String { + let arguments = typeArguments(typeName) + guard let precision = arguments.first.flatMap({ Int($0) }), + (1 ... maximumDecimalDigits).contains(precision) else { return "DOUBLE" } + let scale = arguments.count > 1 ? Int(arguments[1]) : 0 + guard let scale, (0 ... precision).contains(scale) else { return "DOUBLE" } + if scale == 0, precision <= maximumBigIntDigits { return "BIGINT" } + return "DECIMAL(\(precision),\(scale))" + } + + private static func typeArguments(_ typeName: String) -> [String] { + guard let open = typeName.firstIndex(of: "("), + let close = typeName[open...].firstIndex(of: ")") else { return [] } + return typeName[typeName.index(after: open) ..< close] + .split(separator: ",") + .map { $0.trimmingCharacters(in: .whitespaces) } + } + + private static let maximumBigIntDigits = 18 + + private static let maximumDecimalDigits = 38 + + private static let enginesIgnoringDeclaredPrecision: Set = [ + "SQLite", "libSQL", "Turso", "Cloudflare D1" + ] + + private static let fixedPointMoneyTypes: [String: [String: String]] = [ + "SQL Server": ["money": "DECIMAL(19,4)", "smallmoney": "DECIMAL(10,4)"] + ] + private static let integerTypes: Set = [ "int", "int2", "int4", "int8", "integer", "smallint", "bigint", "tinyint", - "mediumint", "serial", "bigserial", "smallserial", "year", "number" + "mediumint", "serial", "bigserial", "smallserial", "year" ] - private static let decimalTypes: Set = [ - "decimal", "numeric", "float", "float4", "float8", "double", "real", "money" + private static let exactNumericTypes: Set = ["decimal", "numeric", "number"] + + private static let approximateNumericTypes: Set = [ + "float", "float4", "float8", "double", "real", "binary_float", "binary_double" ] private static let booleanTypes: Set = ["bool", "boolean", "bit"] diff --git a/TableProTests/Plugins/ExportFormatEscapingTests.swift b/TableProTests/Plugins/ExportFormatEscapingTests.swift index 1c2b418a76..426556a67d 100644 --- a/TableProTests/Plugins/ExportFormatEscapingTests.swift +++ b/TableProTests/Plugins/ExportFormatEscapingTests.swift @@ -8,7 +8,6 @@ import TableProPluginKit import Testing struct MarkdownExportEscapingTests { - /// A pipe closes a cell, so a value holding one would end the cell early and shift every /// column after it. @Test("A pipe in a value is escaped") @@ -66,7 +65,6 @@ struct MarkdownExportEscapingTests { } struct HTMLExportEscapingTests { - /// Every value in an export comes from the database, so a value holding markup reaches a file /// someone opens in a browser. @Test("Markup characters are escaped") @@ -98,7 +96,6 @@ struct HTMLExportEscapingTests { } struct XMLExportEscapingTests { - @Test("The five predefined entities are escaped") func entitiesAreEscaped() { #expect(XMLEscaping.text("") == "<a & b>") @@ -139,43 +136,105 @@ struct XMLExportEscapingTests { } struct ParquetTypeMapperTests { + private func mapped(_ typeName: String, on databaseTypeId: String = "PostgreSQL") -> String { + ParquetTypeMapper.duckDBType(forColumnType: typeName, databaseTypeId: databaseTypeId) + } @Test("Integer families map to BIGINT") func integerFamilies() { for type in ["INT", "int4", "BIGINT", "smallint", "TINYINT", "SERIAL", "MEDIUMINT"] { - #expect(ParquetTypeMapper.duckDBType(forColumnType: type) == "BIGINT", "\(type)") + #expect(mapped(type) == "BIGINT", "\(type)") + } + } + + @Test("Floating families and undeclared exact numerics map to DOUBLE") + func doubleFamilies() { + for type in ["numeric", "FLOAT", "double precision", "REAL"] { + #expect(mapped(type) == "DOUBLE", "\(type)") } } - @Test("Decimal families map to DOUBLE") - func decimalFamilies() { - for type in ["DECIMAL(10,2)", "numeric", "FLOAT", "double precision", "REAL", "money"] { - #expect(ParquetTypeMapper.duckDBType(forColumnType: type) == "DOUBLE", "\(type)") + @Test("PostgreSQL money keeps its text, since it carries a currency symbol no cast can read") + func postgresMoneyStaysText() { + #expect(mapped("money", on: "PostgreSQL") == "VARCHAR") + #expect(mapped("MONEY", on: "PostgreSQL") == "VARCHAR") + } + + @Test("SQL Server money keeps its exact four decimal places") + func sqlServerMoneyIsFixedPoint() { + #expect(mapped("money", on: "SQL Server") == "DECIMAL(19,4)") + #expect(mapped("smallmoney", on: "SQL Server") == "DECIMAL(10,4)") + #expect(mapped("SMALLMONEY", on: "SQL Server") == "DECIMAL(10,4)") + } + + @Test("An exact numeric with no usable declaration maps to DOUBLE") + func undeclaredExactNumericIsDouble() { + let types = ["number", "NUMBER", "NUMBER(*,0)", "numeric", "numeric(65,30)", "NUMBER(2,5)"] + for type in types { + #expect(mapped(type, on: "Oracle") == "DOUBLE", "\(type)") } } + @Test("A declared exact numeric keeps its precision and scale") + func declaredExactNumericKeepsPrecision() { + let expected: [String: String] = [ + "NUMBER(10,2)": "DECIMAL(10,2)", + "number(10, 2)": "DECIMAL(10,2)", + "NUMBER(19)": "DECIMAL(19,0)", + "number(38)": "DECIMAL(38,0)", + "DECIMAL(10,2)": "DECIMAL(10,2)", + "numeric(38,10)": "DECIMAL(38,10)" + ] + for (type, duckDBType) in expected { + #expect(mapped(type, on: "Oracle") == duckDBType, "\(type)") + } + } + + @Test("An exact numeric declared with scale 0 and at most 18 digits maps to BIGINT") + func exactNumericWithScaleZeroIsAnInteger() { + let types = ["number(10)", "NUMBER(18)", "NUMBER(18,0)", "number(1, 0)", "DECIMAL(10,0)", "numeric(5)"] + for type in types { + #expect(mapped(type, on: "Oracle") == "BIGINT", "\(type)") + } + } + + @Test("SQLite-family engines ignore a declared precision, so their exact numerics map to DOUBLE") + func sqliteFamilyIgnoresDeclaredPrecision() { + for engine in ["SQLite", "libSQL", "Turso", "Cloudflare D1"] { + for type in ["DECIMAL(10,2)", "NUMERIC(10)", "number(19)"] { + #expect(mapped(type, on: engine) == "DOUBLE", "\(type) on \(engine)") + } + } + } + + @Test("Oracle's binary floating point types map to DOUBLE") + func oracleBinaryFloatsAreDoubles() { + #expect(mapped("binary_float", on: "Oracle") == "DOUBLE") + #expect(mapped("BINARY_DOUBLE", on: "Oracle") == "DOUBLE") + } + @Test("Temporal families keep their own types") func temporalFamilies() { - #expect(ParquetTypeMapper.duckDBType(forColumnType: "DATE") == "DATE") - #expect(ParquetTypeMapper.duckDBType(forColumnType: "timestamp with time zone") == "TIMESTAMP") - #expect(ParquetTypeMapper.duckDBType(forColumnType: "datetime") == "TIMESTAMP") - #expect(ParquetTypeMapper.duckDBType(forColumnType: "TIME") == "TIME") + #expect(mapped("DATE") == "DATE") + #expect(mapped("timestamp with time zone") == "TIMESTAMP") + #expect(mapped("datetime") == "TIMESTAMP") + #expect(mapped("TIME") == "TIME") } @Test("Booleans and binaries map to their own types") func booleanAndBinary() { - #expect(ParquetTypeMapper.duckDBType(forColumnType: "BOOLEAN") == "BOOLEAN") - #expect(ParquetTypeMapper.duckDBType(forColumnType: "bytea") == "BLOB") - #expect(ParquetTypeMapper.duckDBType(forColumnType: "VARBINARY(50)") == "BLOB") + #expect(mapped("BOOLEAN") == "BOOLEAN") + #expect(mapped("bytea") == "BLOB") + #expect(mapped("VARBINARY(50)") == "BLOB") } /// An unknown type is written as text rather than guessed at. A wrong guess writes a Parquet /// file whose column type disagrees with the data in it. @Test("An unknown type falls back to VARCHAR") func unknownFallsBack() { - #expect(ParquetTypeMapper.duckDBType(forColumnType: "geography") == "VARCHAR") - #expect(ParquetTypeMapper.duckDBType(forColumnType: "") == "VARCHAR") - #expect(ParquetTypeMapper.duckDBType(forColumnType: "hstore") == "VARCHAR") + #expect(mapped("geography") == "VARCHAR") + #expect(mapped("") == "VARCHAR") + #expect(mapped("hstore") == "VARCHAR") } /// A type name carries its width in parentheses and sometimes a modifier after a space, and @@ -211,7 +270,6 @@ struct ParquetTypeMapperTests { } struct PluginRowWritersTests { - /// The values in an export come from the database rather than from the person opening the /// file, so a value that a spreadsheet would run as a formula is neutralised. @Test("Formula leads are neutralised and the value is then quoted") diff --git a/TableProTests/Plugins/ParquetExportCancellationTests.swift b/TableProTests/Plugins/ParquetExportCancellationTests.swift new file mode 100644 index 0000000000..86a07155d0 --- /dev/null +++ b/TableProTests/Plugins/ParquetExportCancellationTests.swift @@ -0,0 +1,102 @@ +// +// ParquetExportCancellationTests.swift +// TableProTests +// + +import Foundation +import TableProPluginKit +import Testing + +struct ParquetExportCancellationTests { + @Test("Stopping between tables removes the files already written") + func stopBetweenTablesRemovesWrittenFiles() async throws { + let directory = try makeDirectory() + defer { try? FileManager.default.removeItem(at: directory) } + let destination = directory.appendingPathComponent("dump.parquet") + let progress = PluginExportProgress(progress: Progress(totalUnitCount: 0)) + var attempted: [String] = [] + + await #expect(throws: PluginExportCancellationError.self) { + try await ParquetTableFiles.write( + [table("users"), table("orders")], destination: destination, progress: progress + ) { table, fileURL in + attempted.append(table.name) + try Data("PAR1".utf8).write(to: fileURL) + progress.cancel() + } + } + + #expect(attempted == ["users"]) + #expect(!fileExists(for: "users", destination: destination)) + #expect(!fileExists(for: "orders", destination: destination)) + } + + @Test("A table that fails to write removes the files already written") + func failedTableRemovesWrittenFiles() async throws { + let directory = try makeDirectory() + defer { try? FileManager.default.removeItem(at: directory) } + let destination = directory.appendingPathComponent("dump.parquet") + let progress = PluginExportProgress(progress: Progress(totalUnitCount: 0)) + + await #expect(throws: PluginExportError.self) { + try await ParquetTableFiles.write( + [table("users"), table("orders")], destination: destination, progress: progress + ) { table, fileURL in + guard table.name == "users" else { throw PluginExportError.exportFailed("orders") } + try Data("PAR1".utf8).write(to: fileURL) + } + } + + #expect(!fileExists(for: "users", destination: destination)) + } + + @Test("A finished multi-table export keeps one file per table") + func finishedExportKeepsEveryFile() async throws { + let directory = try makeDirectory() + defer { try? FileManager.default.removeItem(at: directory) } + let destination = directory.appendingPathComponent("dump.parquet") + let progress = PluginExportProgress(progress: Progress(totalUnitCount: 0)) + + try await ParquetTableFiles.write( + [table("users"), table("orders")], destination: destination, progress: progress + ) { _, fileURL in + try Data("PAR1".utf8).write(to: fileURL) + } + + #expect(fileExists(for: "users", destination: destination)) + #expect(fileExists(for: "orders", destination: destination)) + } + + @Test("A single table is written to the destination itself") + func singleTableWritesToDestination() async throws { + let directory = try makeDirectory() + defer { try? FileManager.default.removeItem(at: directory) } + let destination = directory.appendingPathComponent("dump.parquet") + let progress = PluginExportProgress(progress: Progress(totalUnitCount: 0)) + var targets: [URL] = [] + + try await ParquetTableFiles.write( + [table("users")], destination: destination, progress: progress + ) { _, fileURL in + targets.append(fileURL) + } + + #expect(targets == [destination]) + } + + private func table(_ name: String) -> PluginExportTable { + PluginExportTable(name: name, databaseName: "main", tableType: "table") + } + + private func makeDirectory() throws -> URL { + let directory = FileManager.default.temporaryDirectory + .appendingPathComponent("parquet-export-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + return directory + } + + private func fileExists(for table: String, destination: URL) -> Bool { + let url = ParquetFileNaming.perTableURL(destination: destination, table: table) + return FileManager.default.fileExists(atPath: url.path(percentEncoded: false)) + } +} diff --git a/project.yml b/project.yml index 8ed901d979..84458502ba 100644 --- a/project.yml +++ b/project.yml @@ -540,6 +540,7 @@ targets: - Plugins/JSONExportPlugin/JSONExportModels.swift - Plugins/MarkdownExportPlugin/MarkdownExportModels.swift - Plugins/ParquetExportPlugin/ParquetExportModels.swift + - Plugins/ParquetExportPlugin/ParquetTableFiles.swift - Plugins/ParquetExportPlugin/ParquetTypeMapper.swift - Plugins/JSONImportPlugin/JSONImportOptions.swift - Plugins/JSONImportPlugin/JSONImportOptionsView.swift