Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

char(n) (BPCHAR) length fix #72

Merged
merged 2 commits into from
Feb 12, 2020
Merged

char(n) (BPCHAR) length fix #72

merged 2 commits into from
Feb 12, 2020

Conversation

tanner0101
Copy link
Member

@tanner0101 tanner0101 commented Dec 20, 2019

The char(n) (BPCHAR) data type now correctly supports values longer than one byte when converting to fixed-width integers or strings (fixes #71, fixes #79, #72).

@tanner0101 tanner0101 added the bug Something isn't working label Dec 20, 2019
@tkrajacic
Copy link
Sponsor Contributor

tkrajacic commented Dec 20, 2019

This now crashes in the PostgresDataDecoder.swift on

return convertible.init(postgresData: data)! as! T

with

NIO-ELT-0-#0 (3): Fatal error: Unexpectedly found nil while unwrapping an Optional value

(lldb) po data
▿ 0x0020 (BPCHAR)
  ▿ type : BPCHAR
    - rawValue : 1042
  ▿ typeModifier : Optional<Int32>
    - some : 5
  - formatCode : binary
  ▿ value : Optional<ByteBuffer>
    ▿ some : ByteBuffer { readerIndex: 0, writerIndex: 2, readableBytes: 2, capacity: 2, slice: _ByteBufferSlice { 1453..<1455 }, storage: 0x00007b7000011000 (2048 bytes) }
      ▿ _storage : <_Storage: 0x7b100006d7c0>
      - _readerIndex : 0
      - _writerIndex : 2
      ▿ _slice : _ByteBufferSlice { 1453..<1455 }
        - upperBound : 1455
        ▿ _begin : 1453
          - b12 : 1453
          - b3 : 0

The actual type it crashes on is now

public enum Subscription: Int8, Equatable, Hashable, Codable, CaseIterable {
    case unknown = 0
    case subscribed = 1
    case unsubscribed = 2
}

with

extension Subscription: DatabaseTypeRepresentable {
    public static var databaseType: String { "CHARACTER" }
}

public protocol DatabaseTypeRepresentable {
    static var databaseType: String { get }
}

extension Subscription: PostgresDataConvertible { }

/cc @calebkleveter

@tanner0101
Copy link
Member Author

tanner0101 commented Dec 20, 2019

@tkrajacic can you try CHAR instead of CHARACTER?

From https://dba.stackexchange.com/questions/159090/how-to-store-one-byte-integer-in-postgresql

"char" is an "internal" type intended for simple and cheap enumeration.

Either way, the force cast should be fixed obviously. That's in postgres-kit, right?

@tkrajacic
Copy link
Sponsor Contributor

As it turns out, Postgres has no 1-byte type really, and even a character(1) uses 2 bytes.

See https://www.postgresql.org/docs/current/datatype-character.html

The storage requirement for a short string (up to 126 bytes) is 1 byte plus the actual string, which includes the space padding in the case of character

There would be an internal type of "CHAR" (including the quotes) but this is not adhering to any standard

The type "char" (note the quotes) is different from char(1) in that it only uses one byte of storage. It is internally used in the system catalogs as a simplistic enumeration type.

@tanner0101 tanner0101 changed the title char(n) / bpchar fix char(n) (BPCHAR) length fix Feb 12, 2020
@tanner0101
Copy link
Member Author

Merging this since it at least fixes #79. Solutions for Postgres' lack of a good "single byte" type can be discussed further.

@tanner0101 tanner0101 merged commit 0206f13 into master Feb 12, 2020
@tanner0101
Copy link
Member Author

These changes are now available in 1.0.0-beta.2.8

@tanner0101 tanner0101 deleted the tn-charn-fix branch February 12, 2020 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants