From 8e31e66228a851ab3d508e5655f4f41886971ab6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zsolt=20Va=CC=81radi?= Date: Sat, 10 Jun 2017 17:37:37 +0200 Subject: [PATCH 1/5] Add failing test for numeric parsing (average) --- Tests/PostgreSQLTests/PostgreSQLTests.swift | 48 +++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/Tests/PostgreSQLTests/PostgreSQLTests.swift b/Tests/PostgreSQLTests/PostgreSQLTests.swift index 7109c62..ba347c6 100644 --- a/Tests/PostgreSQLTests/PostgreSQLTests.swift +++ b/Tests/PostgreSQLTests/PostgreSQLTests.swift @@ -304,6 +304,54 @@ class PostgreSQLTests: XCTestCase { } } + func testNumericAverage() throws { + let conn = try postgreSQL.makeConnection() + + try conn.execute("DROP TABLE IF EXISTS jobs") + try conn.execute("CREATE TABLE jobs (id SERIAL PRIMARY KEY NOT NULL, title VARCHAR(255) NOT NULL, pay INT8 NOT NULL)") + + try conn.execute("INSERT INTO jobs (title, pay) VALUES ($1, $2)", ["A", 100]) + try conn.execute("INSERT INTO jobs (title, pay) VALUES ($1, $2)", ["A", 200]) + try conn.execute("INSERT INTO jobs (title, pay) VALUES ($1, $2)", ["A", 300]) + try conn.execute("INSERT INTO jobs (title, pay) VALUES ($1, $2)", ["A", 400]) + try conn.execute("INSERT INTO jobs (title, pay) VALUES ($1, $2)", ["A", 500]) + try conn.execute("INSERT INTO jobs (title, pay) VALUES ($1, $2)", ["B", 100]) + try conn.execute("INSERT INTO jobs (title, pay) VALUES ($1, $2)", ["B", 200]) + try conn.execute("INSERT INTO jobs (title, pay) VALUES ($1, $2)", ["B", 900]) + try conn.execute("INSERT INTO jobs (title, pay) VALUES ($1, $2)", ["C", 100]) + try conn.execute("INSERT INTO jobs (title, pay) VALUES ($1, $2)", ["C", 1100]) + try conn.execute("INSERT INTO jobs (title, pay) VALUES ($1, $2)", ["D", 100]) + try conn.execute("INSERT INTO jobs (title, pay) VALUES ($1, $2)", ["E", 500]) + + defer { + _ = try? conn.execute("DROP TABLE IF EXISTS jobs") + } + + let result = try conn.execute("select title, avg(pay) as average, count(*) as cnt from jobs group by title order by average desc") + + guard let array = result.array else { + XCTFail("Result was not an array") + return + } + + XCTAssertEqual(5, array.count) + XCTAssertEqual("C", array[0]["title"]?.string) + XCTAssertEqual("E", array[1]["title"]?.string) + XCTAssertEqual("B", array[2]["title"]?.string) + XCTAssertEqual("A", array[3]["title"]?.string) + XCTAssertEqual("D", array[4]["title"]?.string) + XCTAssertEqual(2, array[0]["cnt"]?.int) + XCTAssertEqual(1, array[1]["cnt"]?.int) + XCTAssertEqual(3, array[2]["cnt"]?.int) + XCTAssertEqual(5, array[3]["cnt"]?.int) + XCTAssertEqual(1, array[4]["cnt"]?.int) + XCTAssertEqual(600, array[0]["average"]?.double) + XCTAssertEqual(500, array[1]["average"]?.double) + XCTAssertEqual(400, array[2]["average"]?.double) + XCTAssertEqual(300, array[3]["average"]?.double) + XCTAssertEqual(100, array[4]["average"]?.double) + } + func testJSON() throws { let conn = try postgreSQL.makeConnection() From 5b6ee59efd10f137bfd419babf07b9045e18d957 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zsolt=20Va=CC=81radi?= Date: Sat, 10 Jun 2017 19:38:34 +0200 Subject: [PATCH 2/5] Add missing test reference --- Tests/PostgreSQLTests/PostgreSQLTests.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Tests/PostgreSQLTests/PostgreSQLTests.swift b/Tests/PostgreSQLTests/PostgreSQLTests.swift index ba347c6..55d35a2 100644 --- a/Tests/PostgreSQLTests/PostgreSQLTests.swift +++ b/Tests/PostgreSQLTests/PostgreSQLTests.swift @@ -13,6 +13,7 @@ class PostgreSQLTests: XCTestCase { ("testInts", testInts), ("testFloats", testFloats), ("testNumeric", testNumeric), + ("testNumericAverage", testNumericAverage), ("testJSON", testJSON), ("testIntervals", testIntervals), ("testPoints", testPoints), From ce8910f1d48d0dedea5ad165e0e52cc5a5d93f96 Mon Sep 17 00:00:00 2001 From: Steven Roebert Date: Thu, 15 Jun 2017 11:18:28 +0200 Subject: [PATCH 3/5] Fixed numeric parsing by checking the weigth parameter; Added more unit tests for numeric parsing --- Sources/PostgreSQL/Bind/BinaryUtils.swift | 50 ++++++++++++-------- Tests/PostgreSQLTests/BinaryUtilsTests.swift | 4 ++ 2 files changed, 34 insertions(+), 20 deletions(-) diff --git a/Sources/PostgreSQL/Bind/BinaryUtils.swift b/Sources/PostgreSQL/Bind/BinaryUtils.swift index 675907d..2242412 100644 --- a/Sources/PostgreSQL/Bind/BinaryUtils.swift +++ b/Sources/PostgreSQL/Bind/BinaryUtils.swift @@ -167,15 +167,21 @@ struct BinaryUtils { return "0" } + let weight = Int(parseInt16(value: value.advanced(by: 2))) let dscale = Int(parseInt16(value: value.advanced(by: 6))) + var digitIndex = 0 - // Add all digits to a string var number: String = "" - for i in 0..= 0 && index < numberOfDigits { + int16 = parseInt16(value: value.advanced(by: 8 + index * 2)) + } else { + int16 = 0 + } let stringDigits = String(int16) - if i == 0 { + if index == 0 { number += stringDigits } else { @@ -185,25 +191,29 @@ struct BinaryUtils { } } - if dscale > 0 { - // Make sure we have enough decimal digits by pre-padding with zeros - if number.characters.count < dscale { - number = String(repeating: "0", count: dscale - number.characters.count) + number - } - else { - // Remove any trailing zeros - while number.hasSuffix("0") { - number.remove(at: number.index(number.endIndex, offsetBy: -1)) - } + // Add all digits before the decimal point + if weight < 0 { + digitIndex = weight + 1 + number += "0" + } else { + while digitIndex <= weight { + addDigit(atIndex: digitIndex) + digitIndex += 1 } + } + + // Add digits after decimal point + if dscale > 0 { + number += "." + let decimalIndex = number.endIndex - // Insert decimal point - number.insert(".", at: number.index(number.endIndex, offsetBy: -dscale)) - - // If we have at least a zero before the decimal point - if dscale == number.characters.count - 1 { - number = "0"+number + for _ in stride(from: 0, to: dscale, by: 4) { + addDigit(atIndex: digitIndex) + digitIndex += 1 } + + let endIndex = number.index(decimalIndex, offsetBy: dscale + 1) + number = number.substring(to: endIndex) } // Make number negative if necessary diff --git a/Tests/PostgreSQLTests/BinaryUtilsTests.swift b/Tests/PostgreSQLTests/BinaryUtilsTests.swift index bc92f6a..680a8bc 100644 --- a/Tests/PostgreSQLTests/BinaryUtilsTests.swift +++ b/Tests/PostgreSQLTests/BinaryUtilsTests.swift @@ -184,9 +184,13 @@ class BinaryUtilsTests: XCTestCase { ("0000000000000000", "0"), ("00010000000000000001", "1"), ("00010000400000000001", "-1"), + ("00010000000000020001", "1.00"), + ("00010000400000050001", "-1.00000"), ("00000000c0000000", "NaN"), ("0006000200000009000109291a8504d2162e2328", "123456789.123456789"), ("0006000340000006270f270f270f270f270f26ac", "-9999999999999999.999999"), + ("000600020000000a000109291a8504d2162e2328", "123456789.1234567890"), + ("0006000340000009270f270f270f270f270f26ac", "-9999999999999999.999999000"), ("0001000000000000007b", "123"), ("0002ffff0000000526941388", "0.98765"), ("0002ffff4000000526941388", "-0.98765"), From 377e1e155ad6db36df1af78d1b59c3cf5a4410fd Mon Sep 17 00:00:00 2001 From: Steven Roebert Date: Thu, 15 Jun 2017 14:10:32 +0200 Subject: [PATCH 4/5] Added rounding for numeric type and added unit tests for this --- Sources/PostgreSQL/Bind/BinaryUtils.swift | 206 ++++++++++++++----- Tests/PostgreSQLTests/BinaryUtilsTests.swift | 2 + 2 files changed, 151 insertions(+), 57 deletions(-) diff --git a/Sources/PostgreSQL/Bind/BinaryUtils.swift b/Sources/PostgreSQL/Bind/BinaryUtils.swift index 2242412..683cfb2 100644 --- a/Sources/PostgreSQL/Bind/BinaryUtils.swift +++ b/Sources/PostgreSQL/Bind/BinaryUtils.swift @@ -145,83 +145,175 @@ struct BinaryUtils { return Float64(bigEndian: convert(value)) } - // MARK: - Numberic - - struct NumericConstants { - static let signNaN: Int16 = -16384 - static let signNegative: Int16 = 16384 - static let decDigits = 4 - } - - static func parseNumeric(value: UnsafeMutablePointer) -> String { - let sign = parseInt16(value: value.advanced(by: 4)) - - // Check for NaN - guard sign != NumericConstants.signNaN else { - return "NaN" - } - - // Check that we actually have some digits - let numberOfDigits = Int(parseInt16(value: value)) - guard numberOfDigits > 0 else { - return "0" + // MARK: - Numeric + + struct Numeric { + private static let signNaN: Int16 = -16384 + private static let signNegative: Int16 = 16384 + private static let decDigits = 4 + private static let NBASE: Int16 = 10000 + private static let halfNBASE: Int16 = 5000 + private static let roundPowers: [Int16] = [0, 1000, 100, 10] + + var sign: Int16 + var weight: Int + var dscale: Int + var numberOfDigits: Int + var digits: [Int16] + + init(value: UnsafeMutablePointer) { + sign = BinaryUtils.parseInt16(value: value.advanced(by: 4)) + weight = Int(BinaryUtils.parseInt16(value: value.advanced(by: 2))) + + var dscale = Int(BinaryUtils.parseInt16(value: value.advanced(by: 6))) + if dscale < 0 { + dscale = 0 + } + self.dscale = dscale + + numberOfDigits = Int(BinaryUtils.parseInt16(value: value)) + digits = (0.. String { let int16: Int16 if index >= 0 && index < numberOfDigits { - int16 = parseInt16(value: value.advanced(by: 8 + index * 2)) + int16 = digits[index] } else { int16 = 0 } let stringDigits = String(int16) - if index == 0 { - number += stringDigits - } - else { - // The number of digits should be 4 (DEC_DIGITS), - // so pad if necessary. - number += String(repeating: "0", count: 4 - stringDigits.characters.count) + stringDigits + guard index != 0 else { + return stringDigits } + + // The number of digits should be 4 (DEC_DIGITS), + // so pad if necessary. + return String(repeating: "0", count: Numeric.decDigits - stringDigits.characters.count) + stringDigits } - // Add all digits before the decimal point - if weight < 0 { - digitIndex = weight + 1 - number += "0" - } else { - while digitIndex <= weight { - addDigit(atIndex: digitIndex) - digitIndex += 1 + mutating func roundIfNeeded() { + // Decimal digits wanted + var totalDigits = (weight + 1) * Numeric.decDigits + dscale + + // If less than 0, result should be 0 + guard totalDigits >= 0 else { + digits = [] + weight = 0 + sign = 0 + return + } + + // NBASE digits wanted + var nbaseDigits = (totalDigits + Numeric.decDigits - 1) / Numeric.decDigits + + // 0, or number of decimal digits to keep in last NBASE digit + totalDigits = totalDigits % Numeric.decDigits + + guard nbaseDigits < numberOfDigits || (nbaseDigits == numberOfDigits && totalDigits > 0) else { + return + } + + numberOfDigits = nbaseDigits + + var carry: Int16 + if totalDigits == 0 { + carry = digits[0] >= Numeric.halfNBASE ? 1 : 0 + } else { + nbaseDigits -= 1 + + // Must round within last NBASE digit + var pow10 = Numeric.roundPowers[totalDigits] + let extra = digits[nbaseDigits] % pow10 + digits[nbaseDigits] = digits[nbaseDigits] - extra + + carry = 0 + if extra >= pow10 / 2 { + pow10 += digits[nbaseDigits] + if pow10 >= Numeric.NBASE { + pow10 -= Numeric.NBASE + carry = 1 + } + digits[nbaseDigits] = pow10 + } + } + + // Propagate carry if needed + while carry > 0 { + nbaseDigits -= 1 + if nbaseDigits < 0 { + digits.insert(0, at: 0) + nbaseDigits = 0 + + numberOfDigits += 1 + weight += 1 + } + + carry += digits[nbaseDigits] + + if carry >= Numeric.NBASE { + digits[nbaseDigits] = carry - Numeric.NBASE + carry = 1 + } else { + digits[nbaseDigits] = carry + carry = 0 + } } } - // Add digits after decimal point - if dscale > 0 { - number += "." - let decimalIndex = number.endIndex + var string: String { + // Check for NaN + guard sign != Numeric.signNaN else { + return "NaN" + } + + guard !digits.isEmpty else { + return "0" + } + + var digitIndex = 0 + var string: String = "" + + // Make number negative if necessary + if sign == Numeric.signNegative { + string += "-" + } + + // Add all digits before decimal point + if weight < 0 { + digitIndex = weight + 1 + string += "0" + } else { + while digitIndex <= weight { + string += getDigit(atIndex: digitIndex) + digitIndex += 1 + } + } + + guard dscale > 0 else { + return string + } + + // Add digits after decimal point + string += "." + let decimalIndex = string.endIndex - for _ in stride(from: 0, to: dscale, by: 4) { - addDigit(atIndex: digitIndex) + for _ in stride(from: 0, to: dscale, by: Numeric.decDigits) { + string += getDigit(atIndex: digitIndex) digitIndex += 1 } - let endIndex = number.index(decimalIndex, offsetBy: dscale + 1) - number = number.substring(to: endIndex) + let endIndex = string.index(decimalIndex, offsetBy: dscale + 1) + string = string.substring(to: endIndex) + return string } - - // Make number negative if necessary - if sign == NumericConstants.signNegative { - number = "-"+number - } - - return number + } + + static func parseNumeric(value: UnsafeMutablePointer) -> String { + var numeric = Numeric(value: value) + numeric.roundIfNeeded() + return numeric.string } // MARK: - Date / Time diff --git a/Tests/PostgreSQLTests/BinaryUtilsTests.swift b/Tests/PostgreSQLTests/BinaryUtilsTests.swift index 680a8bc..aea8d3f 100644 --- a/Tests/PostgreSQLTests/BinaryUtilsTests.swift +++ b/Tests/PostgreSQLTests/BinaryUtilsTests.swift @@ -194,6 +194,8 @@ class BinaryUtilsTests: XCTestCase { ("0001000000000000007b", "123"), ("0002ffff0000000526941388", "0.98765"), ("0002ffff4000000526941388", "-0.98765"), + ("0002ffff0000000426941388", "0.9877"), + ("0002ffff0000000026941388", "1"), ] for (hexString, numericString) in numericTests { From 408600ce2c5f868eecb8ba39ab0f2214ab6e574f Mon Sep 17 00:00:00 2001 From: Steven Roebert Date: Thu, 15 Jun 2017 14:18:35 +0200 Subject: [PATCH 5/5] Added comment indicating the source of rounding code --- Sources/PostgreSQL/Bind/BinaryUtils.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Sources/PostgreSQL/Bind/BinaryUtils.swift b/Sources/PostgreSQL/Bind/BinaryUtils.swift index 683cfb2..13c7387 100644 --- a/Sources/PostgreSQL/Bind/BinaryUtils.swift +++ b/Sources/PostgreSQL/Bind/BinaryUtils.swift @@ -193,6 +193,8 @@ struct BinaryUtils { return String(repeating: "0", count: Numeric.decDigits - stringDigits.characters.count) + stringDigits } + /// Function for rounding numeric values. + /// The code is based on https://github.com/postgres/postgres/blob/3a0d473192b2045cbaf997df8437e7762d34f3ba/src/backend/utils/adt/numeric.c#L8594 mutating func roundIfNeeded() { // Decimal digits wanted var totalDigits = (weight + 1) * Numeric.decDigits + dscale