Skip to content

Conversation

fungiboletus
Copy link

No description provided.

@slnode
Copy link

slnode commented Mar 25, 2015

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

@fungiboletus fungiboletus changed the title Use TEXT instead of VARCHAR Changes in PostgreSQL datatypes Mar 25, 2015
@bajtos
Copy link
Member

bajtos commented Apr 16, 2015

@slnode ok to test

@bajtos
Copy link
Member

bajtos commented Apr 16, 2015

@yellowiscool thank you for the pull request. I am not familiar enough with this project, I'll let @raymondfeng to review your patch.

However, I would like to point out a possible backwards compatibility issue. IIUC, this change is forcing all users of the PostgreSQL connector to modify their database schema to use these new types, at least when they run auto-update/auto-migrate script.

Perhaps we can add a flag (connector/datasource setting) to allow the users to decide whether they want the old or the new mapping?

@kadim
Copy link

kadim commented May 13, 2015

Why not JSONB? It can be indexed!

@TinOo512
Copy link

@raymondfeng can we have update on this ? If I read the postgresql documentation (http://www.postgresql.org/docs/9.1/static/datatype-character.html) there is no benefits to use character varying with a specified length.

Tip: There is no performance difference among these three types, apart from increased storage space when using the blank-padded type, and a few extra CPU cycles to check the length when storing into a length-constrained column. While character(n) has performance advantages in some other database systems, there is no such advantage in PostgreSQL; in fact character(n) is usually the slowest of the three because of its additional storage costs. In most situations text or character varying should be used instead.

It will also fix those issues : strongloop/loopback-component-passport#84 #122 with loopback-component-passport and probably others, ...

@kadim For the jsonb it will also be a major improvement but we need some other change to be able to look throw the json to perform some filtering operation (@aol-nnov start the work here : #59)

@slnode
Copy link

slnode commented Jan 14, 2016

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

@tobernguyen
Copy link

Why this pull request take so long? Please review and accept this so this problem will be fixed.
strongloop/loopback-component-passport#84

@slnode
Copy link

slnode commented May 15, 2016

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

@superkhau
Copy link
Contributor

Can you add some tests to verify your changes and prevent regressions in the future?

@archenroot
Copy link

Not sure if relevant to put here, but as it refers to new data types, better do, we have also hstore now, similar to redis key-value fundamentally:
#232

@slnode
Copy link

slnode commented Apr 2, 2017

Can one of the admins verify this patch?

2 similar comments
@slnode
Copy link

slnode commented Apr 2, 2017

Can one of the admins verify this patch?

@slnode
Copy link

slnode commented Apr 2, 2017

Can one of the admins verify this patch?

@stale stale bot added the stale label Aug 22, 2017
@stale
Copy link

stale bot commented Aug 22, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot closed this Sep 5, 2017
@stale
Copy link

stale bot commented Sep 5, 2017

This issue has been closed due to continued inactivity. Thank you for your understanding. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment