Skip to content

Conversation

@billy1624
Copy link
Member

@billy1624 billy1624 commented Aug 29, 2022

PR Info

Adds

  • ColumnDef::default_expr() setting the default expression of a column
  • ColumnDef::default_value() setting the default value of a column

Breaking Changes

  • ColumnDef::default() now takes Into<SimpleExpr> instead of Into<Value>. Users are advised to use ColumnDef::default_value().
@billy1624 billy1624 self-assigned this Aug 29, 2022
@billy1624 billy1624 marked this pull request as ready for review August 29, 2022 06:35
@billy1624 billy1624 requested review from ikrivosheev and tyt2y3 and removed request for tyt2y3 August 29, 2022 06:36
@billy1624
Copy link
Member Author

Hey @ikrivosheev, I remember you said you got some ideas for this PR?

@ikrivosheev
Copy link
Contributor

ikrivosheev commented Sep 3, 2022

Hey @ikrivosheev, I remember you said you got some ideas for this PR?

@billy1624 hello! Sorry for delay. I worked on this task and came to the conclusion that we need new trait:

pub trait SqlWriter: Write {
    fn push_param(&mut self, value: &Value, query_builder: &dyn QueryBuilder);
    fn query(self) -> String;
}

And two new struct:

pub struct SqlStringWriter {
    query: String,
}

and

pub struct SqlWriterObj {  // TODO: better name
    counter: usize,
    placeholder: String,
    numbered: bool,
    values: Vec<Value>,
    query: String,
}

And the backend will no longer have to worry about where and how to put the parameter. What do you think? I can create draft PR.

@ikrivosheev
Copy link
Contributor

Hey @ikrivosheev, I remember you said you got some ideas for this PR?

@billy1624 hello! Sorry for delay. I worked on this task and came to the conclusion that we need new trait:

pub trait SqlWriter: Write {
    fn push_param(&mut self, value: &Value, query_builder: &dyn QueryBuilder);
    fn query(self) -> String;
}

And two new struct:

pub struct SqlStringWriter {
    query: String,
}

and

pub struct SqlWriterObj {  // TODO: better name
    counter: usize,
    placeholder: String,
    numbered: bool,
    values: Vec<Value>,
    query: String,
}

And the backend will no longer have to worry about where and how to put the parameter. What do you think? I can create draft PR.

@tyt2y3 what do you think about it?

@billy1624
Copy link
Member Author

Cool! If I understand it correctly. @ikrivosheev

  1. Both SqlStringWriter and SqlWriterObj implement SqlWriter trait.
  2. Then queries (e.g. CRUD operations on table) that require value binding use SqlWriterObj to store the SQL and bindings.
  3. Schema queries (e.g. CRUD operations on schema definition) that doesn't require value binding use SqlStringWriter to store the SQL and values are serialized as part of the SQL.
@ikrivosheev
Copy link
Contributor

Cool! If I understand it correctly. @ikrivosheev

  1. Both SqlStringWriter and SqlWriterObj implement SqlWriter trait.
  2. Then queries (e.g. CRUD operations on table) that require value binding use SqlWriterObj to store the SQL and bindings.
  3. Schema queries (e.g. CRUD operations on schema definition) that doesn't require value binding use SqlStringWriter to store the SQL and values are serialized as part of the SQL.

Yes, exactly! I started work on that. If you want, I can prepare draft PR.

@billy1624
Copy link
Member Author

Feel free to work on that :D

@ikrivosheev ikrivosheev mentioned this pull request Sep 8, 2022
6 tasks
@ikrivosheev
Copy link
Contributor

@billy1624 hello! I create draft PR: #436

@tyt2y3
Copy link
Member

tyt2y3 commented Sep 18, 2022

I think we don't need this now since #436 is merged?

@tyt2y3 tyt2y3 closed this Sep 18, 2022
@billy1624
Copy link
Member Author

I think we don't need this now since #436 is merged?

Correct!

@ikrivosheev ikrivosheev deleted the col-default-expr branch November 30, 2022 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants