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

CITEXT type is understood as Slice instead of String in 0.7.1 #43

Closed
spalladino opened this issue May 26, 2016 · 10 comments · Fixed by #221
Closed

CITEXT type is understood as Slice instead of String in 0.7.1 #43

spalladino opened this issue May 26, 2016 · 10 comments · Fixed by #221

Comments

@spalladino
Copy link

In version cafe4748b64a9c95ab93b4c19315780e8e254291, querying a CITEXT type field would return the result as String, so the following returned the expected result:

DB.exec({String}, "SELECT name FROM users").rows[0][0]

As of 0.7.1, the code above started throwing cast to String failed (TypeCastError), though the following did work and returned the same value:

String.new(DB.exec({Slice}, "SELECT name FROM users").rows[0][0])

It seems that a change in the decoder caused CITEXT fields to stop being understood as actual Strings.

Tested with postgresql 9.4.7.

spalladino added a commit to instedd/mvam-chatbot that referenced this issue May 26, 2016
@will
Copy link
Owner

will commented May 26, 2016

Before types that were unknown to the driver would be returned as strings, but I switched that to return the raw bytes because it is safer.

Unfortunately because citext is an extension, the oid changes from install to install, so to support that type needs a bit of upcoming work that maps names to decoders (and probably per connection, but maybe not (extension types might not actually need it per connection but enums might)).

There could possibly be a work around to make {String} work, by doing something like

class Object
  def _pg_as(kl)
    self.as(kl)
  end
end

class Slice
  def _pg_as(kl)
    if T.is_a?(UInt8) && kl == String
      String.new(self) 
   else
      self.as(kl)
   end
end

but that might not work and is a bit gross

@spalladino
Copy link
Author

How about modifying how a result invokes each decoder? I'm thinking about adding a 3rd parameter to decode which is the requested type, so a DefaultDecoder can rely on that information to choose to cast the current value to that particular type or perform a certain transformation, rather than blindly invoking type.cast. So, requesting a byte[] type as a string, would run String.new(slice) on the decoded value.

The change would impact on https://github.com/will/crystal-pg/blob/master/src/pg/result.cr#L55 and https://github.com/will/crystal-pg/blob/master/src/pg/result.cr#L70 I guess.

However, I'm not sure if this ends up being more work than actually supporting extensions and enums as you say. What do you think?

@asterite
Copy link
Contributor

When we'll make crystal-pg implement crystal-db one will be able to invoke read(String) on the result set, and there the driver can check, if the result type is actually a Slice(UInt8) then it could return a string from it.

@will will mentioned this issue Oct 31, 2016
@grantspeelman
Copy link

grantspeelman commented Mar 3, 2019

Hi there
Getting the following Error when using Crecto and a CITEXT field.
PG::ResultSet#read returned a Slice(UInt8). A (String | Nil) was expected. (Exception)

Is this the same issue?
Thx

@vladfaust
Copy link
Contributor

vladfaust commented Mar 3, 2019

@grantspeelman if you're reading the result with rs.read(String?) then you should switch to rs.read(Bytes). I guess it's Crecto's bug

@straight-shoota
Copy link
Contributor

@grantspeelman Yes, that seems to be the same issue.

I'm not familiar with crecto and how that plays into this, or what can be done to make this work with crecto.

But for plain crystal-pg there are essentially two workarounds:

  • Either change the SQL query to cast the value to text (SELECT cicolumn::text)
  • Or read a Bytes from the result set and explicitly convert that to a string using String.new(rs.read(Bytes)).

@grantspeelman
Copy link

grantspeelman commented Mar 5, 2019

thank you @straight-shoota and @vladfaust

@andrius
Copy link

andrius commented Sep 16, 2019

It looks that I get into similar or same issue.

I get PG::ResultSet#read returned a Slice(UInt8). A (String | Nil) was expected. (Exception) when call query_one or query_all against following table (snippet below).

There is no issues writing data, it accept it as a string and works perfectly

DB table:

CREATE TYPE moh_mode_values AS ENUM ('custom', 'files', 'mp3nb', 'quietmp3nb', 'quietmp3');

CREATE TABLE musiconhold (
    name VARCHAR(80) NOT NULL,
    mode moh_mode_values,
    directory VARCHAR(255),
    application VARCHAR(255),
    digit VARCHAR(1),
    sort VARCHAR(10),
    format VARCHAR(10),
    stamp TIMESTAMP WITHOUT TIME ZONE,
    PRIMARY KEY (name)
);

Model file:

struct MusicOnHold
  # CREATE TYPE moh_mode_values AS ENUM ('custom', 'files', 'mp3nb', 'quietmp3nb', 'quietmp3
  ::DB.mapping({
    name:        {type: String, nilable: false},
    mode:        {type: String, nilable: true, default: ""}, # mode moh_mode_values
    directory:   {type: String, nilable: true, default: ""},
    application: {type: String, nilable: true, default: ""},
    digit:       {type: String, nilable: true, default: ""},
    sort:        {type: String, nilable: true, default: ""},
    format:      {type: String, nilable: true, default: ""}
  })

  JSON.mapping({
    name:        {type: String, nilable: false},
    mode:        {type: String, nilable: true, default: ""}, # mode moh_mode_values
    directory:   {type: String, nilable: true, default: ""},
    application: {type: String, nilable: true, default: ""},
    digit:       {type: String, nilable: true, default: ""},
    sort:        {type: String, nilable: true, default: ""},
    format:      {type: String, nilable: true, default: ""}
  })

  def self.find_by(name = nil)
    query = "select * from musiconhold where name = $1 limit 1"
    # here I get a bug 
    # PG::ResultSet#read returned a Slice(UInt8). A (String | Nil) was expected. (Exception) 
    moh = Database.connection.query_one(query, name, as: MusicOnHold)

@straight-shoota
Copy link
Contributor

straight-shoota commented Sep 16, 2019

Yes, enum data type values have the same issue as citext. And the same workarounds apply as described in #43 (comment)

@straight-shoota
Copy link
Contributor

To move this on: I think there are two aspects to this problem:

  • A: Calling rs.read(String) on a string-like column (citext, enum or similar), it should return a String.
  • B: Calling rs.read on a string-like column, it should probably also return a String.

A should be relatively easy to resolve. When the driver encounters an unknown oid, it could just select the decoder based on the requested return type. Thus a citext would be decoded with StringDecoder. This would only require to override ResultSet#read(type : T.class) in PG::ResultSet.

B is more complicated, because it requires an understanding of server-specific oids. These would need to be requested when the connection is established and mapped by name to the respective decoder. npgsql for example has an extensive table of type mappings and operates that way.
This should also provide a simple API to register named types, for example custom enum types.

A could be a quick fix, but B is the more complete solution and if we implement B there is probably no need for A.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants