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

uuid #405

Closed
wants to merge 1 commit into from
Closed

uuid #405

wants to merge 1 commit into from

Conversation

agnes512
Copy link
Contributor

@agnes512 agnes512 commented Dec 11, 2019

fixes loopbackio/loopback-next#2398
Duplicate of #404 because Jenkins fails randomly...

please review it with whitespace hided

loopbackio/loopback-next#2398 is partially fixed in loopbackio/loopback-datasource-juggler#1783.

@model()
export class Customer extends Entity {
  @property({
    type: 'string',
    id: true,
    useDefaultIdType: false,
    generated: true,
  })
  custid: string;

Issue

The flag useDefaultIdType: false is supposed to allow to use custom type of auto-generated id property. But it only allows the default type, and it won't generated the custom id type itself as different DBs have different syntax/setup.

For example, Postgres and MySQL default identity type is INT. And they can be auto-generated by SERIAL and AUTO_INCREAMENT respectively.

The model definition above won't work on Postgres and MySQL because these two connectors only support to auto-generated the default id type ( integer in this case).

From #3602 what suggests and real world usage, I am proposing to add uuid type to data type, and also add support for auto-generated string type id to Postgres connector:

Proposal

  • Similar to mongoDB, when property type is uuid and auto-generated, by default, use uuid-ossp extension and uuid_generate_v4() function to allow auto-generated string.
  @property({
    id: true,
    type: 'string',
    generated: true,
    useDefaultIdType: false, // this is needed if this property is id
    postgresql: {
      dataType: 'uuid',
    },
  })
  id: String;

when id type is string and auto-generated, treat it as uuid. ( Should we?)

  • If user would like use other extension & function, they can do:
  @property({
    id: true,
    type: 'string',
    generated: true,
    useDefaultIdType: false, // this is needed if this property is id
    postgresql: {
      dataType: 'uuid',
      defaultFn: 'example_function',
      extension: 'my_extension'
    },
  })
  id: String;

However, it is user's responsibility to provide valid extension and function.

  • If user would like to use other type as id type and make it auto-generated, should warn them that LoopBack doesn't support to auto-generate such type for Postgres. They need to do ALTER TABLE to set up the table manually.
  • Document the above setup rules, options, and limitations in both LB3 and LB4

If the above is okay, will fix MySQL as well.

Checklist

👉 Read and sign the CLA (Contributor License Agreement) 👈

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • Commit messages are following our guidelines

@agnes512
Copy link
Contributor Author

@slnode test please

1 similar comment
@agnes512
Copy link
Contributor Author

@slnode test please

@agnes512 agnes512 force-pushed the uuid branch 6 times, most recently from 8bc09bf to 5815f28 Compare December 13, 2019 18:45
@agnes512
Copy link
Contributor Author

@slnode test please

@agnes512 agnes512 force-pushed the uuid branch 2 times, most recently from 5c95421 to 36b2dc6 Compare December 13, 2019 20:52
@agnes512
Copy link
Contributor Author

@slnode test please

2 similar comments
@agnes512
Copy link
Contributor Author

@slnode test please

@rmg
Copy link
Contributor

rmg commented Dec 14, 2019

@slnode test please

@agnes512 agnes512 force-pushed the uuid branch 3 times, most recently from d2fefcf to b8a1f80 Compare December 16, 2019 15:24
@agnes512
Copy link
Contributor Author

@slnode test please

1 similar comment
@agnes512
Copy link
Contributor Author

@slnode test please

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 this pull request may close these issues.

Column is always integer when running npm run migrate
2 participants