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

Column default value as function #150

Closed
CocaColaBear opened this issue Dec 25, 2016 · 9 comments
Closed

Column default value as function #150

CocaColaBear opened this issue Dec 25, 2016 · 9 comments
Labels

Comments

@CocaColaBear
Copy link

I have column Id:

@PrimaryColumn({
    name: "id",
    type: "bigint",
    default: new_id() // new_id() is function in DB (postgresql)
  })
  Id: number;

In database i have function: new_id() for generate ids. How i can implement this query?
id bigint NOT NULL DEFAULT new_id()

@pleerock
Copy link
Member

@PrimaryColumn({
    name: "id",
    type: "bigint",
    default: "new_id()"
  })
  Id: number;

Doesnt it work?

@CocaColaBear
Copy link
Author

@pleerock not work
Error:

{ error: invalid input syntax for integer: "new_id()"
    at Connection.parseE (C:\TS\Test\node_modules\pg\lib\connection.js:554:11)
    at Connection.parseMessage (C:\TS\Test\node_modules\pg\lib\connection.js:381:17)
    at Socket.<anonymous> (C:\TS\Test\node_modules\pg\lib\connection.js:117:22)
    at emitOne (events.js:96:13)
    at Socket.emit (events.js:188:7)
    at readableAddChunk (_stream_readable.js:176:18)
    at Socket.Readable.push (_stream_readable.js:134:10)
    at TCP.onread (net.js:551:20)
  name: 'error',
  length: 94,
  severity: 'ERROR',
  code: '22P02',
  detail: undefined,
  hint: undefined,
  position: undefined,
  internalPosition: undefined,
  internalQuery: undefined,
  where: undefined,
  schema: undefined,
  table: undefined,
  column: undefined,
  dataType: undefined,
  constraint: undefined,
  file: 'int8.c',
  line: '99',
  routine: 'scanint8' }

@pleerock
Copy link
Member

mmm probably because of escaping... Probably we can add another option like preventDefaultEscaping: boolean, but I would like to hear better alternative @eduardoweiland @19majkel94 do you have any insights about this issue?

@CocaColaBear
Copy link
Author

@pleerock mb it is dirty, but we can check default value : function or not (regexp or another way).
But option 'preventDefaultEscaping' looks nice. I can make pull request with this option

@pleerock
Copy link
Member

function or not (regexp or another way)

can we cover all cases with such regexp?

I would like to hear something better then preventDefaultEscaping, maybe other guys have some opinions before you will make a PR with dirty preventDefaultEscaping option.

@eduardoweiland
Copy link
Contributor

I have an idea, but maybe it's too much complex. Wrap the values that are sent to the database in some class, like EscapedValue and RawSql. These classes would hold the logic to convert the value to be sent to the database. I don't know how this would work with multiple drivers, but it's a starting point. What do you think?

@MichalLytek
Copy link
Contributor

MichalLytek commented Dec 26, 2016

5000 option and parameters makes the decorators code big and gross. I would preffer simple solutions like &, @, =, < in AngularJS or in template engines like pug.

I don't know how about TypeORM v0.0.6 but the older version required quotes in string for default values:

@Column("string", { default: "'normal'" })

so we could also make this part of code more inteligent - if field is string, wrap argument in quotes in sql, if number no, etc.

My proposition are twos:

  1. Overloading decorator - you can pass function returning an unescaped string:
@PrimaryColumn({ default: () => "new_id()" })

it could be even default: unescaped => "new_id()" or sth like this in intelisense.

  1. Parse string - if first char is '=' (or '!'), don't escape string. But it's like old JS style.

It could also be an alternative to preventDefaultEscaping option 😉

@pleerock
Copy link
Member

Thank you guys for the feedback.

preventDefaultEscaping really looks shitty beside your ideas.

@eduardoweiland This is really cool idea, much better then preventDefaultEscaping.

@19majkel94 I would like to prevent extra rules like "!" or "&" which aren't intuitive. But you bring a valueable point, what if value is lets say TRUE or FALSE or number, or string, or function.

I think better if we won't do escaping at all, and those who want to use strings should put single quotes. It will solve all the problems and its intuitive enought (but still we need point this in docs).

@pleerock
Copy link
Member

Ive made a decision to use @19majkel94 suggested solution to use function notation for default values for now. It will work this way:

@Column({ default: "Umed" })
name: string;

generates DEFAULT 'Umed'

@Column({ default: 1 })
count: number;

generates DEFAULT 1

@Column({ default: true })
isSomething: boolean;

generates DEFAULT TRUE

@Column({ default: () => "pow(5)" })
count: number;

generates DEFAULT pow(5)

This fix released in 0.0.7-alpha.11

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

No branches or pull requests

4 participants